-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Implement getServerSnapshot in userspace shim #22359
Conversation
Comparing: 86b3e24...c496f7c 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
|
6f29295
to
04cb3aa
Compare
jest.mock('react', () => { | ||
const { | ||
// eslint-disable-next-line no-unused-vars | ||
startTransition: _, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious why do you need to remove these fields for the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's to simulate a build of React 17, which includes neither useSyncExternalStore
nor startTransition
.
The reason startTransition
is relevant is because I use it as a heuristic to check whether you're using an outdated alpha of React 18: https://github.com/facebook/react/blob/main/packages/use-sync-external-store/src/useSyncExternalStore.js#L49-L61
The eventual plan is to load a copy of React 17 from npm and test the shim using that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good stuff
04cb3aa
to
4080516
Compare
Idea is that eventually we'll run these tests against an actual build of React DOM 17 to test backwards compatibility.
If the DOM is not present, we assume that we are running in a server environment and return the result of `getServerSnapshot`. This heuristic doesn't work in React Native, so we'll need to provide a separate native build (using the `.native` extension). I've left this for a follow-up. We can't call `getServerSnapshot` on the client, because in versions of React before 18, there's no built-in mechanism to detect whether we're hydrating. To avoid a server mismatch warning, users must account for this themselves and return the correct value inside `getSnapshot`. Note that none of this is relevant to the built-in API that is being added in 18. This only affects the userspace shim that is provided for backwards compatibility with versions 16 and 17.
4080516
to
c496f7c
Compare
Summary: This sync includes the following changes: - **[579c008a7](facebook/react@579c008a7 )**: [Fizz/Flight] pipeToNodeWritable(..., writable).startWriting() -> renderToPipeableStream(...).pipe(writable) ([#22450](facebook/react#22450)) //<Sebastian Markbåge>// - **[f2c381131](facebook/react@f2c381131 )**: fix: useSyncExternalStoreExtra ([#22500](facebook/react#22500)) //<Daishi Kato>// - **[0ecbbe142](facebook/react@0ecbbe142 )**: Sync hydrate discrete events in capture phase and dont replay discrete events ([#22448](facebook/react#22448)) //<salazarm>// - **[a724a3b57](facebook/react@a724a3b57 )**: [RFC] Codemod invariant -> throw new Error ([#22435](facebook/react#22435)) //<Andrew Clark>// - **[201af81b0](facebook/react@201af81b0 )**: Release pooled cache reference in complete/unwind ([#22464](facebook/react#22464)) //<Joseph Savona>// - **[033efe731](facebook/react@033efe731 )**: Call get snapshot in useSyncExternalStore server shim ([#22453](facebook/react#22453)) //<salazarm>// - **[7843b142a](facebook/react@7843b142a )**: [Fizz/Flight] Pass in Destination lazily to startFlowing instead of in createRequest ([#22449](facebook/react#22449)) //<Sebastian Markbåge>// - **[d9fb383d6](facebook/react@d9fb383d6 )**: Extract queueing logic into shared functions ([#22452](facebook/react#22452)) //<Andrew Clark>// - **[9175f4d15](facebook/react@9175f4d15 )**: Scheduling Profiler: Show Suspense resource .displayName ([#22451](facebook/react#22451)) //<Brian Vaughn>// - **[eba248c39](facebook/react@eba248c39 )**: [Fizz/Flight] Remove reentrancy hack ([#22446](facebook/react#22446)) //<Sebastian Markbåge>// - **[66388150e](facebook/react@66388150e )**: Remove usereducer eager bailout ([#22445](facebook/react#22445)) //<Joseph Savona>// - **[d3e086932](facebook/react@d3e086932 )**: Make root.unmount() synchronous ([#22444](facebook/react#22444)) //<Andrew Clark>// - **[2cc6d79c9](facebook/react@2cc6d79c9 )**: Rename onReadyToStream to onCompleteShell ([#22443](facebook/react#22443)) //<Sebastian Markbåge>// - **[c88fb49d3](facebook/react@c88fb49d3 )**: Improve DEV errors if string coercion throws (Temporal.*, Symbol, etc.) ([#22064](facebook/react#22064)) //<Justin Grant>// - **[05726d72c](facebook/react@05726d72c )**: [Fix] Errors should not "unsuspend" a transition ([#22423](facebook/react#22423)) //<Andrew Clark>// - **[3746eaf98](facebook/react@3746eaf98 )**: Packages/React/src/ReactLazy ---> changing -1 to unintialized ([#22421](facebook/react#22421)) //<BIKI DAS>// - **[04ccc01d9](facebook/react@04ccc01d9 )**: Hydration errors should force a client render ([#22416](facebook/react#22416)) //<Andrew Clark>// - **[029fdcebb](facebook/react@029fdcebb )**: root.hydrate -> root.isDehydrated ([#22420](facebook/react#22420)) //<Andrew Clark>// - **[af87f5a83](facebook/react@af87f5a83 )**: Scheduling Profiler marks should include thrown Errors ([#22417](facebook/react#22417)) //<Brian Vaughn>// - **[d47339ea3](facebook/react@d47339ea3 )**: [Fizz] Remove assignID mechanism ([#22410](facebook/react#22410)) //<Sebastian Markbåge>// - **[3a50d9557](facebook/react@3a50d9557 )**: Never attach ping listeners in legacy Suspense ([#22407](facebook/react#22407)) //<Andrew Clark>// - **[82c8fa90b](facebook/react@82c8fa90b )**: Add back useMutableSource temporarily ([#22396](facebook/react#22396)) //<Andrew Clark>// - **[5b57bc6e3](facebook/react@5b57bc6e3 )**: [Draft] don't patch console during first render ([#22308](facebook/react#22308)) //<Luna Ruan>// - **[cf07c3df1](facebook/react@cf07c3df1 )**: Delete all but one `build2` reference ([#22391](facebook/react#22391)) //<Andrew Clark>// - **[bb0d06935](facebook/react@bb0d06935 )**: [build2 -> build] Local scripts //<Andrew Clark>// - **[0c81d347b](facebook/react@0c81d347b )**: Write artifacts to `build` instead of `build2` //<Andrew Clark>// - **[4da03c9fb](facebook/react@4da03c9fb )**: useSyncExternalStore React Native version ([#22367](facebook/react#22367)) //<salazarm>// - **[48d475c9e](facebook/react@48d475c9e )**: correct typos ([#22294](facebook/react#22294)) //<Bowen>// - **[cb6c619c0](facebook/react@cb6c619c0 )**: Remove Fiber fields that were used for hydrating useMutableSource ([#22368](facebook/react#22368)) //<Sebastian Silbermann>// - **[64e70f82e](facebook/react@64e70f82e )**: [Fizz] add avoidThisFallback support ([#22318](facebook/react#22318)) //<salazarm>// - **[3ee7a004e](facebook/react@3ee7a004e )**: devtools: Display actual ReactDOM API name in root type ([#22363](facebook/react#22363)) //<Sebastian Silbermann>// - **[79b8fc667](facebook/react@79b8fc667 )**: Implement getServerSnapshot in userspace shim ([#22359](facebook/react#22359)) //<Andrew Clark>// - **[86b3e2461](facebook/react@86b3e2461 )**: Implement useSyncExternalStore on server ([#22347](facebook/react#22347)) //<Andrew Clark>// - **[8209de269](facebook/react@8209de269 )**: Delete useMutableSource implementation ([#22292](facebook/react#22292)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions e8feb11...afcb9cd jest_e2e[run_all_tests] Reviewed By: yungsters Differential Revision: D31541359 fbshipit-source-id: c35941bc303fdf55cb061e9996200dc868a6f2af
* Convert useSES shim tests to use React DOM Idea is that eventually we'll run these tests against an actual build of React DOM 17 to test backwards compatibility. * Implement getServerSnapshot in userspace shim If the DOM is not present, we assume that we are running in a server environment and return the result of `getServerSnapshot`. This heuristic doesn't work in React Native, so we'll need to provide a separate native build (using the `.native` extension). I've left this for a follow-up. We can't call `getServerSnapshot` on the client, because in versions of React before 18, there's no built-in mechanism to detect whether we're hydrating. To avoid a server mismatch warning, users must account for this themselves and return the correct value inside `getSnapshot`. Note that none of this is relevant to the built-in API that is being added in 18. This only affects the userspace shim that is provided for backwards compatibility with versions 16 and 17.
If the DOM is not present, we assume that we are running in a server environment and return the result of
getServerSnapshot
.This heuristic doesn't work in React Native, so we'll need to provide a separate native build (using the
.native
extension). I've left this for a follow-up.We can't call
getServerSnapshot
on the client, because in versions of React before 18, there's no built-in mechanism to detect whether we're hydrating. To avoid a server mismatch warning, users must account for this themselves and return the correct value insidegetSnapshot
.Note that none of this is relevant to the built-in API that is being added in 18. This only affects the userspace shim that is provided for backwards compatibility with versions 16 and 17.