Repository navigation
Conversation
The profiler walked the whole tree on every commit and counted a fiber when any flag was set or when props or state differed from its alternate. Flags from an earlier render stay on fibers React bailed out on, and a fiber never updated since mount has no alternate at all, so untouched components were re-counted on every commit and fresh mounts were reported again and again. Count a fiber only when it ran in this commit, the way React DevTools does (didFiberRender): gate on the PerformedWork flag, and descend into a subtree only when its child pointer changed since the previous commit. On mount there is no alternate yet, so the whole tree is walked and mounts are still counted once. Fixes vercel-labs#2508
Contributor
|
@sx4im is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
React 16 stores effect flags in `effectTag`; React 17 renamed the field to `flags`. The renders profiler gated counting on `fiber.flags` only, so on a React 16 page every post-mount render was dropped: `fiber.flags` is undefined there and the PerformedWork check never fired. The old code still counted those renders through its props/state comparison, so this is a regression for React 16 pages. Read the flags through a small helper that returns `fiber.flags` when present and falls back to `fiber.effectTag`. The bit values are the same in both versions. Applies to the render gate and to the DOM-mutation check, which reads the same field.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
react renderscounted components that never rendered in a commit. It walked the whole tree and counted a fiber when any flag was set, or when props or state differed from its alternate. Flags from an earlier render stay on fibers React bailed out on, and a fiber never updated since mount has no alternate, so untouched components were re-counted on every commit and mounts were reported again and again.Count a fiber only when it ran in this commit, the way React DevTools does (didFiberRender): gate on the PerformedWork flag, and descend into a subtree only when its child pointer changed since the previous commit. On mount there is no alternate yet, so the whole tree is walked and mounts still count once.
How verified: ran the profiler scripts in Node against simulated React 19 fiber trees for the repro in the issue (mount plus 10 ticks). Ticker gets 10 re-renders, StaticList and Static get 0. The same harness against the old code reproduces the bug (StaticList 10 re-renders, Static 50 extra mounts). Added a unit test pinning the new rule, and
cargo fmt --checkis clean. The full Rust test run is left to CI.Fixes #2508