Skip to content

Commit

Permalink
Warn if static flag is accidentally cleared (facebook#20807)
Browse files Browse the repository at this point in the history
* Warn if static flag is accidentally cleared

"Static" fiber flags are flags that are meant to exist for the lifetime
of a component. It's really important not to accidentally reset these,
because we use them to decide whether or not to perform some operation
on a tree (which we can do because they get bubbled via `subtreeFlags)`.

We've had several bugs that were caused by this mistake, so we actually
don't rely on static flags anywhere, yet. But we'd like to.

So let's roll out this warning and see if it fires anywhere. Once we
can confirm that there are no warnings, we can assume that it's safe
to start using static flags.

I did not wrap it behind a feature flag, because it's dev-only, and we
can use our internal warning filter to hide this from the console.

* Intentionally clear static flag to test warning

* ...and fix it again
  • Loading branch information
acdlite authored and koto committed Jun 15, 2021
1 parent cba639d commit b9ed6a0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
15 changes: 15 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,21 @@ export function renderWithHooks<Props, SecondArg>(
currentHookNameInDev = null;
hookTypesDev = null;
hookTypesUpdateIndexDev = -1;

// Confirm that a static flag was not added or removed since the last
// render. If this fires, it suggests that we incorrectly reset the static
// flags in some other part of the codebase. This has happened before, for
// example, in the SuspenseList implementation.
if (
current !== null &&
(current.flags & PassiveStaticEffect) !==
(workInProgress.flags & PassiveStaticEffect)
) {
console.error(
'Internal React error: Expected static flag was missing. Please ' +
'notify the React team.',
);
}
}

didScheduleRenderPhaseUpdate = false;
Expand Down
15 changes: 15 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,21 @@ export function renderWithHooks<Props, SecondArg>(
currentHookNameInDev = null;
hookTypesDev = null;
hookTypesUpdateIndexDev = -1;

// Confirm that a static flag was not added or removed since the last
// render. If this fires, it suggests that we incorrectly reset the static
// flags in some other part of the codebase. This has happened before, for
// example, in the SuspenseList implementation.
if (
current !== null &&
(current.flags & PassiveStaticEffect) !==
(workInProgress.flags & PassiveStaticEffect)
) {
console.error(
'Internal React error: Expected static flag was missing. Please ' +
'notify the React team.',
);
}
}

didScheduleRenderPhaseUpdate = false;
Expand Down

0 comments on commit b9ed6a0

Please sign in to comment.