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

useSyncExternalStore React Native version #22367

Merged
merged 19 commits into from
Sep 21, 2021
Merged

useSyncExternalStore React Native version #22367

merged 19 commits into from
Sep 21, 2021

Conversation

salazarm
Copy link
Contributor

Summary

React Native version of useSyncExternalStore shim. Basically a copy-pasta taking the isServerEnvironment = false branch

How did you test this change?

Not sure how to, recommendations?

@sizebot
Copy link

sizebot commented Sep 20, 2021

Comparing: 3ee7a00...bcdfcf3

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 = 127.43 kB 127.43 kB = 40.61 kB 40.61 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 130.26 kB 130.26 kB = 41.53 kB 41.53 kB
facebook-www/ReactDOM-prod.classic.js = 405.48 kB 405.48 kB = 75.07 kB 75.07 kB
facebook-www/ReactDOM-prod.modern.js = 394.07 kB 394.07 kB = 73.36 kB 73.36 kB
facebook-www/ReactDOMForked-prod.classic.js = 405.48 kB 405.48 kB = 75.07 kB 75.07 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store.native.development.js +∞% 0.00 kB 7.37 kB +∞% 0.00 kB 3.02 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store.native.production.min.js +∞% 0.00 kB 0.89 kB +∞% 0.00 kB 0.53 kB
oss-experimental/use-sync-external-store/index.native.js +∞% 0.00 kB 0.24 kB +∞% 0.00 kB 0.16 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store.native.development.js +∞% 0.00 kB 7.37 kB +∞% 0.00 kB 3.02 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store.native.production.min.js +∞% 0.00 kB 0.89 kB +∞% 0.00 kB 0.53 kB
oss-stable-semver/use-sync-external-store/index.native.js +∞% 0.00 kB 0.24 kB +∞% 0.00 kB 0.16 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store.native.development.js +∞% 0.00 kB 7.37 kB +∞% 0.00 kB 3.02 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store.native.production.min.js +∞% 0.00 kB 0.89 kB +∞% 0.00 kB 0.53 kB
oss-stable/use-sync-external-store/index.native.js +∞% 0.00 kB 0.24 kB +∞% 0.00 kB 0.16 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/use-sync-external-store/cjs/use-sync-external-store.native.development.js +∞% 0.00 kB 7.37 kB +∞% 0.00 kB 3.02 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store.native.production.min.js +∞% 0.00 kB 0.89 kB +∞% 0.00 kB 0.53 kB
oss-experimental/use-sync-external-store/index.native.js +∞% 0.00 kB 0.24 kB +∞% 0.00 kB 0.16 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store.native.development.js +∞% 0.00 kB 7.37 kB +∞% 0.00 kB 3.02 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store.native.production.min.js +∞% 0.00 kB 0.89 kB +∞% 0.00 kB 0.53 kB
oss-stable-semver/use-sync-external-store/index.native.js +∞% 0.00 kB 0.24 kB +∞% 0.00 kB 0.16 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store.native.development.js +∞% 0.00 kB 7.37 kB +∞% 0.00 kB 3.02 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store.native.production.min.js +∞% 0.00 kB 0.89 kB +∞% 0.00 kB 0.53 kB
oss-stable/use-sync-external-store/index.native.js +∞% 0.00 kB 0.24 kB +∞% 0.00 kB 0.16 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store.development.js = 8.02 kB 7.89 kB = 3.23 kB 3.16 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store.development.js = 8.02 kB 7.89 kB = 3.23 kB 3.16 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store.development.js = 8.02 kB 7.89 kB = 3.23 kB 3.16 kB

Generated by 🚫 dangerJS against bcdfcf3

@acdlite
Copy link
Collaborator

acdlite commented Sep 20, 2021

Forking the whole file works but in this case I think it'd be easy enough to compose two separate entry points. Something like this should work:

index.js:

import {canUseDOM} from 'shared/ExecutionEnvironment';
import client from './useSyncExternalStoreClient';
import server from './useSyncExternalStoreServer';

export const useSyncExternalStore = canUseDOM ? client : server;

index.native.js

export {default as useSyncExternalStore} from './useSyncExternalStoreClient';

@acdlite
Copy link
Collaborator

acdlite commented Sep 20, 2021

For testing, I think it's sufficient to require use-sync-external-store/index.native directly in a test suite that has @jest-environment node at the top, and write a test using a non-DOM renderer. So React Noop would be sufficient.

@acdlite
Copy link
Collaborator

acdlite commented Sep 20, 2021

You'll also need to update our build script. For example, we need to add a new entry here:

/******* Shim for useSyncExternalStore *******/
{
bundleTypes: [NODE_DEV, NODE_PROD],
moduleType: ISOMORPHIC,
entry: 'use-sync-external-store',
global: 'useSyncExternalStore',
externals: ['react'],
},

Our build setup is a bit confusing if you haven't worked with it before so I recommend using this PR as a reference: #22202. I can also pair with you if you like.

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

Looks great!

@acdlite
Copy link
Collaborator

acdlite commented Sep 21, 2021

Ah, you need to also add the new index.native file to this array:

"LICENSE",
"README.md",
"build-info.json",
"index.js",
"extra.js",
"cjs/"

@salazarm salazarm merged commit 4da03c9 into facebook:main Sep 21, 2021
@salazarm salazarm deleted the useSyncExternalStoreReactNative branch September 21, 2021 15:41
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 12, 2021
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
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
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