react icon indicating copy to clipboard operation
react copied to clipboard

Unify ReactFiberCurrentOwner and ReactCurrentFiber

Open sebmarkbage opened this issue 1 year ago • 2 comments

We previously had two slightly different concepts for "current fiber".

There's the "owner" which is set inside of class components in prod if string refs are enabled, and sometimes inside function components in DEV but not other contexts.

Then we have the "current fiber" which is only set in DEV for various warnings but is enabled in a bunch of contexts.

This unifies them into a single "current fiber".

The concept of string refs shouldn't really exist so this should really be a DEV only concept. In the meantime, this sets the current fiber inside class render only in prod, however, in DEV it's now enabled in more contexts which can affect the string refs. That was already the case that a string ref in a Function component was only connecting to the owner in prod. Any string ref associated with any non-class won't work regardless so that's not an issue. The practical change here is that an element with a string ref created inside a life-cycle associated with a class will work in DEV but not in prod. Since we need the current fiber to be available in more contexts in DEV for the debugging purposes. That wouldn't affect any old code since it would have a broken ref anyway. New code shouldn't use string refs anyway.

The other implication is that "owner" doesn't necessarily mean "rendering" since we need the "owner" to track other debug information like stacks - in other contexts like useEffect, life cycles, etc. Internally we have a separate isRendering flag that actually means we're rendering but even that is a very overloaded concept. So anything that uses "owner" to imply rendering might be wrong with this change.

This is a first step to a larger refactor for tracking current rendering information.

sebmarkbage avatar May 10 '24 01:05 sebmarkbage

Comparing: 55dd0b1d0e2a17f137579f85016e69782cad7cf7...599e50f75f602f98b712904c84fc98a4fdd88979

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB +0.05% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 495.71 kB 495.71 kB = 88.78 kB 88.75 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 500.51 kB 500.51 kB = 89.47 kB 89.43 kB
facebook-www/ReactDOM-prod.classic.js +0.07% 592.86 kB 593.27 kB +0.08% 104.28 kB 104.37 kB
facebook-www/ReactDOM-prod.modern.js +0.10% 569.08 kB 569.65 kB +0.08% 100.69 kB 100.77 kB
test_utils/ReactAllWarnings.js Deleted 64.35 kB 0.00 kB Deleted 16.05 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-reconciler/cjs/react-reconciler-reflection.development.js +0.71% 18.57 kB 18.70 kB +0.90% 5.31 kB 5.36 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler-reflection.development.js +0.71% 18.57 kB 18.70 kB +0.90% 5.31 kB 5.36 kB
oss-stable/react-reconciler/cjs/react-reconciler-reflection.development.js +0.71% 18.57 kB 18.70 kB +0.90% 5.31 kB 5.36 kB
facebook-www/ReactTestUtils-dev.classic.js +0.23% 44.68 kB 44.78 kB +0.32% 12.78 kB 12.82 kB
facebook-www/ReactTestUtils-dev.modern.js +0.23% 44.68 kB 44.78 kB +0.32% 12.78 kB 12.82 kB
test_utils/ReactAllWarnings.js Deleted 64.35 kB 0.00 kB Deleted 16.05 kB 0.00 kB

Generated by :no_entry_sign: dangerJS against 599e50f75f602f98b712904c84fc98a4fdd88979

react-sizebot avatar May 10 '24 02:05 react-sizebot

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2024 4:29pm

vercel[bot] avatar May 21 '24 03:05 vercel[bot]