From 6ca7fbe884d17ef6c18d143421cc3e232bbba516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 2 Jan 2025 14:34:10 -0500 Subject: [PATCH 1/9] [Fiber] Gate Update flag on BeforeMutationMask on flags (#31921) We're currently visiting the snapshot phase for every `Update` flag even though we rarely have to do anything in the Snapshot phase. The only flags that seem to use these wider visits is `enableCreateEventHandleAPI` and `enableUseEffectEventHook` but really neither of those should do that neither. They should schedule explicit Snapshot phases if needed. --- .../react-reconciler/src/ReactFiberFlags.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index 66e9249f15026..0fef260ab23fd 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -7,7 +7,10 @@ * @flow */ -import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; +import { + enableCreateEventHandleAPI, + enableUseEffectEventHook, +} from 'shared/ReactFeatureFlags'; export type Flags = number; @@ -77,17 +80,19 @@ export const MountPassiveDev = /* */ 0b1000000000000000000000000000 // don't contain effects, by checking subtreeFlags. export const BeforeMutationMask: number = - // TODO: Remove Update flag from before mutation phase by re-landing Visibility - // flag logic (see #20043) - Update | Snapshot | (enableCreateEventHandleAPI ? // createEventHandle needs to visit deleted and hidden trees to // fire beforeblur // TODO: Only need to visit Deletions during BeforeMutation phase if an // element is focused. - ChildDeletion | Visibility - : 0); + Update | ChildDeletion | Visibility + : enableUseEffectEventHook + ? // TODO: The useEffectEvent hook uses the snapshot phase for clean up but it + // really should use the mutation phase for this or at least schedule an + // explicit Snapshot phase flag for this. + Update + : 0); export const MutationMask = Placement | From d8b903f49edebdd9ed081ff0514c28fe130cd510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 2 Jan 2025 14:34:26 -0500 Subject: [PATCH 2/9] [Fiber] Avoid return value from commitBeforeMutationEffects (#31922) This is behind an unusual flag (enableCreateEventHandleAPI) that doesn't serve a special return value. I'll be collecting other flags from this phase too. We can just use the global flag and reset it before the next mutation phase. Unlike focusedInstanceHandle this doesn't leak any memory in the meantime. --- packages/react-reconciler/src/ReactFiberCommitWork.js | 9 +++------ packages/react-reconciler/src/ReactFiberWorkLoop.js | 6 ++---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index a244c65e3e886..80a27c450253b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -236,23 +236,20 @@ let inProgressLanes: Lanes | null = null; let inProgressRoot: FiberRoot | null = null; let focusedInstanceHandle: null | Fiber = null; -let shouldFireAfterActiveInstanceBlur: boolean = false; +export let shouldFireAfterActiveInstanceBlur: boolean = false; export function commitBeforeMutationEffects( root: FiberRoot, firstChild: Fiber, -): boolean { +): void { focusedInstanceHandle = prepareForCommit(root.containerInfo); + shouldFireAfterActiveInstanceBlur = false; nextEffect = firstChild; commitBeforeMutationEffects_begin(); // We no longer need to track the active instance fiber - const shouldFire = shouldFireAfterActiveInstanceBlur; - shouldFireAfterActiveInstanceBlur = false; focusedInstanceHandle = null; - - return shouldFire; } function commitBeforeMutationEffects_begin() { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index ded26ed6e2bfb..26544788f1508 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -198,6 +198,7 @@ import { } from './ReactFiberThrow'; import { commitBeforeMutationEffects, + shouldFireAfterActiveInstanceBlur, commitLayoutEffects, commitMutationEffects, commitPassiveMountEffects, @@ -3384,10 +3385,7 @@ function commitRootImpl( // The first phase a "before mutation" phase. We use this phase to read the // state of the host tree right before we mutate it. This is where // getSnapshotBeforeUpdate is called. - const shouldFireAfterActiveInstanceBlur = commitBeforeMutationEffects( - root, - finishedWork, - ); + commitBeforeMutationEffects(root, finishedWork); // The next phase is the mutation phase, where we mutate the host tree. commitMutationEffects(root, finishedWork, lanes); From c81312e3a78dcbf71ed98c8893abe6dbfeaef3f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 2 Jan 2025 14:55:34 -0500 Subject: [PATCH 3/9] [Fiber] Refactor Commit Phase into Separate Functions for Before Mutation/Mutation/Layout (#31930) This is doing some general clean up to be able to split the commit root three phases into three separate async steps. --- .../react-reconciler/src/ReactFiberLane.js | 16 +- .../react-reconciler/src/ReactFiberRoot.js | 2 - .../src/ReactFiberRootScheduler.js | 11 + .../src/ReactFiberWorkLoop.js | 294 ++++++++++-------- .../src/ReactInternalTypes.js | 4 - .../src/__tests__/ReactDeferredValue-test.js | 1 + 6 files changed, 174 insertions(+), 154 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 4c96207bd6696..71a1e973c3af8 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -221,7 +221,11 @@ function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes { } } -export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { +export function getNextLanes( + root: FiberRoot, + wipLanes: Lanes, + rootHasPendingCommit: boolean, +): Lanes { // Early bailout if there's no pending work left. const pendingLanes = root.pendingLanes; if (pendingLanes === NoLanes) { @@ -246,16 +250,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes { // a brief amount of time (i.e. below the "Just Noticeable Difference" // threshold). // - // TODO: finishedLanes is also set when a Suspensey resource, like CSS or - // images, suspends during the commit phase. (We could detect that here by - // checking for root.cancelPendingCommit.) These are also expected to resolve - // quickly, because of preloading, but theoretically they could block forever - // like in a normal "suspend indefinitely" scenario. In the future, we should - // consider only blocking for up to some time limit before discarding the - // commit in favor of prerendering. If we do discard a pending commit, then - // the commit phase callback should act as a ping to try the original - // render again. - const rootHasPendingCommit = root.finishedLanes !== NoLanes; // Do not work on any idle work until all the non-idle work has finished, // even if the work is suspended. diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 29bc7923d893e..4971bb4c2be34 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -61,7 +61,6 @@ function FiberRootNode( this.pendingChildren = null; this.current = null; this.pingCache = null; - this.finishedWork = null; this.timeoutHandle = noTimeout; this.cancelPendingCommit = null; this.context = null; @@ -76,7 +75,6 @@ function FiberRootNode( this.pingedLanes = NoLanes; this.warmLanes = NoLanes; this.expiredLanes = NoLanes; - this.finishedLanes = NoLanes; this.errorRecoveryDisabledLanes = NoLanes; this.shellSuspendCounter = 0; diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index e3791f4c8dcff..d57458fdbf41c 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -69,6 +69,7 @@ import { scheduleMicrotask, shouldAttemptEagerTransition, trackSchedulerEvent, + noTimeout, } from './ReactFiberConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -207,11 +208,15 @@ function flushSyncWorkAcrossRoots_impl( const workInProgressRoot = getWorkInProgressRoot(); const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); + const rootHasPendingCommit = + root.cancelPendingCommit !== null || + root.timeoutHandle !== noTimeout; const nextLanes = getNextLanes( root, root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, + rootHasPendingCommit, ); if ( includesSyncLane(nextLanes) && @@ -335,6 +340,8 @@ function scheduleTaskForRootDuringMicrotask( const pendingPassiveEffectsLanes = getPendingPassiveEffectsLanes(); const workInProgressRoot = getWorkInProgressRoot(); const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); + const rootHasPendingCommit = + root.cancelPendingCommit !== null || root.timeoutHandle !== noTimeout; const nextLanes = enableYieldingBeforePassive && root === rootWithPendingPassiveEffects ? // This will schedule the callback at the priority of the lane but we used to @@ -345,6 +352,7 @@ function scheduleTaskForRootDuringMicrotask( : getNextLanes( root, root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, + rootHasPendingCommit, ); const existingCallbackNode = root.callbackNode; @@ -488,9 +496,12 @@ function performWorkOnRootViaSchedulerTask( // it's available). const workInProgressRoot = getWorkInProgressRoot(); const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); + const rootHasPendingCommit = + root.cancelPendingCommit !== null || root.timeoutHandle !== noTimeout; const lanes = getNextLanes( root, root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes, + rootHasPendingCommit, ); if (lanes === NoLanes) { // No more work on this root. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 26544788f1508..ce253f5436cea 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -14,7 +14,6 @@ import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane'; import type {SuspenseState} from './ReactFiberSuspenseComponent'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; -import type {EventPriority} from './ReactEventPriorities'; import type { PendingTransitionCallbacks, PendingBoundaries, @@ -1240,16 +1239,12 @@ function finishConcurrentRender( } } - // Only set these if we have a complete tree that is ready to be committed. - // We use these fields to determine later whether or not the work should be - // discarded for a fresh render attempt. - root.finishedWork = finishedWork; - root.finishedLanes = lanes; - if (shouldForceFlushFallbacksInDEV()) { // We're inside an `act` scope. Commit immediately. commitRoot( root, + finishedWork, + lanes, workInProgressRootRecoverableErrors, workInProgressTransitions, workInProgressRootDidIncludeRecursiveRenderUpdate, @@ -1282,7 +1277,7 @@ function finishConcurrentRender( didAttemptEntireTree, ); - const nextLanes = getNextLanes(root, NoLanes); + const nextLanes = getNextLanes(root, NoLanes, true); if (nextLanes !== NoLanes) { // There's additional work we can do on this root. We might as well // attempt to work on that while we're suspended. @@ -1352,6 +1347,8 @@ function commitRootWhenReady( completedRenderStartTime: number, // Profiling-only completedRenderEndTime: number, // Profiling-only ) { + root.timeoutHandle = noTimeout; + // TODO: Combine retry throttling with Suspensey commits. Right now they run // one after the other. const BothVisibilityAndMaySuspendCommit = Visibility | MaySuspendCommit; @@ -1385,6 +1382,8 @@ function commitRootWhenReady( commitRoot.bind( null, root, + finishedWork, + lanes, recoverableErrors, transitions, didIncludeRenderPhaseUpdate, @@ -1406,6 +1405,8 @@ function commitRootWhenReady( // Otherwise, commit immediately.; commitRoot( root, + finishedWork, + lanes, recoverableErrors, transitions, didIncludeRenderPhaseUpdate, @@ -1843,9 +1844,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { } } - root.finishedWork = null; - root.finishedLanes = NoLanes; - const timeoutHandle = root.timeoutHandle; if (timeoutHandle !== noTimeout) { // The root previous suspended and scheduled a timeout to commit a fallback @@ -3129,6 +3127,8 @@ const THROTTLED_COMMIT = 2; function commitRoot( root: FiberRoot, + finishedWork: null | Fiber, + lanes: Lanes, recoverableErrors: null | Array>, transitions: Array | null, didIncludeRenderPhaseUpdate: boolean, @@ -3139,48 +3139,9 @@ function commitRoot( suspendedCommitReason: SuspendedCommitReason, // Profiling-only completedRenderStartTime: number, // Profiling-only completedRenderEndTime: number, // Profiling-only -) { - // TODO: This no longer makes any sense. We already wrap the mutation and - // layout phases. Should be able to remove. - const prevTransition = ReactSharedInternals.T; - const previousUpdateLanePriority = getCurrentUpdatePriority(); - try { - setCurrentUpdatePriority(DiscreteEventPriority); - ReactSharedInternals.T = null; - commitRootImpl( - root, - recoverableErrors, - transitions, - didIncludeRenderPhaseUpdate, - previousUpdateLanePriority, - spawnedLane, - updatedLanes, - suspendedRetryLanes, - exitStatus, - suspendedCommitReason, - completedRenderStartTime, - completedRenderEndTime, - ); - } finally { - ReactSharedInternals.T = prevTransition; - setCurrentUpdatePriority(previousUpdateLanePriority); - } -} +): void { + root.cancelPendingCommit = null; -function commitRootImpl( - root: FiberRoot, - recoverableErrors: null | Array>, - transitions: Array | null, - didIncludeRenderPhaseUpdate: boolean, - renderPriorityLevel: EventPriority, - spawnedLane: Lane, - updatedLanes: Lanes, - suspendedRetryLanes: Lanes, - exitStatus: RootExitStatus, // Profiling-only - suspendedCommitReason: SuspendedCommitReason, // Profiling-only - completedRenderStartTime: number, // Profiling-only - completedRenderEndTime: number, // Profiling-only -) { do { // `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which // means `flushPassiveEffects` will sometimes result in additional @@ -3196,9 +3157,6 @@ function commitRootImpl( throw new Error('Should not already be working.'); } - const finishedWork = root.finishedWork; - const lanes = root.finishedLanes; - if (enableProfilerTimer && enableComponentPerformanceTrack) { // Log the previous render phase once we commit. I.e. we weren't interrupted. setCurrentTrackFromLanes(lanes); @@ -3234,19 +3192,17 @@ function commitRootImpl( if (enableSchedulingProfiler) { markCommitStopped(); } - return null; + return; } else { if (__DEV__) { if (lanes === NoLanes) { console.error( - 'root.finishedLanes should not be empty during a commit. This is a ' + + 'finishedLanes should not be empty during a commit. This is a ' + 'bug in React.', ); } } } - root.finishedWork = null; - root.finishedLanes = NoLanes; if (finishedWork === root.current) { throw new Error( @@ -3292,7 +3248,6 @@ function commitRootImpl( // might get scheduled in the commit phase. (See #16714.) // TODO: Delete all other places that schedule the passive effect callback // They're redundant. - let rootDoesHavePassiveEffects: boolean = false; if ( // If this subtree rendered with profiling this commit, we need to visit it to log it. (enableProfilerTimer && @@ -3301,7 +3256,6 @@ function commitRootImpl( (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || (finishedWork.flags & PassiveMask) !== NoFlags ) { - rootDoesHavePassiveEffects = true; pendingPassiveEffectsRemainingLanes = remainingLanes; pendingPassiveEffectsRenderEndTime = completedRenderEndTime; // workInProgressTransitions might be overwritten, so we want @@ -3319,7 +3273,6 @@ function commitRootImpl( // So we can clear these now to allow a new callback to be scheduled. root.callbackNode = null; root.callbackPriority = NoLane; - root.cancelPendingCommit = null; scheduleCallback(NormalSchedulerPriority, () => { if (enableProfilerTimer && enableComponentPerformanceTrack) { // Track the currently executing event if there is one so we can ignore this @@ -3338,7 +3291,6 @@ function commitRootImpl( // so we can clear the callback now. root.callbackNode = null; root.callbackPriority = NoLane; - root.cancelPendingCommit = null; } if (enableProfilerTimer) { @@ -3355,79 +3307,136 @@ function commitRootImpl( } } + // The commit phase is broken into several sub-phases. We do a separate pass + // of the effect list for each phase: all mutation effects come before all + // layout effects, and so on. + // Check if there are any effects in the whole tree. // TODO: This is left over from the effect list implementation, where we had // to check for the existence of `firstEffect` to satisfy Flow. I think the // only other reason this optimization exists is because it affects profiling. // Reconsider whether this is necessary. - const subtreeHasEffects = - (finishedWork.subtreeFlags & - (BeforeMutationMask | MutationMask | LayoutMask | PassiveMask)) !== - NoFlags; - const rootHasEffect = - (finishedWork.flags & - (BeforeMutationMask | MutationMask | LayoutMask | PassiveMask)) !== + const subtreeHasBeforeMutationEffects = + (finishedWork.subtreeFlags & (BeforeMutationMask | MutationMask)) !== NoFlags; + const rootHasBeforeMutationEffect = + (finishedWork.flags & (BeforeMutationMask | MutationMask)) !== NoFlags; - if (subtreeHasEffects || rootHasEffect) { + if (subtreeHasBeforeMutationEffects || rootHasBeforeMutationEffect) { const prevTransition = ReactSharedInternals.T; ReactSharedInternals.T = null; const previousPriority = getCurrentUpdatePriority(); setCurrentUpdatePriority(DiscreteEventPriority); - const prevExecutionContext = executionContext; executionContext |= CommitContext; + try { + // The first phase a "before mutation" phase. We use this phase to read the + // state of the host tree right before we mutate it. This is where + // getSnapshotBeforeUpdate is called. + commitBeforeMutationEffects(root, finishedWork); + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; + } + } + flushMutationEffects(root, finishedWork, lanes); + flushLayoutEffects( + root, + finishedWork, + lanes, + recoverableErrors, + didIncludeRenderPhaseUpdate, + suspendedCommitReason, + completedRenderEndTime, + ); +} - // The commit phase is broken into several sub-phases. We do a separate pass - // of the effect list for each phase: all mutation effects come before all - // layout effects, and so on. - - // The first phase a "before mutation" phase. We use this phase to read the - // state of the host tree right before we mutate it. This is where - // getSnapshotBeforeUpdate is called. - commitBeforeMutationEffects(root, finishedWork); +function flushMutationEffects( + root: FiberRoot, + finishedWork: Fiber, + lanes: Lanes, +): void { + const subtreeMutationHasEffects = + (finishedWork.subtreeFlags & MutationMask) !== NoFlags; + const rootMutationHasEffect = (finishedWork.flags & MutationMask) !== NoFlags; - // The next phase is the mutation phase, where we mutate the host tree. - commitMutationEffects(root, finishedWork, lanes); + if (subtreeMutationHasEffects || rootMutationHasEffect) { + const prevTransition = ReactSharedInternals.T; + ReactSharedInternals.T = null; + const previousPriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + const prevExecutionContext = executionContext; + executionContext |= CommitContext; + try { + // The next phase is the mutation phase, where we mutate the host tree. + commitMutationEffects(root, finishedWork, lanes); - if (enableCreateEventHandleAPI) { - if (shouldFireAfterActiveInstanceBlur) { - afterActiveInstanceBlur(); + if (enableCreateEventHandleAPI) { + if (shouldFireAfterActiveInstanceBlur) { + afterActiveInstanceBlur(); + } } + resetAfterCommit(root.containerInfo); + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; } - resetAfterCommit(root.containerInfo); - - // The work-in-progress tree is now the current tree. This must come after - // the mutation phase, so that the previous tree is still current during - // componentWillUnmount, but before the layout phase, so that the finished - // work is current during componentDidMount/Update. - root.current = finishedWork; - - // The next phase is the layout phase, where we call effects that read - // the host tree after it's been mutated. The idiomatic use case for this is - // layout, but class component lifecycles also fire here for legacy reasons. - if (enableSchedulingProfiler) { - markLayoutEffectsStarted(lanes); - } - commitLayoutEffects(finishedWork, root, lanes); - if (enableSchedulingProfiler) { - markLayoutEffectsStopped(); - } + } - // Tell Scheduler to yield at the end of the frame, so the browser has an - // opportunity to paint. - requestPaint(); + // The work-in-progress tree is now the current tree. This must come after + // the mutation phase, so that the previous tree is still current during + // componentWillUnmount, but before the layout phase, so that the finished + // work is current during componentDidMount/Update. + root.current = finishedWork; +} - executionContext = prevExecutionContext; +function flushLayoutEffects( + root: FiberRoot, + finishedWork: Fiber, + lanes: Lanes, + recoverableErrors: null | Array>, + didIncludeRenderPhaseUpdate: boolean, + suspendedCommitReason: SuspendedCommitReason, // Profiling-only + completedRenderEndTime: number, // Profiling-only +): void { + const subtreeHasLayoutEffects = + (finishedWork.subtreeFlags & LayoutMask) !== NoFlags; + const rootHasLayoutEffect = (finishedWork.flags & LayoutMask) !== NoFlags; - // Reset the priority to the previous non-sync value. - setCurrentUpdatePriority(previousPriority); - ReactSharedInternals.T = prevTransition; - } else { - // No effects. - root.current = finishedWork; + if (subtreeHasLayoutEffects || rootHasLayoutEffect) { + const prevTransition = ReactSharedInternals.T; + ReactSharedInternals.T = null; + const previousPriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + const prevExecutionContext = executionContext; + executionContext |= CommitContext; + try { + // The next phase is the layout phase, where we call effects that read + // the host tree after it's been mutated. The idiomatic use case for this is + // layout, but class component lifecycles also fire here for legacy reasons. + if (enableSchedulingProfiler) { + markLayoutEffectsStarted(lanes); + } + commitLayoutEffects(finishedWork, root, lanes); + if (enableSchedulingProfiler) { + markLayoutEffectsStopped(); + } + } finally { + // Reset the priority to the previous non-sync value. + executionContext = prevExecutionContext; + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; + } } + // Tell Scheduler to yield at the end of the frame, so the browser has an + // opportunity to paint. + requestPaint(); + if (enableProfilerTimer && enableComponentPerformanceTrack) { recordCommitEndTime(); logCommitPhase( @@ -3439,18 +3448,22 @@ function commitRootImpl( ); } - const rootDidHavePassiveEffects = rootDoesHavePassiveEffects; + const rootDidHavePassiveEffects = // If this subtree rendered with profiling this commit, we need to visit it to log it. + (enableProfilerTimer && + enableComponentPerformanceTrack && + finishedWork.actualDuration !== 0) || + (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || + (finishedWork.flags & PassiveMask) !== NoFlags; - if (rootDoesHavePassiveEffects) { + if (rootDidHavePassiveEffects) { // This commit has passive effects. Stash a reference to them. But don't // schedule a callback until after flushing layout work. - rootDoesHavePassiveEffects = false; rootWithPendingPassiveEffects = root; pendingPassiveEffectsLanes = lanes; } else { // There were no passive effects, so we can immediately release the cache // pool for this render. - releaseRootPooledCache(root, remainingLanes); + releaseRootPooledCache(root, root.pendingLanes); if (__DEV__) { nestedPassiveUpdateCount = 0; rootWithPassiveNestedUpdates = null; @@ -3458,7 +3471,7 @@ function commitRootImpl( } // Read this again, since an effect might have updated it - remainingLanes = root.pendingLanes; + let remainingLanes = root.pendingLanes; // Check if there's remaining work on this root // TODO: This is part of the `componentDidCatch` implementation. Its purpose @@ -3482,7 +3495,8 @@ function commitRootImpl( } } - onCommitRootDevTools(finishedWork.stateNode, renderPriorityLevel); + const renderPriority = lanesToEventPriority(lanes); + onCommitRootDevTools(finishedWork.stateNode, renderPriority); if (enableUpdaterTracking) { if (isDevToolsPresent) { @@ -3495,22 +3509,31 @@ function commitRootImpl( } if (recoverableErrors !== null) { - // There were errors during this render, but recovered from them without - // needing to surface it to the UI. We log them here. - const onRecoverableError = root.onRecoverableError; - for (let i = 0; i < recoverableErrors.length; i++) { - const recoverableError = recoverableErrors[i]; - const errorInfo = makeErrorInfo(recoverableError.stack); - if (__DEV__) { - runWithFiberInDEV( - recoverableError.source, - onRecoverableError, - recoverableError.value, - errorInfo, - ); - } else { - onRecoverableError(recoverableError.value, errorInfo); + const prevTransition = ReactSharedInternals.T; + const previousUpdateLanePriority = getCurrentUpdatePriority(); + setCurrentUpdatePriority(DiscreteEventPriority); + ReactSharedInternals.T = null; + try { + // There were errors during this render, but recovered from them without + // needing to surface it to the UI. We log them here. + const onRecoverableError = root.onRecoverableError; + for (let i = 0; i < recoverableErrors.length; i++) { + const recoverableError = recoverableErrors[i]; + const errorInfo = makeErrorInfo(recoverableError.stack); + if (__DEV__) { + runWithFiberInDEV( + recoverableError.source, + onRecoverableError, + recoverableError.value, + errorInfo, + ); + } else { + onRecoverableError(recoverableError.value, errorInfo); + } } + } finally { + ReactSharedInternals.T = prevTransition; + setCurrentUpdatePriority(previousUpdateLanePriority); } } @@ -3610,8 +3633,6 @@ function commitRootImpl( }); } } - - return null; } function makeErrorInfo(componentStack: ?string) { @@ -3705,7 +3726,6 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { // We've finished our work for this render pass. root.callbackNode = null; root.callbackPriority = NoLane; - root.cancelPendingCommit = null; } if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index afe853202f141..859da37cadf32 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -220,8 +220,6 @@ type BaseFiberRootProperties = { pingCache: WeakMap> | Map> | null, - // A finished work-in-progress HostRoot that's ready to be committed. - finishedWork: Fiber | null, // Timeout handle returned by setTimeout. Used to cancel a pending timeout, if // it's superseded by a new one. timeoutHandle: TimeoutHandle | NoTimeout, @@ -252,8 +250,6 @@ type BaseFiberRootProperties = { errorRecoveryDisabledLanes: Lanes, shellSuspendCounter: number, - finishedLanes: Lanes, - entangledLanes: Lanes, entanglements: LaneMap, diff --git a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js index bbd01cd551e86..f57fc0ac0239a 100644 --- a/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDeferredValue-test.js @@ -744,6 +744,7 @@ describe('ReactDeferredValue', () => { , ); // We should switch to pre-rendering the new preview. + await waitForPaint([]); await waitForPaint(['Preview [B]']); expect(root).toMatchRenderedOutput(); From a7c898d83a991c48f3981fcc65d969f1d90d80a1 Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 2 Jan 2025 15:28:06 -0500 Subject: [PATCH 4/9] [assert helpers] react-dom (pt 1) (#31897) Converts ~half of react-dom tests --- .../__tests__/CSSPropertyOperations-test.js | 62 +- .../__tests__/DOMPropertyOperations-test.js | 21 +- .../__tests__/InvalidEventListeners-test.js | 17 +- .../__tests__/ReactChildReconciler-test.js | 90 +-- .../src/__tests__/ReactComponent-test.js | 83 ++- .../__tests__/ReactComponentLifeCycle-test.js | 605 +++++++++++------- .../__tests__/ReactCompositeComponent-test.js | 214 ++++--- .../ReactCompositeComponentState-test.js | 48 +- .../react-dom/src/__tests__/ReactDOM-test.js | 115 ++-- .../src/__tests__/ReactDOMAttribute-test.js | 69 +- .../ReactDeprecationWarnings-test.js | 18 +- .../src/__tests__/findDOMNodeFB-test.js | 10 +- .../ReactDOMServerIntegrationTestUtils.js | 2 +- 13 files changed, 781 insertions(+), 573 deletions(-) diff --git a/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js b/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js index bda95b8434d46..b5671cf759cef 100644 --- a/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js @@ -13,6 +13,8 @@ const React = require('react'); const ReactDOMClient = require('react-dom/client'); const ReactDOMServer = require('react-dom/server'); const act = require('internal-test-utils').act; +const assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; describe('CSSPropertyOperations', () => { it('should automatically append `px` to relevant styles', () => { @@ -103,15 +105,14 @@ describe('CSSPropertyOperations', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Unsupported style property background-color. Did you mean backgroundColor?' + '\n in div (at **)' + '\n in Comp (at **)', - ); + ]); }); it('should warn when updating hyphenated style names', async () => { @@ -132,11 +133,10 @@ describe('CSSPropertyOperations', () => { await act(() => { root.render(); }); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev([ + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Unsupported style property -ms-transform. Did you mean msTransform?' + '\n in div (at **)' + '\n in Comp (at **)', @@ -165,11 +165,10 @@ describe('CSSPropertyOperations', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev([ + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ // msTransform is correct already and shouldn't warn 'Unsupported vendor-prefixed style property oTransform. ' + 'Did you mean OTransform?' + @@ -202,11 +201,10 @@ describe('CSSPropertyOperations', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev([ + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ "Style property values shouldn't contain a semicolon. " + 'Try "backgroundColor: blue" instead.' + '\n in div (at **)' + @@ -229,15 +227,14 @@ describe('CSSPropertyOperations', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ '`NaN` is an invalid value for the `fontSize` css style property.' + '\n in div (at **)' + '\n in Comp (at **)', - ); + ]); }); it('should not warn when setting CSS custom properties', async () => { @@ -265,15 +262,14 @@ describe('CSSPropertyOperations', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ '`Infinity` is an invalid value for the `fontSize` css style property.' + '\n in div (at **)' + '\n in Comp (at **)', - ); + ]); }); it('should not add units to CSS custom properties', async () => { diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index cecc71b45a6c0..ef09e49bf36c1 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -1333,7 +1333,8 @@ describe('DOMPropertyOperations', () => { }); assertConsoleErrorDev([ - 'The `popoverTarget` prop expects the ID of an Element as a string. Received HTMLDivElement {} instead.', + 'The `popoverTarget` prop expects the ID of an Element as a string. Received HTMLDivElement {} instead.\n' + + ' in button (at **)', ]); // Dedupe warning @@ -1375,13 +1376,17 @@ describe('DOMPropertyOperations', () => { expect(container.firstChild.getAttribute('value')).toBe('foo'); } expect(container.firstChild.value).toBe('foo'); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'A component is changing a controlled input to be uncontrolled', - ); + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'A component is changing a controlled input to be uncontrolled. ' + + 'This is likely caused by the value changing from a defined to undefined, ' + + 'which should not happen. Decide between using a controlled or uncontrolled ' + + 'input element for the lifetime of the component. ' + + 'More info: https://react.dev/link/controlled-components\n' + + ' in input (at **)', + ]); if (disableInputAttributeSyncing) { expect(container.firstChild.hasAttribute('value')).toBe(false); } else { diff --git a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js index d7045d2756ad4..8bff125c82c32 100644 --- a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js +++ b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js @@ -15,13 +15,14 @@ describe('InvalidEventListeners', () => { let React; let ReactDOMClient; let act; + let assertConsoleErrorDev; let container; beforeEach(() => { jest.resetModules(); React = require('react'); ReactDOMClient = require('react-dom/client'); - act = require('internal-test-utils').act; + ({act, assertConsoleErrorDev} = require('internal-test-utils')); container = document.createElement('div'); document.body.appendChild(container); @@ -34,13 +35,13 @@ describe('InvalidEventListeners', () => { it('should prevent non-function listeners, at dispatch', async () => { const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Expected `onClick` listener to be a function, instead got a value of `string` type.', - ); + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Expected `onClick` listener to be a function, instead got a value of `string` type.\n' + + ' in div (at **)', + ]); const node = container.firstChild; console.error = jest.fn(); diff --git a/packages/react-dom/src/__tests__/ReactChildReconciler-test.js b/packages/react-dom/src/__tests__/ReactChildReconciler-test.js index 1c1475534f3e0..ba0c028fa863a 100644 --- a/packages/react-dom/src/__tests__/ReactChildReconciler-test.js +++ b/packages/react-dom/src/__tests__/ReactChildReconciler-test.js @@ -15,6 +15,7 @@ let React; let ReactDOMClient; let act; +let assertConsoleErrorDev; describe('ReactChildReconciler', () => { beforeEach(() => { @@ -22,7 +23,7 @@ describe('ReactChildReconciler', () => { React = require('react'); ReactDOMClient = require('react-dom/client'); - act = require('internal-test-utils').act; + ({act, assertConsoleErrorDev} = require('internal-test-utils')); }); function createIterable(array) { @@ -62,15 +63,21 @@ describe('ReactChildReconciler', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render( -
-

{iterableFunction}

-
, - ); - }); - }).toErrorDev('Functions are not valid as a React child'); + await act(() => { + root.render( +
+

{iterableFunction}

+
, + ); + }); + assertConsoleErrorDev([ + 'Functions are not valid as a React child. ' + + 'This may happen if you return fn instead of from render. ' + + 'Or maybe you meant to call this function rather than return it.\n' + + '

{fn}

\n' + + ' in h1 (at **)' + + (gate('enableOwnerStacks') ? '' : '\n in div (at **)'), + ]); const node = container.firstChild; expect(node.innerHTML).toContain(''); // h1 @@ -85,16 +92,18 @@ describe('ReactChildReconciler', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'Keys should be unique so that components maintain their identity ' + - 'across updates. Non-unique keys may cause children to be ' + - 'duplicated and/or omitted — the behavior is unsupported and ' + - 'could change in a future version.', - ); + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Encountered two children with the same key, `1`. ' + + 'Keys should be unique so that components maintain their identity across updates. ' + + 'Non-unique keys may cause children to be duplicated and/or omitted — ' + + 'the behavior is unsupported and could change in a future version.\n' + + (gate('enableOwnerStacks') ? '' : ' in div (at **)\n') + + ' in div (at **)\n' + + ' in Component (at **)', + ]); }); it('warns for duplicated array keys with component stack info', async () => { @@ -118,11 +127,10 @@ describe('ReactChildReconciler', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Encountered two children with the same key, `1`. ' + 'Keys should be unique so that components maintain their identity ' + 'across updates. Non-unique keys may cause children to be ' + @@ -135,7 +143,7 @@ describe('ReactChildReconciler', () => { ? '' : ' in Parent (at **)\n') + ' in GrandParent (at **)', - ); + ]); }); it('warns for duplicated iterable keys', async () => { @@ -147,16 +155,19 @@ describe('ReactChildReconciler', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'Keys should be unique so that components maintain their identity ' + + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Encountered two children with the same key, `1`. ' + + 'Keys should be unique so that components maintain their identity ' + 'across updates. Non-unique keys may cause children to be ' + 'duplicated and/or omitted — the behavior is unsupported and ' + - 'could change in a future version.', - ); + 'could change in a future version.\n' + + ' in div (at **)\n' + + (gate(flags => flags.enableOwnerStacks) ? '' : ' in div (at **)\n') + + ' in Component (at **)', + ]); }); it('warns for duplicated iterable keys with component stack info', async () => { @@ -180,11 +191,10 @@ describe('ReactChildReconciler', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Encountered two children with the same key, `1`. ' + 'Keys should be unique so that components maintain their identity ' + 'across updates. Non-unique keys may cause children to be ' + @@ -197,6 +207,6 @@ describe('ReactChildReconciler', () => { ? '' : ' in Parent (at **)\n') + ' in GrandParent (at **)', - ); + ]); }); }); diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 6459114697540..c59ba61e01c44 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -411,7 +411,9 @@ describe('ReactComponent', () => { assertConsoleErrorDev( [ 'React.jsx: type is invalid -- expected a string (for built-in components) ' + - 'or a class/function (for composite components) but got: undefined.', + 'or a class/function (for composite components) but got: undefined. ' + + "You likely forgot to export your component from the file it's defined in, " + + 'or you might have mixed up default and named imports.', ], {withoutStack: true}, ); @@ -491,16 +493,11 @@ describe('ReactComponent', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect( - expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'React.jsx: type is invalid -- expected a string (for built-in components) ' + - 'or a class/function (for composite components) but got: undefined.', - ), - ).rejects.toThrowError( + await expect(async () => { + await act(() => { + root.render(); + }); + }).rejects.toThrowError( 'Element type is invalid: expected a string (for built-in components) ' + 'or a class/function (for composite components) but got: undefined.' + (__DEV__ @@ -509,6 +506,26 @@ describe('ReactComponent', () => { '\n\nCheck the render method of `Bar`.' : ''), ); + if (!gate('enableOwnerStacks')) { + assertConsoleErrorDev([ + 'React.jsx: type is invalid -- expected a string (for built-in components) ' + + 'or a class/function (for composite components) but got: undefined.' + + (__DEV__ + ? " You likely forgot to export your component from the file it's defined in, " + + 'or you might have mixed up default and named imports.\n' + + ' in Bar (at **)\n' + + ' in Foo (at **)' + : ''), + 'React.jsx: type is invalid -- expected a string (for built-in components) ' + + 'or a class/function (for composite components) but got: undefined.' + + (__DEV__ + ? " You likely forgot to export your component from the file it's defined in, " + + 'or you might have mixed up default and named imports.\n' + + ' in Bar (at **)\n' + + ' in Foo (at **)' + : ''), + ]); + } }); it('throws if a plain object is used as a child', async () => { @@ -624,17 +641,16 @@ describe('ReactComponent', () => { } const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Functions are not valid as a React child. This may happen if ' + 'you return Foo instead of from render. ' + 'Or maybe you meant to call this function rather than return it.\n' + ' {Foo}\n' + ' in Foo (at **)', - ); + ]); }); it('warns on function as a return value from a class', async () => { @@ -644,19 +660,18 @@ describe('ReactComponent', () => { } } const container = document.createElement('div'); - await expect(async () => { - const root = ReactDOMClient.createRoot(container); + const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Functions are not valid as a React child. This may happen if ' + 'you return Foo instead of from render. ' + 'Or maybe you meant to call this function rather than return it.\n' + ' {Foo}\n' + ' in Foo (at **)', - ); + ]); }); it('warns on function as a child to host component', async () => { @@ -669,11 +684,10 @@ describe('ReactComponent', () => { } const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Functions are not valid as a React child. This may happen if ' + 'you return Foo instead of from render. ' + 'Or maybe you meant to call this function rather than return it.\n' + @@ -683,7 +697,7 @@ describe('ReactComponent', () => { ? '' : ' in div (at **)\n') + ' in Foo (at **)', - ); + ]); }); it('does not warn for function-as-a-child that gets resolved', async () => { @@ -724,11 +738,10 @@ describe('ReactComponent', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); let component; - await expect(async () => { - await act(() => { - root.render( (component = current)} />); - }); - }).toErrorDev([ + await act(() => { + root.render( (component = current)} />); + }); + assertConsoleErrorDev([ 'Functions are not valid as a React child. This may happen if ' + 'you return Foo instead of from render. ' + 'Or maybe you meant to call this function rather than return it.\n' + diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index 428dac1f1ea9e..75e4cebe80610 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -14,6 +14,8 @@ let act; let React; let ReactDOM; let ReactDOMClient; +let assertConsoleErrorDev; +let assertConsoleWarnDev; const clone = function (o) { return JSON.parse(JSON.stringify(o)); @@ -90,7 +92,11 @@ describe('ReactComponentLifeCycle', () => { beforeEach(() => { jest.resetModules(); - act = require('internal-test-utils').act; + ({ + act, + assertConsoleErrorDev, + assertConsoleWarnDev, + } = require('internal-test-utils')); React = require('react'); ReactDOM = require('react-dom'); @@ -239,15 +245,15 @@ describe('ReactComponentLifeCycle', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'StatefulComponent: It is not recommended to assign props directly to state ' + "because updates to props won't be reflected in state. " + - 'In most cases, it is better to use props directly.', - ); + 'In most cases, it is better to use props directly.\n' + + ' in StatefulComponent (at **)', + ]); }); it('should not allow update state inside of getInitialState', async () => { @@ -266,16 +272,16 @@ describe('ReactComponentLifeCycle', () => { let container = document.createElement('div'); let root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ "Can't call setState on a component that is not yet mounted. " + 'This is a no-op, but it might indicate a bug in your application. ' + 'Instead, assign to `this.state` directly or define a `state = {};` ' + - 'class property with the desired state in the StatefulComponent component.', - ); + 'class property with the desired state in the StatefulComponent component.\n' + + ' in StatefulComponent (at **)', + ]); container = document.createElement('div'); root = ReactDOMClient.createRoot(container); @@ -308,11 +314,17 @@ describe('ReactComponentLifeCycle', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(element); - }); - }).toErrorDev('Component is accessing isMounted inside its render()'); + await act(() => { + root.render(element); + }); + assertConsoleErrorDev([ + 'Component is accessing isMounted inside its render() function. ' + + 'render() should be a pure function of props and state. ' + + 'It should never access something that requires stale data ' + + 'from the previous render, such as refs. ' + + 'Move this logic to componentDidMount and componentDidUpdate instead.\n' + + ' in Component (at **)', + ]); expect(instance._isMounted()).toBeTruthy(); }); @@ -340,11 +352,17 @@ describe('ReactComponentLifeCycle', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(element); - }); - }).toErrorDev('Component is accessing isMounted inside its render()'); + await act(() => { + root.render(element); + }); + assertConsoleErrorDev([ + 'Component is accessing isMounted inside its render() function. ' + + 'render() should be a pure function of props and state. ' + + 'It should never access something that requires stale data ' + + 'from the previous render, such as refs. ' + + 'Move this logic to componentDidMount and componentDidUpdate instead.\n' + + ' in Component (at **)', + ]); expect(instance._isMounted()).toBeTruthy(); }); @@ -390,11 +408,17 @@ describe('ReactComponentLifeCycle', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev('Component is accessing findDOMNode inside its render()'); + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Component is accessing findDOMNode inside its render(). ' + + 'render() should be a pure function of props and state. ' + + 'It should never access something that requires stale data ' + + 'from the previous render, such as refs. ' + + 'Move this logic to componentDidMount and componentDidUpdate instead.\n' + + ' in Component (at **)', + ]); }); it('should carry through each of the phases of setup', async () => { @@ -453,13 +477,17 @@ describe('ReactComponentLifeCycle', () => { const root = ReactDOMClient.createRoot(document.createElement('div')); const instanceRef = React.createRef(); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'LifeCycleComponent is accessing isMounted inside its render() function', - ); + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'LifeCycleComponent is accessing isMounted inside its render() function. ' + + 'render() should be a pure function of props and state. ' + + 'It should never access something that requires stale data ' + + 'from the previous render, such as refs. ' + + 'Move this logic to componentDidMount and componentDidUpdate instead.\n' + + ' in LifeCycleComponent (at **)', + ]); const instance = instanceRef.current; // getInitialState @@ -781,19 +809,45 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.', - ); - }).toWarnDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'Component uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillReceiveProps\n' + + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in Component (at **)', + ]); + assertConsoleWarnDev( [ - 'componentWillMount has been renamed', - 'componentWillReceiveProps has been renamed', - 'componentWillUpdate has been renamed', + 'componentWillMount has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move code with side effects to componentDidMount, and set initial state in the constructor.\n' + + '* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: Component', + 'componentWillReceiveProps has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + "* If you're updating state whenever props change, refactor your code to use " + + 'memoization techniques or move it to static getDerivedStateFromProps. ' + + 'Learn more at: https://react.dev/link/derived-state\n' + + '* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: Component', + 'componentWillUpdate has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + '* Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: Component', ], {withoutStack: true}, ); @@ -821,20 +875,45 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await expect( - async () => - await act(() => { - root.render(); - }), - ).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.', - ); - }).toWarnDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'Component uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillReceiveProps\n' + + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in Component (at **)', + ]); + assertConsoleWarnDev( [ - 'componentWillMount has been renamed', - 'componentWillReceiveProps has been renamed', - 'componentWillUpdate has been renamed', + 'componentWillMount has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move code with side effects to componentDidMount, and set initial state in the constructor.\n' + + '* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: Component', + 'componentWillReceiveProps has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + "* If you're updating state whenever props change, refactor your code to use " + + 'memoization techniques or move it to static getDerivedStateFromProps. ' + + 'Learn more at: https://react.dev/link/derived-state\n' + + '* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: Component', + 'componentWillUpdate has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + '* Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: Component', ], {withoutStack: true}, ); @@ -865,14 +944,19 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect( - async () => - await act(() => { - root.render(); - }), - ).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.', - ); + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'Component uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' UNSAFE_componentWillMount\n' + + ' UNSAFE_componentWillReceiveProps\n' + + ' UNSAFE_componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in Component (at **)', + ]); await act(() => { root.render(); }); @@ -893,24 +977,35 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + - 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillMount\n' + - ' UNSAFE_componentWillReceiveProps\n' + - ' componentWillUpdate\n\n' + - 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); - }).toWarnDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' UNSAFE_componentWillReceiveProps\n' + + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in AllLegacyLifecycles (at **)', + ]); + assertConsoleWarnDev( [ - 'componentWillMount has been renamed', - 'componentWillUpdate has been renamed', + 'componentWillMount has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move code with side effects to componentDidMount, and set initial state in the constructor.\n' + + '* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: AllLegacyLifecycles', + 'componentWillUpdate has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + '* Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: AllLegacyLifecycles', ], {withoutStack: true}, ); @@ -926,17 +1021,17 @@ describe('ReactComponentLifeCycle', () => { } } - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' UNSAFE_componentWillMount\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in WillMount (at **)', + ]); class WillMountAndUpdate extends React.Component { state = {}; @@ -950,23 +1045,30 @@ describe('ReactComponentLifeCycle', () => { } } - await expect(async () => { - await expect( - async () => - await act(() => { - root.render(); - }), - ).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + - 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillMount\n' + - ' UNSAFE_componentWillUpdate\n\n' + - 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); - }).toWarnDev(['componentWillMount has been renamed'], { - withoutStack: true, + await act(() => { + root.render(); }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' UNSAFE_componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in WillMountAndUpdate (at **)', + ]); + assertConsoleWarnDev( + [ + 'componentWillMount has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move code with side effects to componentDidMount, and set initial state in the constructor.\n' + + '* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: WillMountAndUpdate', + ], + {withoutStack: true}, + ); class WillReceiveProps extends React.Component { state = {}; @@ -979,21 +1081,34 @@ describe('ReactComponentLifeCycle', () => { } } - await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + - 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillReceiveProps\n\n' + - 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); - }).toWarnDev(['componentWillReceiveProps has been renamed'], { - withoutStack: true, + await act(() => { + root.render(); }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillReceiveProps\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in WillReceiveProps (at **)', + ]); + assertConsoleWarnDev( + [ + 'componentWillReceiveProps has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + "* If you're updating state whenever props change, refactor your code to use " + + 'memoization techniques or move it to static getDerivedStateFromProps. ' + + 'Learn more at: https://react.dev/link/derived-state\n' + + '* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: WillReceiveProps', + ], + { + withoutStack: true, + }, + ); }); it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', async () => { @@ -1010,24 +1125,35 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + - 'AllLegacyLifecycles uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + - ' componentWillMount\n' + - ' UNSAFE_componentWillReceiveProps\n' + - ' componentWillUpdate\n\n' + - 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); - }).toWarnDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'AllLegacyLifecycles uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' UNSAFE_componentWillReceiveProps\n' + + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in AllLegacyLifecycles (at **)', + ]); + assertConsoleWarnDev( [ - 'componentWillMount has been renamed', - 'componentWillUpdate has been renamed', + 'componentWillMount has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move code with side effects to componentDidMount, and set initial state in the constructor.\n' + + '* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: AllLegacyLifecycles', + 'componentWillUpdate has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + '* Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: AllLegacyLifecycles', ], {withoutStack: true}, ); @@ -1042,17 +1168,17 @@ describe('ReactComponentLifeCycle', () => { } } - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + 'WillMount uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + ' UNSAFE_componentWillMount\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in WillMount (at **)', + ]); class WillMountAndUpdate extends React.Component { state = {}; @@ -1065,22 +1191,32 @@ describe('ReactComponentLifeCycle', () => { } } - await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + - 'WillMountAndUpdate uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + - ' componentWillMount\n' + - ' UNSAFE_componentWillUpdate\n\n' + - 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); - }).toWarnDev(['componentWillMount has been renamed'], { - withoutStack: true, + await act(() => { + root.render(); }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'WillMountAndUpdate uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' UNSAFE_componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in WillMountAndUpdate (at **)', + ]); + assertConsoleWarnDev( + [ + 'componentWillMount has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move code with side effects to componentDidMount, and set initial state in the constructor.\n' + + '* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: WillMountAndUpdate', + ], + { + withoutStack: true, + }, + ); class WillReceiveProps extends React.Component { state = {}; @@ -1092,22 +1228,34 @@ describe('ReactComponentLifeCycle', () => { } } - await expect(async () => { - await expect( - async () => - await act(() => { - root.render(); - }), - ).toErrorDev( - 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + - 'WillReceiveProps uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + - ' componentWillReceiveProps\n\n' + - 'The above lifecycles should be removed. Learn more about this warning here:\n' + - 'https://react.dev/link/unsafe-component-lifecycles', - ); - }).toWarnDev(['componentWillReceiveProps has been renamed'], { - withoutStack: true, + await act(() => { + root.render(); }); + assertConsoleErrorDev([ + 'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' + + 'WillReceiveProps uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' + + ' componentWillReceiveProps\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://react.dev/link/unsafe-component-lifecycles\n' + + ' in WillReceiveProps (at **)', + ]); + assertConsoleWarnDev( + [ + 'componentWillReceiveProps has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + "* If you're updating state whenever props change, refactor your code to use " + + 'memoization techniques or move it to static getDerivedStateFromProps. ' + + 'Learn more at: https://react.dev/link/derived-state\n' + + '* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: WillReceiveProps', + ], + { + withoutStack: true, + }, + ); }); it('should warn if getDerivedStateFromProps returns undefined', async () => { @@ -1120,14 +1268,14 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'MyComponent.getDerivedStateFromProps(): A valid state object (or null) must ' + - 'be returned. You have returned undefined.', - ); + 'be returned. You have returned undefined.\n' + + ' in MyComponent (at **)', + ]); // De-duped await act(() => { @@ -1146,16 +1294,16 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ '`MyComponent` uses `getDerivedStateFromProps` but its initial state is ' + 'undefined. This is not recommended. Instead, define the initial state by ' + 'assigning an object to `this.state` in the constructor of `MyComponent`. ' + - 'This ensures that `getDerivedStateFromProps` arguments have a consistent shape.', - ); + 'This ensures that `getDerivedStateFromProps` arguments have a consistent shape.\n' + + ' in MyComponent (at **)', + ]); // De-duped await act(() => { @@ -1191,15 +1339,35 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toWarnDev( + await act(() => { + root.render(); + }); + assertConsoleWarnDev( [ - 'componentWillMount has been renamed', - 'componentWillReceiveProps has been renamed', - 'componentWillUpdate has been renamed', + 'componentWillMount has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move code with side effects to componentDidMount, and set initial state in the constructor.\n' + + '* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: MyComponent', + 'componentWillReceiveProps has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + "* If you're updating state whenever props change, refactor your code to use " + + 'memoization techniques or move it to static getDerivedStateFromProps. ' + + 'Learn more at: https://react.dev/link/derived-state\n' + + '* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: MyComponent', + 'componentWillUpdate has been renamed, and is not recommended for use. ' + + 'See https://react.dev/link/unsafe-component-lifecycles for details.\n\n' + + '* Move data fetching code or side effects to componentDidUpdate.\n' + + '* Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. ' + + 'In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, ' + + 'you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder.\n\n' + + 'Please update the following components: MyComponent', ], {withoutStack: true}, ); @@ -1444,14 +1612,14 @@ describe('ReactComponentLifeCycle', () => { root.render(); }); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'MyComponent.getSnapshotBeforeUpdate(): A snapshot value (or null) must ' + - 'be returned. You have returned undefined.', - ); + 'be returned. You have returned undefined.\n' + + ' in MyComponent (at **)', + ]); // De-duped await act(() => { @@ -1470,14 +1638,14 @@ describe('ReactComponentLifeCycle', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'MyComponent: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' + - 'This component defines getSnapshotBeforeUpdate() only.', - ); + 'This component defines getSnapshotBeforeUpdate() only.\n' + + ' in MyComponent (at **)', + ]); // De-duped await act(() => { @@ -1497,11 +1665,10 @@ describe('ReactComponentLifeCycle', () => { const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toWarnDev( + await act(() => { + root.render(); + }); + assertConsoleWarnDev( [ `componentWillMount has been renamed, and is not recommended for use. See https://react.dev/link/unsafe-component-lifecycles for details. diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 30b5ced5cf2e3..3d5f61c1ed520 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -18,6 +18,7 @@ let ReactSharedInternals; let Scheduler; let assertLog; let act; +let assertConsoleErrorDev; describe('ReactCompositeComponent', () => { const hasOwnProperty = Object.prototype.hasOwnProperty; @@ -71,7 +72,7 @@ describe('ReactCompositeComponent', () => { require('react').__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE; Scheduler = require('scheduler'); assertLog = require('internal-test-utils').assertLog; - act = require('internal-test-utils').act; + ({act, assertConsoleErrorDev} = require('internal-test-utils')); }); describe('MorphingComponent', () => { @@ -308,16 +309,16 @@ describe('ReactCompositeComponent', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ "Can't call forceUpdate on a component that is not yet mounted. " + 'This is a no-op, but it might indicate a bug in your application. ' + 'Instead, assign to `this.state` directly or define a `state = {};` ' + - 'class property with the desired state in the MyComponent component.', - ); + 'class property with the desired state in the MyComponent component.\n' + + ' in MyComponent (at **)', + ]); // No additional warning should be recorded const container2 = document.createElement('div'); @@ -342,16 +343,16 @@ describe('ReactCompositeComponent', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ "Can't call setState on a component that is not yet mounted. " + 'This is a no-op, but it might indicate a bug in your application. ' + 'Instead, assign to `this.state` directly or define a `state = {};` ' + - 'class property with the desired state in the MyComponent component.', - ); + 'class property with the desired state in the MyComponent component.\n' + + ' in MyComponent (at **)', + ]); // No additional warning should be recorded const container2 = document.createElement('div'); @@ -478,16 +479,16 @@ describe('ReactCompositeComponent', () => { } const root = ReactDOMClient.createRoot(container); await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).rejects.toThrow(TypeError); - }).toErrorDev( + await act(() => { + root.render(); + }); + }).rejects.toThrow(TypeError); + assertConsoleErrorDev([ 'The component appears to have a render method, ' + "but doesn't extend React.Component. This is likely to cause errors. " + - 'Change ClassWithRenderNotExtended to extend React.Component instead.', - ); + 'Change ClassWithRenderNotExtended to extend React.Component instead.\n' + + ' in ClassWithRenderNotExtended (at **)', + ]); // Test deduplication await expect(async () => { @@ -514,14 +515,14 @@ describe('ReactCompositeComponent', () => { let instance; const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { - root.render( (instance = ref)} />); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render( (instance = ref)} />); + }); + assertConsoleErrorDev([ 'Cannot update during an existing state transition (such as within ' + - '`render`). Render methods should be a pure function of props and state.', - ); + '`render`). Render methods should be a pure function of props and state.\n' + + ' in Component (at **)', + ]); // The setState call is queued and then executed as a second pass. This // behavior is undefined though so we're free to change it to suit the @@ -618,14 +619,14 @@ describe('ReactCompositeComponent', () => { root.render( (instance = ref)} />); }); - expect(() => { - ReactDOM.flushSync(() => { - instance.setState({bogus: true}); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + instance.setState({bogus: true}); + }); + assertConsoleErrorDev([ 'ClassComponent.shouldComponentUpdate(): Returned undefined instead of a ' + - 'boolean value. Make sure to return true or false.', - ); + 'boolean value. Make sure to return true or false.\n' + + ' in ClassComponent (at **)', + ]); }); it('should warn when componentDidUnmount method is defined', async () => { @@ -638,15 +639,15 @@ describe('ReactCompositeComponent', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Component has a method called ' + 'componentDidUnmount(). But there is no such lifecycle method. ' + - 'Did you mean componentWillUnmount()?', - ); + 'Did you mean componentWillUnmount()?\n' + + ' in Component (at **)', + ]); }); it('should warn when componentDidReceiveProps method is defined', () => { @@ -660,17 +661,17 @@ describe('ReactCompositeComponent', () => { const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Component has a method called ' + 'componentDidReceiveProps(). But there is no such lifecycle method. ' + 'If you meant to update the state in response to changing props, ' + 'use componentWillReceiveProps(). If you meant to fetch data or ' + - 'run side-effects or mutations after React has updated the UI, use componentDidUpdate().', - ); + 'run side-effects or mutations after React has updated the UI, use componentDidUpdate().\n' + + ' in Component (at **)', + ]); }); it('should warn when defaultProps was defined as an instance property', () => { @@ -686,14 +687,14 @@ describe('ReactCompositeComponent', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Setting defaultProps as an instance property on Component is not supported ' + - 'and will be ignored. Instead, define defaultProps as a static property on Component.', - ); + 'and will be ignored. Instead, define defaultProps as a static property on Component.\n' + + ' in Component (at **)', + ]); }); it('should skip update when rerendering element in container', async () => { @@ -739,16 +740,16 @@ describe('ReactCompositeComponent', () => { } } - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Render methods should be a pure function of props and state; ' + 'triggering nested component updates from render is not allowed. If ' + - 'necessary, trigger nested updates in componentDidUpdate.\n\nCheck the ' + - 'render method of Outer.', - ); + 'necessary, trigger nested updates in componentDidUpdate.\n\n' + + 'Check the render method of Outer.\n' + + ' in Outer (at **)', + ]); }); it('only renders once if updated in componentWillReceiveProps', async () => { @@ -836,14 +837,14 @@ describe('ReactCompositeComponent', () => { } const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'When calling super() in `Foo`, make sure to pass ' + - "up the same props that your component's constructor was passed.", - ); + "up the same props that your component's constructor was passed.\n" + + ' in Foo (at **)', + ]); }); it('should only call componentWillUnmount once', async () => { @@ -1185,16 +1186,17 @@ describe('ReactCompositeComponent', () => { const root = ReactDOMClient.createRoot(document.createElement('div')); await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).rejects.toThrow(); - }).toErrorDev([ + await act(() => { + root.render(); + }); + }).rejects.toThrow(); + assertConsoleErrorDev([ 'No `render` method found on the RenderTextInvalidConstructor instance: ' + - 'did you accidentally return an object from the constructor?', + 'did you accidentally return an object from the constructor?\n' + + ' in RenderTextInvalidConstructor (at **)', 'No `render` method found on the RenderTextInvalidConstructor instance: ' + - 'did you accidentally return an object from the constructor?', + 'did you accidentally return an object from the constructor?\n' + + ' in RenderTextInvalidConstructor (at **)', ]); }); @@ -1210,14 +1212,14 @@ describe('ReactCompositeComponent', () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'It looks like Bad is reassigning its own `this.props` while rendering. ' + - 'This is not supported and can lead to confusing bugs.', - ); + 'This is not supported and can lead to confusing bugs.\n' + + ' in Bad (at **)', + ]); }); it('should return error if render is not defined', async () => { @@ -1225,16 +1227,17 @@ describe('ReactCompositeComponent', () => { const root = ReactDOMClient.createRoot(document.createElement('div')); await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).rejects.toThrow(); - }).toErrorDev([ + await act(() => { + root.render(); + }); + }).rejects.toThrow(); + assertConsoleErrorDev([ 'No `render` method found on the RenderTestUndefinedRender instance: ' + - 'you may have forgotten to define `render`.', + 'you may have forgotten to define `render`.\n' + + ' in RenderTestUndefinedRender (at **)', 'No `render` method found on the RenderTestUndefinedRender instance: ' + - 'you may have forgotten to define `render`.', + 'you may have forgotten to define `render`.\n' + + ' in RenderTestUndefinedRender (at **)', ]); }); @@ -1386,13 +1389,18 @@ describe('ReactCompositeComponent', () => { } const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( - 'Cannot update a component (`A`) while rendering a different component (`B`)', - ); + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ + 'Cannot update a component (`A`) while rendering a different component (`B`). ' + + 'To locate the bad setState() call inside `B`, ' + + 'follow the stack trace as described in https://react.dev/link/setstate-in-render\n' + + (gate('enableOwnerStacks') + ? '' + : ' in B (at **)\n' + ' in div (at **)\n') + + ' in Parent (at **)', + ]); // We error, but still update the state. expect(ref.textContent).toBe('1'); diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js index a651a477a8e76..d0d1c36e514a9 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js @@ -17,13 +17,14 @@ let Scheduler; let assertLog; let TestComponent; let testComponentInstance; +let assertConsoleErrorDev; describe('ReactCompositeComponent-state', () => { beforeEach(() => { React = require('react'); ReactDOM = require('react-dom'); ReactDOMClient = require('react-dom/client'); - act = require('internal-test-utils').act; + ({act, assertConsoleErrorDev} = require('internal-test-utils')); Scheduler = require('scheduler'); const InternalTestUtils = require('internal-test-utils'); @@ -469,15 +470,15 @@ describe('ReactCompositeComponent-state', () => { root.render(); }); // Update - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Test.componentWillReceiveProps(): Assigning directly to ' + "this.state is deprecated (except inside a component's constructor). " + - 'Use setState instead.', - ); + 'Use setState instead.\n' + + ' in Test (at **)', + ]); assertLog([ 'render -- step: 1, extra: true', @@ -518,15 +519,15 @@ describe('ReactCompositeComponent-state', () => { // Mount const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ 'Test.componentWillMount(): Assigning directly to ' + "this.state is deprecated (except inside a component's constructor). " + - 'Use setState instead.', - ); + 'Use setState instead.\n' + + ' in Test (at **)', + ]); assertLog([ 'render -- step: 3, extra: false', @@ -566,13 +567,16 @@ describe('ReactCompositeComponent-state', () => { }); expect(el.textContent).toBe('A'); - expect(() => { - ReactDOM.flushSync(() => { - root.render(); - }); - }).toErrorDev( - "Can't perform a React state update on a component that hasn't mounted yet", - ); + ReactDOM.flushSync(() => { + root.render(); + }); + assertConsoleErrorDev([ + "Can't perform a React state update on a component that hasn't mounted yet. " + + 'This indicates that you have a side-effect in your render function that ' + + 'asynchronously later calls tries to update the component. ' + + 'Move this work to useEffect instead.\n' + + ' in B (at **)', + ]); }); // @gate !disableLegacyMode diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index c6c90d1cf15f2..92571ad69e10c 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -14,7 +14,7 @@ let ReactDOM; let findDOMNode; let ReactDOMClient; let ReactDOMServer; - +let assertConsoleErrorDev; let act; describe('ReactDOM', () => { @@ -28,7 +28,7 @@ describe('ReactDOM', () => { ReactDOM.__DOM_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE .findDOMNode; - act = require('internal-test-utils').act; + ({act, assertConsoleErrorDev} = require('internal-test-utils')); }); it('should bubble onSubmit', async () => { @@ -188,15 +188,14 @@ describe('ReactDOM', () => { const myDiv = document.createElement('div'); await expect(async () => { - await expect(async () => { - await act(() => { - ReactDOM.render(, myDiv, 'no'); - }); - }).rejects.toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: no', - ); - }).toErrorDev( + await act(() => { + ReactDOM.render(, myDiv, 'no'); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', + ); + assertConsoleErrorDev( [ 'Expected the last optional `callback` argument to be a function. Instead received: no.', 'Expected the last optional `callback` argument to be a function. Instead received: no.', @@ -205,15 +204,14 @@ describe('ReactDOM', () => { ); await expect(async () => { - await expect(async () => { - await act(() => { - ReactDOM.render(, myDiv, {foo: 'bar'}); - }); - }).rejects.toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', - ); - }).toErrorDev( + await act(() => { + ReactDOM.render(, myDiv, {foo: 'bar'}); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + assertConsoleErrorDev( [ "Expected the last optional `callback` argument to be a function. Instead received: { foo: 'bar' }", "Expected the last optional `callback` argument to be a function. Instead received: { foo: 'bar' }.", @@ -222,15 +220,14 @@ describe('ReactDOM', () => { ); await expect(async () => { - await expect(async () => { - await act(() => { - ReactDOM.render(, myDiv, new Foo()); - }); - }).rejects.toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', - ); - }).toErrorDev( + await act(() => { + ReactDOM.render(, myDiv, new Foo()); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + assertConsoleErrorDev( [ 'Expected the last optional `callback` argument to be a function. Instead received: Foo { a: 1, b: 2 }.', 'Expected the last optional `callback` argument to be a function. Instead received: Foo { a: 1, b: 2 }.', @@ -257,15 +254,14 @@ describe('ReactDOM', () => { const myDiv = document.createElement('div'); ReactDOM.render(, myDiv); await expect(async () => { - await expect(async () => { - await act(() => { - ReactDOM.render(, myDiv, 'no'); - }); - }).rejects.toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: no', - ); - }).toErrorDev( + await act(() => { + ReactDOM.render(, myDiv, 'no'); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', + ); + assertConsoleErrorDev( [ 'Expected the last optional `callback` argument to be a function. Instead received: no.', 'Expected the last optional `callback` argument to be a function. Instead received: no.', @@ -275,15 +271,14 @@ describe('ReactDOM', () => { ReactDOM.render(, myDiv); // Re-mount await expect(async () => { - await expect(async () => { - await act(() => { - ReactDOM.render(, myDiv, {foo: 'bar'}); - }); - }).rejects.toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', - ); - }).toErrorDev( + await act(() => { + ReactDOM.render(, myDiv, {foo: 'bar'}); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + assertConsoleErrorDev( [ "Expected the last optional `callback` argument to be a function. Instead received: { foo: 'bar' }.", "Expected the last optional `callback` argument to be a function. Instead received: { foo: 'bar' }.", @@ -293,15 +288,14 @@ describe('ReactDOM', () => { ReactDOM.render(, myDiv); // Re-mount await expect(async () => { - await expect(async () => { - await act(() => { - ReactDOM.render(, myDiv, new Foo()); - }); - }).rejects.toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', - ); - }).toErrorDev( + await act(() => { + ReactDOM.render(, myDiv, new Foo()); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + assertConsoleErrorDev( [ 'Expected the last optional `callback` argument to be a function. Instead received: Foo { a: 1, b: 2 }.', 'Expected the last optional `callback` argument to be a function. Instead received: Foo { a: 1, b: 2 }.', @@ -544,11 +538,10 @@ describe('ReactDOM', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev([ + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ // ReactDOM(App > div > span) 'Invalid ARIA attribute `ariaTypo`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' + ' in span (at **)\n' + diff --git a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js index 9b7d9a30cbd53..cd1d055c09d1c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js @@ -13,12 +13,15 @@ describe('ReactDOM unknown attribute', () => { let React; let ReactDOMClient; let act; + let assertConsoleErrorDev; beforeEach(() => { jest.resetModules(); React = require('react'); ReactDOMClient = require('react-dom/client'); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; }); async function testUnknownAttributeRemoval(givenValue) { @@ -62,12 +65,13 @@ describe('ReactDOM unknown attribute', () => { }); it('changes values true, false to null, and also warns once', async () => { - await expect(() => testUnknownAttributeAssignment(true, null)).toErrorDev( + await testUnknownAttributeAssignment(true, null); + assertConsoleErrorDev([ 'Received `true` for a non-boolean attribute `unknown`.\n\n' + 'If you want to write it to the DOM, pass a string instead: ' + 'unknown="true" or unknown={value.toString()}.\n' + ' in div (at **)', - ); + ]); await testUnknownAttributeAssignment(false, null); }); @@ -92,11 +96,9 @@ describe('ReactDOM unknown attribute', () => { const el = document.createElement('div'); const root = ReactDOMClient.createRoot(el); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev([]); + await act(() => { + root.render(
); + }); expect(el.firstChild.getAttribute('inert')).toBe(true ? '' : null); }); @@ -105,15 +107,15 @@ describe('ReactDOM unknown attribute', () => { const el = document.createElement('div'); const root = ReactDOMClient.createRoot(el); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev([ + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ 'Received an empty string for a boolean attribute `inert`. ' + 'This will treat the attribute as if it were false. ' + 'Either pass `false` to silence this warning, or ' + - 'pass `true` if you used an empty string in earlier versions of React to indicate this attribute is true.', + 'pass `true` if you used an empty string in earlier versions of React to indicate this attribute is true.\n' + + ' in div (at **)', ]); expect(el.firstChild.getAttribute('inert')).toBe(true ? null : ''); @@ -136,11 +138,12 @@ describe('ReactDOM unknown attribute', () => { }); it('coerces NaN to strings and warns', async () => { - await expect(() => testUnknownAttributeAssignment(NaN, 'NaN')).toErrorDev( + await testUnknownAttributeAssignment(NaN, 'NaN'); + assertConsoleErrorDev([ 'Received NaN for the `unknown` attribute. ' + 'If this is expected, cast the value to a string.\n' + ' in div (at **)', - ); + ]); }); it('coerces objects to strings and warns', async () => { @@ -167,52 +170,52 @@ describe('ReactDOM unknown attribute', () => { } const test = () => testUnknownAttributeAssignment(new TemporalLike(), null); - await expect(() => - expect(test).rejects.toThrowError(new TypeError('prod message')), - ).toErrorDev( + + await expect(test).rejects.toThrowError(new TypeError('prod message')); + assertConsoleErrorDev([ 'The provided `unknown` attribute is an unsupported type TemporalLike.' + - ' This value must be coerced to a string before using it here.', - ); + ' This value must be coerced to a string before using it here.\n' + + ' in div (at **)', + ]); }); it('removes symbols and warns', async () => { - await expect(() => testUnknownAttributeRemoval(Symbol('foo'))).toErrorDev( + await testUnknownAttributeRemoval(Symbol('foo')); + assertConsoleErrorDev([ 'Invalid value for prop `unknown` on
tag. Either remove it ' + 'from the element, or pass a string or number value to keep it ' + 'in the DOM. For details, see https://react.dev/link/attribute-behavior \n' + ' in div (at **)', - ); + ]); }); it('removes functions and warns', async () => { - await expect(() => - testUnknownAttributeRemoval(function someFunction() {}), - ).toErrorDev( + await testUnknownAttributeRemoval(function someFunction() {}); + assertConsoleErrorDev([ 'Invalid value for prop `unknown` on
tag. Either remove ' + 'it from the element, or pass a string or number value to ' + 'keep it in the DOM. For details, see ' + 'https://react.dev/link/attribute-behavior \n' + ' in div (at **)', - ); + ]); }); it('allows camelCase unknown attributes and warns', async () => { const el = document.createElement('div'); - await expect(async () => { - const root = ReactDOMClient.createRoot(el); + const root = ReactDOMClient.createRoot(el); - await act(() => { - root.render(
); - }); - }).toErrorDev( + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ 'React does not recognize the `helloWorld` prop on a DOM element. ' + 'If you intentionally want it to appear in the DOM as a custom ' + 'attribute, spell it as lowercase `helloworld` instead. ' + 'If you accidentally passed it from a parent component, remove ' + 'it from the DOM element.\n' + ' in div (at **)', - ); + ]); expect(el.firstChild.getAttribute('helloworld')).toBe('something'); }); diff --git a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js index e0022dd99004b..a617770cf5f3b 100644 --- a/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js +++ b/packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js @@ -12,6 +12,7 @@ let React; let ReactNoop; let waitForAll; +let assertConsoleErrorDev; describe('ReactDeprecationWarnings', () => { beforeEach(() => { @@ -20,6 +21,7 @@ describe('ReactDeprecationWarnings', () => { ReactNoop = require('react-noop-renderer'); const InternalTestUtils = require('internal-test-utils'); waitForAll = InternalTestUtils.waitForAll; + assertConsoleErrorDev = InternalTestUtils.assertConsoleErrorDev; }); // @gate !disableDefaultPropsExceptForClasses || !__DEV__ @@ -33,11 +35,13 @@ describe('ReactDeprecationWarnings', () => { }; ReactNoop.render(); - await expect(async () => await waitForAll([])).toErrorDev( + await waitForAll([]); + assertConsoleErrorDev([ 'FunctionalComponent: Support for defaultProps ' + 'will be removed from function components in a future major ' + - 'release. Use JavaScript default parameters instead.', - ); + 'release. Use JavaScript default parameters instead.\n' + + ' in FunctionalComponent (at **)', + ]); }); // @gate !disableDefaultPropsExceptForClasses || !__DEV__ @@ -55,10 +59,12 @@ describe('ReactDeprecationWarnings', () => {
, ); - await expect(async () => await waitForAll([])).toErrorDev( + await waitForAll([]); + assertConsoleErrorDev([ 'FunctionalComponent: Support for defaultProps ' + 'will be removed from memo components in a future major ' + - 'release. Use JavaScript default parameters instead.', - ); + 'release. Use JavaScript default parameters instead.\n' + + ' in div (at **)', + ]); }); }); diff --git a/packages/react-dom/src/__tests__/findDOMNodeFB-test.js b/packages/react-dom/src/__tests__/findDOMNodeFB-test.js index 417ae0c40e7f8..28212c0d5cf82 100644 --- a/packages/react-dom/src/__tests__/findDOMNodeFB-test.js +++ b/packages/react-dom/src/__tests__/findDOMNodeFB-test.js @@ -12,6 +12,8 @@ const React = require('react'); const ReactDOM = require('react-dom'); const StrictMode = React.StrictMode; +const assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; describe('findDOMNode', () => { // @gate www && classic @@ -128,8 +130,8 @@ describe('findDOMNode', () => { container, ); - let match; - expect(() => (match = ReactDOM.findDOMNode(parent))).toErrorDev([ + const match = ReactDOM.findDOMNode(parent); + assertConsoleErrorDev([ 'findDOMNode is deprecated in StrictMode. ' + 'findDOMNode was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' + 'Instead, add a ref directly to the element you want to reference. ' + @@ -161,8 +163,8 @@ describe('findDOMNode', () => { container, ); - let match; - expect(() => (match = ReactDOM.findDOMNode(parent))).toErrorDev([ + const match = ReactDOM.findDOMNode(parent); + assertConsoleErrorDev([ 'findDOMNode is deprecated in StrictMode. ' + 'findDOMNode was passed an instance of IsInStrictMode which is inside StrictMode. ' + 'Instead, add a ref directly to the element you want to reference. ' + diff --git a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js index 76dea3e732632..67d29fd33efdc 100644 --- a/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js +++ b/packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js @@ -87,7 +87,7 @@ module.exports = function (initModules) { console.error.mockClear(); } else { // TODO: Rewrite tests that use this helper to enumerate expected errors. - // This will enable the helper to use the .toErrorDev() matcher instead of spying. + // This will enable the helper to use the assertConsoleErrorDev instead of spying. spyOnDev(console, 'error').mockImplementation(() => {}); } From d8a08f8e39972978cd0666f277409a1657083bb5 Mon Sep 17 00:00:00 2001 From: Ricky Date: Thu, 2 Jan 2025 15:28:15 -0500 Subject: [PATCH 5/9] [assert helpers] ReactDOMComponent-test (#31898) Splitting out ReactDOMComponent to it's own PR because it's huge. --- .../src/__tests__/ReactDOMComponent-test.js | 2020 +++++++++-------- 1 file changed, 1117 insertions(+), 903 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index ce71a6334ee64..65f82dcd3690a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -19,6 +19,7 @@ describe('ReactDOMComponent', () => { let act; let assertLog; let Scheduler; + let assertConsoleErrorDev; beforeEach(() => { jest.resetModules(); @@ -28,6 +29,8 @@ describe('ReactDOMComponent', () => { ReactDOMServer = require('react-dom/server'); Scheduler = require('scheduler'); act = require('internal-test-utils').act; + assertConsoleErrorDev = + require('internal-test-utils').assertConsoleErrorDev; assertLog = require('internal-test-utils').assertLog; }); @@ -189,73 +192,72 @@ describe('ReactDOMComponent', () => { it('should warn for unknown prop', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(
{}} />); - }); - }).toErrorDev( + await act(() => { + root.render(
{}} />); + }); + assertConsoleErrorDev([ 'Invalid value for prop `foo` on
tag. Either remove it ' + 'from the element, or pass a string or number value to keep ' + 'it in the DOM. For details, see https://react.dev/link/attribute-behavior ' + '\n in div (at **)', - ); + ]); }); it('should group multiple unknown prop warnings together', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(
{}} baz={() => {}} />); - }); - }).toErrorDev( + await act(() => { + root.render(
{}} baz={() => {}} />); + }); + assertConsoleErrorDev([ 'Invalid values for props `foo`, `baz` on
tag. Either remove ' + 'them from the element, or pass a string or number value to keep ' + 'them in the DOM. For details, see https://react.dev/link/attribute-behavior ' + '\n in div (at **)', - ); + ]); }); it('should warn for onDblClick prop', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(
{}} />); - }); - }).toErrorDev( - 'Invalid event handler property `onDblClick`. Did you mean `onDoubleClick`?\n in div (at **)', - ); + await act(() => { + root.render(
{}} />); + }); + assertConsoleErrorDev([ + 'Invalid event handler property `onDblClick`. Did you mean `onDoubleClick`?\n' + + ' in div (at **)', + ]); }); it('should warn for unknown string event handlers', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)', - ); + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Unknown event handler property `onUnknown`. It will be ignored.\n' + + ' in div (at **)', + ]); expect(container.firstChild.hasAttribute('onUnknown')).toBe(false); expect(container.firstChild.onUnknown).toBe(undefined); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)', - ); + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Unknown event handler property `onunknown`. It will be ignored.\n' + + ' in div (at **)', + ]); expect(container.firstChild.hasAttribute('onunknown')).toBe(false); expect(container.firstChild.onunknown).toBe(undefined); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)', - ); + + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Unknown event handler property `on-unknown`. It will be ignored.\n' + + ' in div (at **)', + ]); expect(container.firstChild.hasAttribute('on-unknown')).toBe(false); expect(container.firstChild['on-unknown']).toBe(undefined); }); @@ -263,31 +265,31 @@ describe('ReactDOMComponent', () => { it('should warn for unknown function event handlers', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Unknown event handler property `onUnknown`. It will be ignored.\n in div (at **)', - ); + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Unknown event handler property `onUnknown`. It will be ignored.\n' + + ' in div (at **)', + ]); expect(container.firstChild.hasAttribute('onUnknown')).toBe(false); expect(container.firstChild.onUnknown).toBe(undefined); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Unknown event handler property `onunknown`. It will be ignored.\n in div (at **)', - ); + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Unknown event handler property `onunknown`. It will be ignored.\n' + + ' in div (at **)', + ]); expect(container.firstChild.hasAttribute('onunknown')).toBe(false); expect(container.firstChild.onunknown).toBe(undefined); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Unknown event handler property `on-unknown`. It will be ignored.\n in div (at **)', - ); + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Unknown event handler property `on-unknown`. It will be ignored.\n' + + ' in div (at **)', + ]); expect(container.firstChild.hasAttribute('on-unknown')).toBe(false); expect(container.firstChild['on-unknown']).toBe(undefined); }); @@ -295,13 +297,13 @@ describe('ReactDOMComponent', () => { it('should warn for badly cased React attributes', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(
); - }); - }).toErrorDev( - 'Invalid DOM property `CHILDREN`. Did you mean `children`?\n in div (at **)', - ); + await act(() => { + root.render(
); + }); + assertConsoleErrorDev([ + 'Invalid DOM property `CHILDREN`. Did you mean `children`?\n' + + ' in div (at **)', + ]); expect(container.firstChild.getAttribute('CHILDREN')).toBe('5'); }); @@ -323,14 +325,13 @@ describe('ReactDOMComponent', () => { const style = {fontSize: NaN}; const div = document.createElement('div'); const root = ReactDOMClient.createRoot(div); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - '`NaN` is an invalid value for the `fontSize` css style property.' + - '\n in span (at **)', - ); + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + '`NaN` is an invalid value for the `fontSize` css style property.\n' + + ' in span (at **)', + ]); await act(() => { root.render(); }); @@ -350,15 +351,18 @@ describe('ReactDOMComponent', () => { const style = {fontSize: new TemporalLike()}; const root = ReactDOMClient.createRoot(document.createElement('div')); await expect(async () => { - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'The provided `fontSize` CSS property is an unsupported type TemporalLike.' + - ' This value must be coerced to a string before using it here.', - ); + await act(() => { + root.render(); + }); }).rejects.toThrowError(new TypeError('prod message')); + assertConsoleErrorDev([ + 'The provided `fontSize` CSS property is an unsupported type TemporalLike.' + + ' This value must be coerced to a string before using it here.\n' + + ' in span (at **)', + 'The provided `fontSize` CSS property is an unsupported type TemporalLike.' + + ' This value must be coerced to a string before using it here.\n' + + ' in span (at **)', + ]); }); it('should update styles if initially null', async () => { @@ -590,16 +594,16 @@ describe('ReactDOMComponent', () => { it('should not add an empty src attribute', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'An empty string ("") was passed to the src attribute. ' + 'This may cause the browser to download the whole page again over the network. ' + 'To fix this, either do not render the element at all ' + - 'or pass null to src instead of an empty string.', - ); + 'or pass null to src instead of an empty string.\n' + + ' in img (at **)', + ]); const node = container.firstChild; expect(node.hasAttribute('src')).toBe(false); @@ -608,31 +612,31 @@ describe('ReactDOMComponent', () => { }); expect(node.hasAttribute('src')).toBe(true); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'An empty string ("") was passed to the src attribute. ' + 'This may cause the browser to download the whole page again over the network. ' + 'To fix this, either do not render the element at all ' + - 'or pass null to src instead of an empty string.', - ); + 'or pass null to src instead of an empty string.\n' + + ' in img (at **)', + ]); expect(node.hasAttribute('src')).toBe(false); }); it('should not add an empty href attribute', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'An empty string ("") was passed to the href attribute. ' + 'To fix this, either do not render the element at all ' + - 'or pass null to href instead of an empty string.', - ); + 'or pass null to href instead of an empty string.\n' + + ' in link (at **)', + ]); const node = container.firstChild; expect(node.hasAttribute('href')).toBe(false); @@ -641,15 +645,15 @@ describe('ReactDOMComponent', () => { }); expect(node.hasAttribute('href')).toBe(true); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'An empty string ("") was passed to the href attribute. ' + 'To fix this, either do not render the element at all ' + - 'or pass null to href instead of an empty string.', - ); + 'or pass null to href instead of an empty string.\n' + + ' in link (at **)', + ]); expect(node.hasAttribute('href')).toBe(false); }); @@ -871,204 +875,235 @@ describe('ReactDOMComponent', () => { }); it('should reject attribute key injection attack on markup for regular DOM (SSR)', () => { - expect(() => { - for (let i = 0; i < 3; i++) { - const element1 = React.createElement( - 'div', - {'blah" onclick="beevil" noise="hi': 'selected'}, - null, - ); - const element2 = React.createElement( - 'div', - {'>
': 'selected'}, - null, - ); - const result1 = ReactDOMServer.renderToString(element1); - const result2 = ReactDOMServer.renderToString(element2); - expect(result1.toLowerCase()).not.toContain('onclick'); - expect(result2.toLowerCase()).not.toContain('script'); - } - }).toErrorDev([ - 'Invalid attribute name: `blah" onclick="beevil" noise="hi`', - 'Invalid attribute name: `>
`', + for (let i = 0; i < 3; i++) { + const element1 = React.createElement( + 'div', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ); + const element2 = React.createElement( + 'div', + {'>
': 'selected'}, + null, + ); + const result1 = ReactDOMServer.renderToString(element1); + const result2 = ReactDOMServer.renderToString(element2); + expect(result1.toLowerCase()).not.toContain('onclick'); + expect(result2.toLowerCase()).not.toContain('script'); + } + assertConsoleErrorDev([ + 'Invalid attribute name: `blah" onclick="beevil" noise="hi`\n' + + ' in div (at **)', + 'Invalid attribute name: `>
`\n' + + ' in div (at **)', ]); }); it('should reject attribute key injection attack on markup for custom elements (SSR)', () => { - expect(() => { - for (let i = 0; i < 3; i++) { - const element1 = React.createElement( - 'x-foo-component', - {'blah" onclick="beevil" noise="hi': 'selected'}, - null, - ); - const element2 = React.createElement( - 'x-foo-component', - {'>': 'selected'}, - null, - ); - const result1 = ReactDOMServer.renderToString(element1); - const result2 = ReactDOMServer.renderToString(element2); - expect(result1.toLowerCase()).not.toContain('onclick'); - expect(result2.toLowerCase()).not.toContain('script'); - } - }).toErrorDev([ - 'Invalid attribute name: `blah" onclick="beevil" noise="hi`', - 'Invalid attribute name: `>`', + for (let i = 0; i < 3; i++) { + const element1 = React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ); + const element2 = React.createElement( + 'x-foo-component', + {'>': 'selected'}, + null, + ); + const result1 = ReactDOMServer.renderToString(element1); + const result2 = ReactDOMServer.renderToString(element2); + expect(result1.toLowerCase()).not.toContain('onclick'); + expect(result2.toLowerCase()).not.toContain('script'); + } + assertConsoleErrorDev([ + 'Invalid attribute name: `blah" onclick="beevil" noise="hi`\n' + + ' in x-foo-component (at **)', + 'Invalid attribute name: `>`\n' + + ' in x-foo-component (at **)', ]); }); it('should reject attribute key injection attack on mount for regular DOM', async () => { - await expect(async () => { - for (let i = 0; i < 3; i++) { - const container = document.createElement('div'); - let root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - React.createElement( - 'div', - {'blah" onclick="beevil" noise="hi': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); - await act(() => { - root.unmount(); - }); - root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - React.createElement( - 'div', - {'>
': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); + for (let i = 0; i < 3; i++) { + const container = document.createElement('div'); + let root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + React.createElement( + 'div', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + ); + }); + + expect(container.firstChild.attributes.length).toBe(0); + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `blah" onclick="beevil" noise="hi`\n' + + ' in div (at **)', + ]); } - }).toErrorDev([ - 'Invalid attribute name: `blah" onclick="beevil" noise="hi`', - 'Invalid attribute name: `>
`', - ]); + await act(() => { + root.unmount(); + }); + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + React.createElement( + 'div', + {'>
': 'selected'}, + null, + ), + ); + }); + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `>
`\n' + + ' in div (at **)', + ]); + } + + expect(container.firstChild.attributes.length).toBe(0); + } }); it('should reject attribute key injection attack on mount for custom elements', async () => { - await expect(async () => { - for (let i = 0; i < 3; i++) { - const container = document.createElement('div'); - let root = ReactDOMClient.createRoot(container); - - await act(() => { - root.render( - React.createElement( - 'x-foo-component', - {'blah" onclick="beevil" noise="hi': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); - await act(() => { - root.unmount(); - }); - root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - React.createElement( - 'x-foo-component', - {'>': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); + for (let i = 0; i < 3; i++) { + const container = document.createElement('div'); + let root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render( + React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + ); + }); + + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `blah" onclick="beevil" noise="hi`\n' + + ' in x-foo-component (at **)', + ]); } - }).toErrorDev([ - 'Invalid attribute name: `blah" onclick="beevil" noise="hi`', - 'Invalid attribute name: `>`', - ]); + expect(container.firstChild.attributes.length).toBe(0); + await act(() => { + root.unmount(); + }); + + root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + React.createElement( + 'x-foo-component', + {'>': 'selected'}, + null, + ), + ); + }); + + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `>`\n' + + ' in x-foo-component (at **)', + ]); + } + expect(container.firstChild.attributes.length).toBe(0); + } }); it('should reject attribute key injection attack on update for regular DOM', async () => { - await expect(async () => { - for (let i = 0; i < 3; i++) { - const container = document.createElement('div'); - const beforeUpdate = React.createElement('div', {}, null); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(beforeUpdate); - }); - await act(() => { - root.render( - React.createElement( - 'div', - {'blah" onclick="beevil" noise="hi': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); - await act(() => { - root.render( - React.createElement( - 'div', - {'>
': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); + for (let i = 0; i < 3; i++) { + const container = document.createElement('div'); + const beforeUpdate = React.createElement('div', {}, null); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(beforeUpdate); + }); + await act(() => { + root.render( + React.createElement( + 'div', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + ); + }); + + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `blah" onclick="beevil" noise="hi`\n' + + ' in div (at **)', + ]); } - }).toErrorDev([ - 'Invalid attribute name: `blah" onclick="beevil" noise="hi`', - 'Invalid attribute name: `>
`', - ]); + expect(container.firstChild.attributes.length).toBe(0); + await act(() => { + root.render( + React.createElement( + 'div', + {'>
': 'selected'}, + null, + ), + ); + }); + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `>
`\n' + + ' in div (at **)', + ]); + } + + expect(container.firstChild.attributes.length).toBe(0); + } }); it('should reject attribute key injection attack on update for custom elements', async () => { - await expect(async () => { - for (let i = 0; i < 3; i++) { - const container = document.createElement('div'); - const beforeUpdate = React.createElement('x-foo-component', {}, null); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(beforeUpdate); - }); - await act(() => { - root.render( - React.createElement( - 'x-foo-component', - {'blah" onclick="beevil" noise="hi': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); - await act(() => { - root.render( - React.createElement( - 'x-foo-component', - {'>': 'selected'}, - null, - ), - ); - }); - - expect(container.firstChild.attributes.length).toBe(0); + for (let i = 0; i < 3; i++) { + const container = document.createElement('div'); + const beforeUpdate = React.createElement('x-foo-component', {}, null); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render(beforeUpdate); + }); + await act(() => { + root.render( + React.createElement( + 'x-foo-component', + {'blah" onclick="beevil" noise="hi': 'selected'}, + null, + ), + ); + }); + + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `blah" onclick="beevil" noise="hi`\n' + + ' in x-foo-component (at **)', + ]); } - }).toErrorDev([ - 'Invalid attribute name: `blah" onclick="beevil" noise="hi`', - 'Invalid attribute name: `>`', - ]); + expect(container.firstChild.attributes.length).toBe(0); + await act(() => { + root.render( + React.createElement( + 'x-foo-component', + {'>': 'selected'}, + null, + ), + ); + }); + + if (i === 0) { + assertConsoleErrorDev([ + 'Invalid attribute name: `>`\n' + + ' in x-foo-component (at **)', + ]); + } + expect(container.firstChild.attributes.length).toBe(0); + } }); it('should update arbitrary attributes for tags containing dashes', async () => { @@ -1382,36 +1417,38 @@ describe('ReactDOMComponent', () => { }); expect(nodeValueSetter).toHaveBeenCalledTimes(1); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'A component is changing a controlled input to be uncontrolled. This is likely caused by ' + 'the value changing from a defined to undefined, which should not happen. Decide between ' + - 'using a controlled or uncontrolled input element for the lifetime of the component.', - ); + 'using a controlled or uncontrolled input element for the lifetime of the component. ' + + 'More info: https://react.dev/link/controlled-components\n' + + ' in input (at **)', + ]); expect(nodeValueSetter).toHaveBeenCalledTimes(1); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - 'value` prop on `input` should not be null. Consider using an empty string to clear the ' + - 'component or `undefined` for uncontrolled components.', - ); + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ + '`value` prop on `input` should not be null. Consider using an empty string to clear the ' + + 'component or `undefined` for uncontrolled components.\n' + + ' in input (at **)', + ]); expect(nodeValueSetter).toHaveBeenCalledTimes(1); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( + await act(() => { + root.render(); + }); + assertConsoleErrorDev([ 'A component is changing an uncontrolled input to be controlled. This is likely caused by ' + 'the value changing from undefined to a defined value, which should not happen. Decide between ' + - 'using a controlled or uncontrolled input element for the lifetime of the component.', - ); + 'using a controlled or uncontrolled input element for the lifetime of the component. ' + + 'More info: https://react.dev/link/controlled-components\n' + + ' in input (at **)', + ]); expect(nodeValueSetter).toHaveBeenCalledTimes(2); await act(() => { @@ -1462,14 +1499,14 @@ describe('ReactDOMComponent', () => { it('should warn about non-string "is" attribute', async () => { const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(