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.

Sign up for GitHub

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

Jump to bottom

fix: useSyncExternalStoreExtra #22500

Merged
merged 4 commits into from
Oct 4, 2021
Merged

fix: useSyncExternalStoreExtra #22500

merged 4 commits into from
Oct 4, 2021

Conversation

dai-shi
Copy link
Contributor

@dai-shi dai-shi commented Oct 4, 2021 •
edited
Loading

Summary

Discussion from: reactwg/react-18#86 (comment)

This change is to fix the case when selector and isEqual throw.

  • initial fix to see if CI passes
  • add failing test
  • finalize PR

How did you test this change?

Test added.

@facebook-github-bot
Copy link

Hi @dai-shi!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@sizebot
Copy link

sizebot commented Oct 4, 2021 •
edited
Loading

Comparing: a4bc8ae...f3eecd8

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 = 130.05 kB 130.05 kB = 41.34 kB 41.34 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 132.88 kB 132.88 kB = 42.31 kB 42.30 kB
facebook-www/ReactDOM-prod.classic.js = 412.54 kB 412.54 kB = 76.28 kB 76.28 kB
facebook-www/ReactDOM-prod.modern.js = 401.13 kB 401.13 kB = 74.56 kB 74.56 kB
facebook-www/ReactDOMForked-prod.classic.js = 412.54 kB 412.54 kB = 76.28 kB 76.28 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store-extra.production.min.js +2.36% 1.06 kB 1.09 kB = 0.60 kB 0.60 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store-extra.production.min.js +2.36% 1.06 kB 1.09 kB = 0.60 kB 0.60 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store-extra.production.min.js +2.36% 1.06 kB 1.09 kB = 0.60 kB 0.60 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-extra.production.min.js +2.36% 1.06 kB 1.09 kB = 0.60 kB 0.60 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store-extra.production.min.js +2.36% 1.06 kB 1.09 kB = 0.60 kB 0.60 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store-extra.production.min.js +2.36% 1.06 kB 1.09 kB = 0.60 kB 0.60 kB

Generated by 🚫 dangerJS against f3eecd8

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@dai-shi dai-shi marked this pull request as ready for review October 4, 2021 07:27
@Andarist
Copy link
Contributor

Andarist commented Oct 4, 2021

As a distant spectator: great job @dai-shi at debugging this, fixing and preparing your first PR to React 🎉

@acdlite
Copy link
Collaborator

acdlite commented Oct 4, 2021

Great PR, thank you!

@acdlite acdlite merged commit f2c3811 into facebook:main Oct 4, 2021
@dai-shi dai-shi deleted the fix-uses-extra branch October 4, 2021 14:46
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 12, 2021
@facebook-github-bot
React Native sync for revisions e8feb11...afcb9cd
c75ad2c 
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
@dai-shi @zhengjitf
fix: useSyncExternalStoreExtra (facebook#22500)
d1088fe 
* move setting memoizedSnapshot

* Revert "move setting memoizedSnapshot"

This reverts commit f013206.

* add failed tests

* Revert "Revert "move setting memoizedSnapshot""

This reverts commit cb43c4f.
@jellevoost jellevoost mentioned this pull request Jan 4, 2023
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crypt096 crypt096 crypt096 approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dai-shi @facebook-github-bot @sizebot @Andarist @acdlite @crypt096