Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 91 additions & 20 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,69 @@ function cancelAllViewTransitionAnimations(scope: Element) {
// either cached the font or preloaded it earlier.
const SUSPENSEY_FONT_TIMEOUT = 500;

function customizeViewTransitionError(error: Object): mixed {
if (typeof error === 'object' && error !== null) {
switch (error.name) {
case 'TimeoutError': {
// We assume that the only reason a Timeout can happen is because the Navigation
// promise. We expect any other work to either be fast or have a timeout (fonts).
if (__DEV__) {
// eslint-disable-next-line react-internal/prod-error-codes
return new Error(
'A ViewTransition timed out because a Navigation stalled. ' +
'This can happen if a Navigation is blocked on React itself. ' +
"Such as if it's resolved inside useEffect. " +
'This can be solved by moving the resolution to useLayoutEffect.',
Comment on lines +1589 to +1591

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not following this one. Any chance we can manually trigger this scenario in the fixture?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this switches to useEffect instead of useLayoutEffect it should trigger it.

https://github.com/facebook/react/blob/main/fixtures/view-transition/src/components/App.js#L84-L86

Since the navigation should be blocked by the commit and the navigation blocks the commit from preceding the timing of unblocking the navigation needs to be in useLayoutEffect.

{cause: error},
);
}
break;
}
case 'AbortError': {
if (__DEV__) {
// eslint-disable-next-line react-internal/prod-error-codes
return new Error(
'A ViewTransition was aborted early. This might be because you have ' +
'other View Transition libraries on the page and only one can run at ' +
"a time. To avoid this, use only React's built-in <ViewTransition> " +
'to coordinate.',
{cause: error},
);
}
break;
}
case 'InvalidStateError': {
if (
error.message ===
'View transition was skipped because document visibility state is hidden.' ||
error.message ===
'Skipping view transition because document visibility state has become hidden.' ||
error.message ===
'Skipping view transition because viewport size changed.'
) {
// Skip logging this. This is not considered an error.
return null;
}
if (__DEV__) {
if (
error.message === 'Transition was aborted because of invalid state'
) {
// Chrome doesn't include the reason in the message but logs it in the console..
// Redirect the user to look there.
// eslint-disable-next-line react-internal/prod-error-codes
return new Error(
'A ViewTransition could not start. See the console for more details.',
{cause: error},
);
}
}
break;
}
}
}
return error;
}

export function startViewTransition(
rootContainer: Container,
transitionTypes: null | TransitionTypes,
Expand All @@ -1584,6 +1647,7 @@ export function startViewTransition(
afterMutationCallback: () => void,
spawnedWorkCallback: () => void,
passiveCallback: () => mixed,
errorCallback: mixed => void,
): boolean {
const ownerDocument: Document =
rootContainer.nodeType === DOCUMENT_NODE
Expand Down Expand Up @@ -1641,24 +1705,19 @@ export function startViewTransition(
});
// $FlowFixMe[prop-missing]
ownerDocument.__reactViewTransition = transition;
if (__DEV__) {
transition.ready.then(undefined, (reason: mixed) => {
if (
typeof reason === 'object' &&
reason !== null &&
reason.name === 'TimeoutError'
) {
console.error(
'A ViewTransition timed out because a Navigation stalled. ' +
'This can happen if a Navigation is blocked on React itself. ' +
"Such as if it's resolved inside useEffect. " +
'This can be solved by moving the resolution to useLayoutEffect.',
);
const handleError = (error: mixed) => {
try {
error = customizeViewTransitionError(error);
if (error !== null) {
errorCallback(error);
}
});
}
transition.ready.then(spawnedWorkCallback, spawnedWorkCallback);
transition.finished.then(() => {
} finally {
// Continue the reset of the work.
spawnedWorkCallback();
}
};
transition.ready.then(spawnedWorkCallback, handleError);
transition.finished.finally(() => {
cancelAllViewTransitionAnimations((ownerDocument.documentElement: any));
// $FlowFixMe[prop-missing]
if (ownerDocument.__reactViewTransition === transition) {
Expand Down Expand Up @@ -1802,6 +1861,7 @@ export function startGestureTransition(
transitionTypes: null | TransitionTypes,
mutationCallback: () => void,
animateCallback: () => void,
errorCallback: mixed => void,
): null | RunningGestureTransition {
const ownerDocument: Document =
rootContainer.nodeType === DOCUMENT_NODE
Expand All @@ -1815,7 +1875,7 @@ export function startGestureTransition(
});
// $FlowFixMe[prop-missing]
ownerDocument.__reactViewTransition = transition;
const readyCallback = (x: any) => {
const readyCallback = () => {
const documentElement: Element = (ownerDocument.documentElement: any);
// Loop through all View Transition Animations.
const animations = documentElement.getAnimations({subtree: true});
Expand Down Expand Up @@ -1935,8 +1995,19 @@ export function startGestureTransition(
navigator.userAgent.indexOf('Chrome') !== -1
? () => requestAnimationFrame(readyCallback)
: readyCallback;
transition.ready.then(readyForAnimations, readyCallback);
transition.finished.then(() => {
const handleError = (error: mixed) => {
try {
error = customizeViewTransitionError(error);
if (error !== null) {
errorCallback(error);
}
} finally {
// Continue the reset of the work.
readyCallback();
}
};
transition.ready.then(readyForAnimations, handleError);
transition.finished.finally(() => {
cancelAllViewTransitionAnimations((ownerDocument.documentElement: any));
// $FlowFixMe[prop-missing]
if (ownerDocument.__reactViewTransition === transition) {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native-renderer/src/ReactFiberConfigNative.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ export function startViewTransition(
afterMutationCallback: () => void,
spawnedWorkCallback: () => void,
passiveCallback: () => mixed,
errorCallback: mixed => void,
): boolean {
return false;
}
Expand All @@ -633,6 +634,7 @@ export function startGestureTransition(
transitionTypes: null | TransitionTypes,
mutationCallback: () => void,
animateCallback: () => void,
errorCallback: mixed => void,
): RunningGestureTransition {
mutationCallback();
animateCallback();
Expand Down
2 changes: 2 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
afterMutationCallback: () => void,
layoutCallback: () => void,
passiveCallback: () => mixed,
errorCallback: mixed => void,
): boolean {
return false;
},
Expand All @@ -821,6 +822,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
transitionTypes: null | TransitionTypes,
mutationCallback: () => void,
animateCallback: () => void,
errorCallback: mixed => void,
): RunningGestureTransition {
mutationCallback();
animateCallback();
Expand Down
12 changes: 12 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -3511,6 +3511,7 @@ function commitRoot(
flushAfterMutationEffects,
flushSpawnedWork,
flushPassiveEffects,
reportViewTransitionError,
);
if (!startedViewTransition) {
// Flush synchronously.
Expand All @@ -3521,6 +3522,16 @@ function commitRoot(
}
}

function reportViewTransitionError(error: mixed) {
// Report errors that happens while preparing a View Transition.
if (pendingEffectsStatus === NO_PENDING_EFFECTS) {
return;
}
const root = pendingEffectsRoot;
const onRecoverableError = root.onRecoverableError;
onRecoverableError(error, makeErrorInfo(null));
}

function flushAfterMutationEffects(): void {
if (pendingEffectsStatus !== PENDING_AFTER_MUTATION_PHASE) {
return;
Expand Down Expand Up @@ -3911,6 +3922,7 @@ function commitGestureOnRoot(
pendingTransitionTypes,
flushGestureMutations,
flushGestureAnimations,
reportViewTransitionError,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to run this in the context of finishedWork or is that not a relevant Owner of a potential error?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finishedWork is just the HostRoot so it has no owner at all.

In general these errors has no owner at all. There are other warnings we can add earlier to prevent some of these errors from happening in the first place. For example, we should have a warning when two ViewTransition components are mounted with the same name. Even if they don't happen to be part of the current transition and those could have owners.

);
}

Expand Down
2 changes: 2 additions & 0 deletions packages/react-test-renderer/src/ReactFiberConfigTestHost.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ export function startViewTransition(
afterMutationCallback: () => void,
spawnedWorkCallback: () => void,
passiveCallback: () => mixed,
errorCallback: mixed => void,
): boolean {
return false;
}
Expand All @@ -431,6 +432,7 @@ export function startGestureTransition(
transitionTypes: null | TransitionTypes,
mutationCallback: () => void,
animateCallback: () => void,
errorCallback: mixed => void,
): RunningGestureTransition {
mutationCallback();
animateCallback();
Expand Down