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

Eagerly initialize an mutable object for instance.refs #25696

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

sebmarkbage
Copy link
Collaborator

This micro-optimization never made sense and less so now that they're rare.

This still initializes the class with a shared immutable object in the constructor - which is also what createClass() does.

Then we override it during mount. This is done in case someone messes up the initialization of the super() constructor for example, which was more common in polyfills.

This change means that if a ref is initialized during the constructor itself it wouldn't be lazily initialized but that's not user code that does it, it's React so that shouldn't happen.

This makes string refs codemoddable as described in. #25334

This micro-optimization never made sense and less so now that they're rare.

This still initializes the class with a shared immutable object in the
constructor - which is also what createClass() does.

Then we override it during mount. This is done in case someone messes up
the initialization of the super() constructor for example, which was
more common in polyfills.

This change means that if a ref is initialized during the constructor
itself it wouldn't be lazily initialized but that's not user code that
does it, it's React so that shouldn't happen.
@sizebot
Copy link

sizebot commented Nov 16, 2022

Comparing: e1dd0a2...2976957

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.min.js = 153.64 kB 153.57 kB = 48.90 kB 48.82 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 155.56 kB 155.49 kB = 49.51 kB 49.46 kB
facebook-www/ReactDOM-prod.classic.js = 530.41 kB 530.25 kB = 94.67 kB 94.12 kB
facebook-www/ReactDOM-prod.modern.js = 515.66 kB 515.51 kB = 92.49 kB 91.93 kB
facebook-www/ReactDOMForked-prod.classic.js = 530.41 kB 530.25 kB = 94.67 kB 94.12 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 2976957

Copy link
Collaborator

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

🥳

@sebmarkbage sebmarkbage merged commit d65b88d into facebook:main Nov 16, 2022
mofeiZ pushed a commit to mofeiZ/react that referenced this pull request Nov 17, 2022
This micro-optimization never made sense and less so now that they're
rare.

This still initializes the class with a shared immutable object in the
constructor - which is also what createClass() does.

Then we override it during mount. This is done in case someone messes up
the initialization of the super() constructor for example, which was
more common in polyfills.

This change means that if a ref is initialized during the constructor
itself it wouldn't be lazily initialized but that's not user code that
does it, it's React so that shouldn't happen.

This makes string refs codemoddable as described in.
facebook#25334
rickhanlonii pushed a commit that referenced this pull request Dec 3, 2022
This micro-optimization never made sense and less so now that they're
rare.

This still initializes the class with a shared immutable object in the
constructor - which is also what createClass() does.

Then we override it during mount. This is done in case someone messes up
the initialization of the super() constructor for example, which was
more common in polyfills.

This change means that if a ref is initialized during the constructor
itself it wouldn't be lazily initialized but that's not user code that
does it, it's React so that shouldn't happen.

This makes string refs codemoddable as described in.
#25334
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jan 30, 2023
Summary:
Three problems popped up during the sync:
- facebook/react@07f46ecf2 breaks breaks tests
- facebook/react@6fb8133ed breaks fbsource tests. I added a workaround and created a test for the team that owns the test.
- https://fb.workplace.com/groups/flowlang/permalink/1198137807458547/ enables local type interference in fbsource but not in github React repo and some code breaks. Addressed in facebook/react#26064

This sync includes the following changes:
- **[17f6912a4](facebook/react@17f6912a4 )**: Add flow types to ReactFiberHooks ([#25752](facebook/react#25752)) //<Samuel Susla>//
- **[f101c2d0d](facebook/react@f101c2d0d )**: Remove Reconciler fork (2/2) ([#25775](facebook/react#25775)) //<Jan Kassens>//
- **[420f0b7fa](facebook/react@420f0b7fa )**: Remove Reconciler fork (1/2) ([#25774](facebook/react#25774)) //<Jan Kassens>//
- **[3ba7add60](facebook/react@3ba7add60 )**: Allow async blocks in `to(Error|Warn)Dev` ([#25338](facebook/react#25338)) //<Sebastian Silbermann>//
- **[fa11bd6ec](facebook/react@fa11bd6ec )**: [ServerRenderer] Add option to send instructions as data attributes ([#25437](facebook/react#25437)) //<mofeiZ>//
- **[e98225485](facebook/react@e98225485 )**: Add ref cleanup function ([#25686](facebook/react#25686)) //<Samuel Susla>//
- **[15557fa67](facebook/react@15557fa67 )**: [Fix] properly track `useId` use in StrictMode in development ([#25713](facebook/react#25713)) //<Josh Story>//
- **[8a23def32](facebook/react@8a23def32 )**: Resubmit Add HydrationSyncLane ([#25711](facebook/react#25711)) //<Tianyu Yao>//
- **[2655c9354](facebook/react@2655c9354 )**: Fizz Browser: fix precomputed chunk being cleared on Node 18 ([#25645](facebook/react#25645)) //<Jimmy Lai>//
- **[c08d8b804](facebook/react@c08d8b804 )**: Revert "Add SyncHydrationLane" ([#25708](facebook/react#25708)) //<Tianyu Yao>//
- **[56ffca8b9](facebook/react@56ffca8b9 )**: Add Bun streaming server renderer ([#25597](facebook/react#25597)) //<Colin McDonnell>//
- **[f31005d6a](facebook/react@f31005d6a )**: Add SyncHydrationLane ([#25698](facebook/react#25698)) //<Tianyu Yao>//
- **[f284d9faf](facebook/react@f284d9faf )**: Track ThenableState alongside other hooks //<Andrew Clark>//
- **[6b4c0314e](facebook/react@6b4c0314e )**: Check thenable instead of thenableState //<Andrew Clark>//
- **[33e3d2878](facebook/react@33e3d2878 )**: Reuse hooks when replaying a suspended component //<Andrew Clark>//
- **[4387d752d](facebook/react@4387d752d )**: Allow more hooks to be added when replaying mount //<Andrew Clark>//
- **[5eb78d0a0](facebook/react@5eb78d0a0 )**: Pass ThenableState to replaySuspendedUnitOfWork //<Andrew Clark>//
- **[4a2d86bdd](facebook/react@4a2d86bdd )**: Don't reset work loop until stack is unwound //<Andrew Clark>//
- **[9dfbd9fa9](facebook/react@9dfbd9fa9 )**: use: Don't suspend if there are pending updates //<Andrew Clark>//
- **[44c4e6f4d](facebook/react@44c4e6f4d )**: Force unwind work loop during selective hydration ([#25695](facebook/react#25695)) //<Andrew Clark>//
- **[7b17f7bbf](facebook/react@7b17f7bbf )**: Enable warning for defaultProps on function components for everyone ([#25699](facebook/react#25699)) //<Sebastian Markbåge>//
- **[6fb8133ed](facebook/react@6fb8133ed )**: Turn on string ref deprecation warning for everybody (not codemoddable) ([#25383](facebook/react#25383)) //<Sebastian Silbermann>//
- **[07f46ecf2](facebook/react@07f46ecf2 )**: Turn on key spread warning in jsx-runtime for everyone ([#25697](facebook/react#25697)) //<Sebastian Markbåge>//
- **[d65b88d03](facebook/react@d65b88d03 )**: Eagerly initialize an mutable object for instance.refs ([#25696](facebook/react#25696)) //<Sebastian Markbåge>//
- **[c343f8025](facebook/react@c343f8025 )**: [react-float] feature detect getRootNode ([#25689](facebook/react#25689)) //<Jan Kassens>//
- **[e1dd0a2f5](facebook/react@e1dd0a2f5 )**: Remove recoverable error when a sync update flows into a dehydrated boundary ([#25692](facebook/react#25692)) //<Sebastian Markbåge>//
- **[c54e3541b](facebook/react@c54e3541b )**: [DevTools] bug fix for Hydrating fibers ([#25663](facebook/react#25663)) //<Mengdi Chen>//

Changelog:
[General][Changed] - React Native sync for revisions d1e35c7...17f6912

jest_e2e[run_all_tests]

Reviewed By: makovkastar

Differential Revision: D42804802

fbshipit-source-id: 6a9f00724cc73378025bbd04edb2d17760a87280
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
Three problems popped up during the sync:
- facebook/react@07f46ecf2 breaks breaks tests
- facebook/react@6fb8133ed breaks fbsource tests. I added a workaround and created a test for the team that owns the test.
- https://fb.workplace.com/groups/flowlang/permalink/1198137807458547/ enables local type interference in fbsource but not in github React repo and some code breaks. Addressed in facebook/react#26064

This sync includes the following changes:
- **[17f6912a4](facebook/react@17f6912a4 )**: Add flow types to ReactFiberHooks ([facebook#25752](facebook/react#25752)) //<Samuel Susla>//
- **[f101c2d0d](facebook/react@f101c2d0d )**: Remove Reconciler fork (2/2) ([facebook#25775](facebook/react#25775)) //<Jan Kassens>//
- **[420f0b7fa](facebook/react@420f0b7fa )**: Remove Reconciler fork (1/2) ([facebook#25774](facebook/react#25774)) //<Jan Kassens>//
- **[3ba7add60](facebook/react@3ba7add60 )**: Allow async blocks in `to(Error|Warn)Dev` ([facebook#25338](facebook/react#25338)) //<Sebastian Silbermann>//
- **[fa11bd6ec](facebook/react@fa11bd6ec )**: [ServerRenderer] Add option to send instructions as data attributes ([facebook#25437](facebook/react#25437)) //<mofeiZ>//
- **[e98225485](facebook/react@e98225485 )**: Add ref cleanup function ([facebook#25686](facebook/react#25686)) //<Samuel Susla>//
- **[15557fa67](facebook/react@15557fa67 )**: [Fix] properly track `useId` use in StrictMode in development ([facebook#25713](facebook/react#25713)) //<Josh Story>//
- **[8a23def32](facebook/react@8a23def32 )**: Resubmit Add HydrationSyncLane ([facebook#25711](facebook/react#25711)) //<Tianyu Yao>//
- **[2655c9354](facebook/react@2655c9354 )**: Fizz Browser: fix precomputed chunk being cleared on Node 18 ([facebook#25645](facebook/react#25645)) //<Jimmy Lai>//
- **[c08d8b804](facebook/react@c08d8b804 )**: Revert "Add SyncHydrationLane" ([facebook#25708](facebook/react#25708)) //<Tianyu Yao>//
- **[56ffca8b9](facebook/react@56ffca8b9 )**: Add Bun streaming server renderer ([facebook#25597](facebook/react#25597)) //<Colin McDonnell>//
- **[f31005d6a](facebook/react@f31005d6a )**: Add SyncHydrationLane ([facebook#25698](facebook/react#25698)) //<Tianyu Yao>//
- **[f284d9faf](facebook/react@f284d9faf )**: Track ThenableState alongside other hooks //<Andrew Clark>//
- **[6b4c0314e](facebook/react@6b4c0314e )**: Check thenable instead of thenableState //<Andrew Clark>//
- **[33e3d2878](facebook/react@33e3d2878 )**: Reuse hooks when replaying a suspended component //<Andrew Clark>//
- **[4387d752d](facebook/react@4387d752d )**: Allow more hooks to be added when replaying mount //<Andrew Clark>//
- **[5eb78d0a0](facebook/react@5eb78d0a0 )**: Pass ThenableState to replaySuspendedUnitOfWork //<Andrew Clark>//
- **[4a2d86bdd](facebook/react@4a2d86bdd )**: Don't reset work loop until stack is unwound //<Andrew Clark>//
- **[9dfbd9fa9](facebook/react@9dfbd9fa9 )**: use: Don't suspend if there are pending updates //<Andrew Clark>//
- **[44c4e6f4d](facebook/react@44c4e6f4d )**: Force unwind work loop during selective hydration ([facebook#25695](facebook/react#25695)) //<Andrew Clark>//
- **[7b17f7bbf](facebook/react@7b17f7bbf )**: Enable warning for defaultProps on function components for everyone ([facebook#25699](facebook/react#25699)) //<Sebastian Markbåge>//
- **[6fb8133ed](facebook/react@6fb8133ed )**: Turn on string ref deprecation warning for everybody (not codemoddable) ([facebook#25383](facebook/react#25383)) //<Sebastian Silbermann>//
- **[07f46ecf2](facebook/react@07f46ecf2 )**: Turn on key spread warning in jsx-runtime for everyone ([facebook#25697](facebook/react#25697)) //<Sebastian Markbåge>//
- **[d65b88d03](facebook/react@d65b88d03 )**: Eagerly initialize an mutable object for instance.refs ([facebook#25696](facebook/react#25696)) //<Sebastian Markbåge>//
- **[c343f8025](facebook/react@c343f8025 )**: [react-float] feature detect getRootNode ([facebook#25689](facebook/react#25689)) //<Jan Kassens>//
- **[e1dd0a2f5](facebook/react@e1dd0a2f5 )**: Remove recoverable error when a sync update flows into a dehydrated boundary ([facebook#25692](facebook/react#25692)) //<Sebastian Markbåge>//
- **[c54e3541b](facebook/react@c54e3541b )**: [DevTools] bug fix for Hydrating fibers ([facebook#25663](facebook/react#25663)) //<Mengdi Chen>//

Changelog:
[General][Changed] - React Native sync for revisions d1e35c7...17f6912

jest_e2e[run_all_tests]

Reviewed By: makovkastar

Differential Revision: D42804802

fbshipit-source-id: 6a9f00724cc73378025bbd04edb2d17760a87280
acdlite added a commit to acdlite/react that referenced this pull request Apr 25, 2024
It turns out we already made refs writable in facebook#25696, which has been
in canary for over a year. The approach in that PR also has the benefit
of being slightly more perf sensitive because it still uses a shared
object until the fiber is mounted. So let's just go back to that.
acdlite added a commit that referenced this pull request Apr 25, 2024
It turns out we already made refs writable in #25696, which has been in
canary for over a year. The approach in that PR also has the benefit of
being slightly more perf sensitive because it still uses a shared object
until the fiber is mounted. So let's just go back to that.
github-actions bot pushed a commit that referenced this pull request Apr 25, 2024
It turns out we already made refs writable in #25696, which has been in
canary for over a year. The approach in that PR also has the benefit of
being slightly more perf sensitive because it still uses a shared object
until the fiber is mounted. So let's just go back to that.

DiffTrain build for commit d285b3a.
github-actions bot pushed a commit that referenced this pull request Apr 25, 2024
It turns out we already made refs writable in #25696, which has been in
canary for over a year. The approach in that PR also has the benefit of
being slightly more perf sensitive because it still uses a shared object
until the fiber is mounted. So let's just go back to that.

DiffTrain build for [d285b3a](d285b3a)
bigfootjon pushed a commit that referenced this pull request Apr 25, 2024
It turns out we already made refs writable in #25696, which has been in
canary for over a year. The approach in that PR also has the benefit of
being slightly more perf sensitive because it still uses a shared object
until the fiber is mounted. So let's just go back to that.

DiffTrain build for commit d285b3a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants