-
Notifications
You must be signed in to change notification settings - Fork 46.3k
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
Point useSubscription to useSyncExternalStore shim #24289
Conversation
Comparing: 4bc465a...41c3664 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
@gaearon the failures are because Specifically, react/packages/use-subscription/src/useSubscription.js Lines 88 to 108 in d5f1b06
And react/packages/react-reconciler/src/ReactFiberHooks.new.js Lines 1477 to 1480 in d5f1b06
Which schedules a sync lane update here (instead of calling react/packages/react-reconciler/src/ReactFiberHooks.new.js Lines 1497 to 1499 in d5f1b06
For the first test, the sync update interrupts the in progress transition and forces two sync updates. That's why you see that grandchild isn't rendered (I think, I'm not 100% sure why grandchild isn't rendered there), and once fixed you'll see two sync renders. For the second test, since the mutation forces a sync render, |
So arguably this can be considered a bugfix and we can change the test? I want to understand if we can release this in a minor. |
Yes, I think that's fair. The test don't fail because of broken behavior, but different semantics and the new semantics are better. |
* Point useSubscription to useSyncExternalStore shim * Update tests * Update README * Ad hoc case
* Point useSubscription to useSyncExternalStore shim * Update tests * Update README * Ad hoc case
* Point useSubscription to useSyncExternalStore shim * Update tests * Update README * Ad hoc case
* Point useSubscription to useSyncExternalStore shim * Update tests * Update README * Ad hoc case
Summary: This sync includes the following changes: - **[bd4784c8f](facebook/react@bd4784c8f )**: Revert #24236 (Don't recreate the same fallback on the client if hydrating suspends) ([#24434](facebook/react#24434)) //<dan>// - **[6d3b6d0f4](facebook/react@6d3b6d0f4 )**: forwardRef et al shouldn't affect if props reused ([#24421](facebook/react#24421)) //<Andrew Clark>// - **[bd0813766](facebook/react@bd0813766 )**: Fix: useDeferredValue should reuse previous value ([#24413](facebook/react#24413)) //<Andrew Clark>// - **[9ae80d6a2](facebook/react@9ae80d6a2 )**: Suppress hydration warnings when a preceding sibling suspends ([#24404](facebook/react#24404)) //<Josh Story>// - **[0dc4e6663](facebook/react@0dc4e6663 )**: Land enableClientRenderFallbackOnHydrationMismatch ([#24410](facebook/react#24410)) //<Andrew Clark>// - **[354772952](facebook/react@354772952 )**: Land enableSelectiveHydration flag ([#24406](facebook/react#24406)) //<Andrew Clark>// - **[392808a1f](facebook/react@392808a1f )**: Land enableClientRenderFallbackOnTextMismatch flag ([#24405](facebook/react#24405)) //<Andrew Clark>// - **[1e748b452](facebook/react@1e748b452 )**: Land enableLazyElements flag ([#24407](facebook/react#24407)) //<Andrew Clark>// - **[4175f0593](facebook/react@4175f0593 )**: Temporarily feature flag numeric fallback for symbols ([#24401](facebook/react#24401)) //<Ricky>// - **[a6d53f346](facebook/react@a6d53f346 )**: Revert "Clean up Selective Hydration / Event Replay flag ([#24156](facebook/react#24156))" ([#24402](facebook/react#24402)) //<Ricky>// - **[ab9cdd34f](facebook/react@ab9cdd34f )**: Bugfix: In legacy mode, call suspended tree's unmount effects when it is deleted ([#24400](facebook/react#24400)) //<Andrew Clark>// - **[168da8d55](facebook/react@168da8d55 )**: Fix typo that happened during rebasing //<Andrew Clark>// - **[8bc527a4c](facebook/react@8bc527a4c )**: Bugfix: Fix race condition between interleaved and non-interleaved updates ([#24353](facebook/react#24353)) //<Andrew Clark>// - **[f7cf077cc](facebook/react@f7cf077cc )**: [Transition Tracing] Add Offscreen Queue ([#24341](facebook/react#24341)) //<Luna Ruan>// - **[4fc394bbe](facebook/react@4fc394bbe )**: Fix suspense fallback throttling ([#24253](facebook/react#24253)) //<sunderls>// - **[80170a068](facebook/react@80170a068 )**: Match bundle.name and match upper case entry points ([#24346](facebook/react#24346)) //<Sebastian Markbåge>// - **[fea6f8da6](facebook/react@fea6f8da6 )**: [Transition Tracing] Add transition to OffscreenState and pendingSuspenseBoundaries to RootState ([#24340](facebook/react#24340)) //<Luna Ruan>// - **[8e2f9b086](facebook/react@8e2f9b086 )**: move passive flag ([#24339](facebook/react#24339)) //<Luna Ruan>// - **[55a21ef7e](facebook/react@55a21ef7e )**: fix pushTransition for transition tracing ([#24338](facebook/react#24338)) //<Luna Ruan>// - **[069d23bb7](facebook/react@069d23bb7 )**: [eslint-plugin-exhaustive-deps] Fix exhaustive deps check for unstable vars ([#24343](facebook/react#24343)) //<Afzal Sayed>// - **[4997515b9](facebook/react@4997515b9 )**: Point useSubscription to useSyncExternalStore shim ([#24289](facebook/react#24289)) //<dan>// - **[01e2bff1d](facebook/react@01e2bff1d )**: Remove unnecessary check ([#24332](facebook/react#24332)) //<zhoulixiang>// - **[d9a0f9e20](facebook/react@d9a0f9e20 )**: Delete create-subscription folder ([#24288](facebook/react#24288)) //<dan>// - **[f993ffc51](facebook/react@f993ffc51 )**: Fix infinite update loop that happens when an unmemoized value is passed to useDeferredValue ([#24247](facebook/react#24247)) //<Andrew Clark>// - **[fa5800226](facebook/react@fa5800226 )**: [Fizz] Pipeable Stream Perf ([#24291](facebook/react#24291)) //<Josh Story>// - **[0568c0f8c](facebook/react@0568c0f8c )**: Replace zero with NoLanes for consistency in FiberLane ([#24327](facebook/react#24327)) //<Leo>// - **[e0160d50c](facebook/react@e0160d50c )**: add transition tracing transitions stack ([#24321](facebook/react#24321)) //<Luna Ruan>// - **[b0f13e5d3](facebook/react@b0f13e5d3 )**: add pendingPassiveTransitions ([#24320](facebook/react#24320)) //<Luna Ruan>// Changelog: [General][Changed] - React Native sync for revisions 60e63b9...bd4784c jest_e2e[run_all_tests] Reviewed By: kacieb Differential Revision: D35899012 fbshipit-source-id: 86a885e336fca9f0efa80cd2b8ca040f2cb53853
- [x] Make sure the linting passes by running `yarn lint` Back in 2019, React released the first version of `use-subscription` (facebook/react#15022). At the time, we only has limited information about concurrent rendering, and #9026 add the initial concurrent mode support. In 2020, React provides a first-party official API `useMutableSource` (reactjs/rfcs#147, facebook/react#18000): > ... enables React components to safely and efficiently read from a mutable external source in Concurrent Mode. React 18 introduces `useMutableSource`'s replacement `useSyncExternalStore` (see details here: reactwg/react-18#86), and React changes `use-subscription` implementation to use `useSyncExternalStore` directly: facebook/react#24289 > In React 18, `React.useSyncExternalStore` is a built-in replacement for `useSubscription`. > > This PR makes `useSubscription` simply use `React.useSyncExternalStore` when available. For pre-18, it uses a `use-sync-external-store` shim which is very similar in `use-subscription` but fixes some flaws with concurrent rendering. And according to `use-subscription`: > You may now migrate to [`use-sync-external-store`](https://www.npmjs.com/package/use-sync-external-store) directly instead, which has the same API as `React.useSyncExternalStore`. The `use-subscription` package is now a thin wrapper over `use-sync-external-store` and will not be updated further. The PR does exactly that: - Removes the precompiled `use-subscription` introduced in #35746 - Adds the `use-sync-external-store` to the dependencies. - The `use-sync-external-store` package enables compatibility with React 16 and React 17. - Do not pre-compile `use-sync-external-store` since it is also the dependency of some popular React state management libraries like `react-redux`, `zustand`, `valtio`, `@xstate/react` and `@apollo/client`, etc. By install - Replace `useSubscription` usage with `useSyncExternalStore` --- Ref: #9026, #35746 and #36159 Co-authored-by: Jiachi Liu <[email protected]>
In React 18,
React.useSyncExternalStore
is a built-in replacement foruseSubscription
.This PR makes
useSubscription
simply useReact.useSyncExternalStore
when available. For pre-18, it uses ause-sync-external-store
shim which is very similar inuse-subscription
but fixes some flaws with concurrent rendering.