-
Notifications
You must be signed in to change notification settings - Fork 49.6k
Warn for duplicate ViewTransition names #32752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -109,6 +109,7 @@ import { | |
ForceClientRender, | ||
DidCapture, | ||
AffectedParentLayout, | ||
ViewTransitionNamedStatic, | ||
} from './ReactFiberFlags'; | ||
import { | ||
commitStartTime, | ||
|
@@ -254,6 +255,10 @@ import { | |
pushMutationContext, | ||
popMutationContext, | ||
} from './ReactFiberMutationTracking'; | ||
import { | ||
trackNamedViewTransition, | ||
untrackNamedViewTransition, | ||
} from './ReactFiberDuplicateViewTransitions'; | ||
|
||
// Used during the commit phase to track the state of the Offscreen component stack. | ||
// Allows us to avoid traversing the return path to find the nearest Offscreen ancestor. | ||
|
@@ -738,6 +743,11 @@ function commitLayoutEffectOnFiber( | |
} | ||
case ViewTransitionComponent: { | ||
if (enableViewTransition) { | ||
if (__DEV__) { | ||
if (flags & ViewTransitionNamedStatic) { | ||
trackNamedViewTransition(finishedWork); | ||
} | ||
} | ||
recursivelyTraverseLayoutEffects( | ||
finishedRoot, | ||
finishedWork, | ||
|
@@ -1551,11 +1561,34 @@ function commitDeletionEffectsOnFiber( | |
} | ||
break; | ||
} | ||
case ViewTransitionComponent: { | ||
if (enableViewTransition) { | ||
if (__DEV__) { | ||
if (deletedFiber.flags & ViewTransitionNamedStatic) { | ||
untrackNamedViewTransition(deletedFiber); | ||
} | ||
} | ||
safelyDetachRef(deletedFiber, nearestMountedAncestor); | ||
recursivelyTraverseDeletionEffects( | ||
finishedRoot, | ||
nearestMountedAncestor, | ||
deletedFiber, | ||
); | ||
return; | ||
} | ||
// Fallthrough | ||
} | ||
case Fragment: { | ||
if (enableFragmentRefs) { | ||
if (!offscreenSubtreeWasHidden) { | ||
safelyDetachRef(deletedFiber, nearestMountedAncestor); | ||
} | ||
recursivelyTraverseDeletionEffects( | ||
finishedRoot, | ||
nearestMountedAncestor, | ||
deletedFiber, | ||
); | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can safe some bytes by falling through but it really only works for one case and it's easy to get wrong when copying it so I made this explicit. |
||
} | ||
// Fallthrough | ||
} | ||
|
@@ -2594,6 +2627,11 @@ export function disappearLayoutEffects(finishedWork: Fiber) { | |
} | ||
case ViewTransitionComponent: { | ||
if (enableViewTransition) { | ||
if (__DEV__) { | ||
if (finishedWork.flags & ViewTransitionNamedStatic) { | ||
untrackNamedViewTransition(finishedWork); | ||
} | ||
} | ||
safelyDetachRef(finishedWork, finishedWork.return); | ||
} | ||
recursivelyTraverseDisappearLayoutEffects(finishedWork); | ||
|
@@ -2803,6 +2841,11 @@ export function reappearLayoutEffects( | |
finishedWork, | ||
includeWorkInProgressEffects, | ||
); | ||
if (__DEV__) { | ||
if (flags & ViewTransitionNamedStatic) { | ||
trackNamedViewTransition(finishedWork); | ||
} | ||
} | ||
safelyAttachRef(finishedWork, finishedWork.return); | ||
break; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/** | ||
* Copyright (c) Meta Platforms, Inc. and affiliates. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
* | ||
* @flow | ||
*/ | ||
|
||
import type {Fiber} from './ReactInternalTypes'; | ||
import type {ViewTransitionProps} from './ReactFiberViewTransitionComponent'; | ||
import {runWithFiberInDEV} from './ReactCurrentFiber'; | ||
|
||
// Use in DEV to track mounted named ViewTransitions. This is used to warn for | ||
// duplicate names. This should technically be tracked per Document because you could | ||
// have two different documents that can have separate namespaces, but to keep things | ||
// simple we just use a global Map. Technically it should also include any manually | ||
// assigned view-transition-name outside React too. | ||
const mountedNamedViewTransitions: Map<string, Fiber> = __DEV__ | ||
? new Map() | ||
: (null: any); | ||
const didWarnAboutName: {[string]: boolean} = __DEV__ ? {} : (null: any); | ||
|
||
export function trackNamedViewTransition(fiber: Fiber): void { | ||
if (__DEV__) { | ||
const name = (fiber.memoizedProps: ViewTransitionProps).name; | ||
if (name != null && name !== 'auto') { | ||
const existing = mountedNamedViewTransitions.get(name); | ||
if (existing !== undefined) { | ||
if (existing !== fiber && existing !== fiber.alternate) { | ||
if (!didWarnAboutName[name]) { | ||
didWarnAboutName[name] = true; | ||
const stringifiedName = JSON.stringify(name); | ||
runWithFiberInDEV(fiber, () => { | ||
console.error( | ||
'There are two <ViewTransition name=%s> components with the same name mounted ' + | ||
'at the same time. This is not supported and will cause View Transitions ' + | ||
'to error. Try to use a more unique name e.g. by using a namespace prefix ' + | ||
'and adding the id of an item to the name.', | ||
stringifiedName, | ||
); | ||
}); | ||
runWithFiberInDEV(existing, () => { | ||
console.error( | ||
'The existing <ViewTransition name=%s> duplicate has this stack trace.', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how to phrase this so it's not to repetitive when shown together but also somewhat makes sense in isolation if they're show independently. This log entry has the other Fiber's owner/async stack trace. I think we have this pattern of two logs somewhere else. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, you're logging the other component stack. Might be weird to see this in places where the component stack is less visible/collapsed? Could we add the owner component name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
stringifiedName, | ||
); | ||
}); | ||
} | ||
} | ||
} else { | ||
mountedNamedViewTransitions.set(name, fiber); | ||
} | ||
} | ||
} | ||
} | ||
|
||
export function untrackNamedViewTransition(fiber: Fiber): void { | ||
if (__DEV__) { | ||
const name = (fiber.memoizedProps: ViewTransitionProps).name; | ||
if (name != null && name !== 'auto') { | ||
const existing = mountedNamedViewTransitions.get(name); | ||
if ( | ||
existing !== undefined && | ||
(existing === fiber || existing === fiber.alternate) | ||
) { | ||
mountedNamedViewTransitions.delete(name); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also noticed that we weren't detaching ViewTransition refs when unmounting. Only hiding.