Skip to content
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

DevTools: Revert force deep re-mount when Fast Refresh detected #21539

Merged
merged 1 commit into from
May 20, 2021
Merged
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
27 changes: 4 additions & 23 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -829,12 +829,6 @@ export function attach(
throw Error('Cannot modify filter preferences while profiling');
}

unmountAndRemountAllRoots(() => {
applyComponentFilters(componentFilters);
});
}

function unmountAndRemountAllRoots(callback?: Function) {
// Recursively unmount all roots.
hook.getFiberRoots(rendererID).forEach(root => {
currentRootID = getOrGenerateFiberID(root.current);
Expand All @@ -846,9 +840,7 @@ export function attach(
currentRootID = -1;
});

if (typeof callback === 'function') {
callback();
}
applyComponentFilters(componentFilters);

// Reset pseudo counters so that new path selections will be persisted.
rootDisplayNameCounter.clear();
Expand Down Expand Up @@ -1797,20 +1789,6 @@ export function attach(
);
}

const unsafeID = getFiberIDUnsafe(fiber);
if (fiber._debugNeedsRemount) {
if (unsafeID === null) {
// This inidicates a case we can't recover from:
// Fast Refresh has force remounted a component in a way that we don't have an id for.
// We could throw but that's a bad user experience.
// Or we could ignore the unmount but then Store might end up with a duplicate node.
// So a fallback is to completely reset the Store.
// This is costly but since Fast Refresh is only used in DEV builds, it should be okay.
setTimeout(unmountAndRemountAllRoots, 0);
return;
}
}

if (trackedPathMatchFiber !== null) {
// We're in the process of trying to restore previous selection.
// If this fiber matched but is being unmounted, there's no use trying.
Expand All @@ -1823,11 +1801,14 @@ export function attach(
}
}

const unsafeID = getFiberIDUnsafe(fiber);
if (unsafeID === null) {
// If we've never seen this Fiber, it might be inside of a legacy render Suspense fragment (so the store is not even aware of it).
// In that case we can just ignore it or it will cause errors later on.
// One example of this is a Lazy component that never resolves before being unmounted.
//
// This also might indicate a Fast Refresh force-remount scenario.
//
// TODO: This is fragile and can obscure actual bugs.
return;
}
Expand Down