fix getSnapshot warning when a selector returns NaN

[hachibeeDI]

• cla
• lint
• prettier
• test
  (but issue remains)

Summary

When I use a useSyncExternalStoreWithSelector with selecter which possibly returns NaN, I received a warning says "The result of getSnapshot should be cached to avoid an infinite loop" despite it was cached.

        const selectedNaN = useSyncExternalStoreWithSelector(
          store.subscribe,
          store.getState,
          null,
          s => parseInt(s.nan, 10),
        );

How did you test this change?

I added a unit test to src/packges/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js.

One more thing I noticed

While I was testing the code, the unit test doesn't use the functions declared in useSyncExternalStoreShimClient.js but packages/react-reconciler/src/ReactFiberHooks.new.js.

I guess it was due to that the shim could be bypassed if the React has a native version of the function.
I fixed both but a former does not have any unit test as it was (I checked it in my local env).

Thanks!

[sizebot]

Comparing: 40eaa22...35a7af8

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.97 kB	130.97 kB	=	41.94 kB	41.94 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	135.84 kB	135.84 kB	=	43.38 kB	43.38 kB
facebook-www/ReactDOM-prod.classic.js	=	432.04 kB	432.04 kB	=	79.17 kB	79.17 kB
facebook-www/ReactDOM-prod.modern.js	=	421.81 kB	421.81 kB	=	77.70 kB	77.70 kB
facebook-www/ReactDOMForked-prod.classic.js	=	432.04 kB	432.04 kB	=	79.17 kB	79.17 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-shim.native.development.js	+0.59%	7.79 kB	7.83 kB	+0.26%	3.08 kB	3.09 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store-shim.native.development.js	+0.59%	7.79 kB	7.83 kB	+0.26%	3.08 kB	3.09 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store-shim.native.development.js	+0.59%	7.79 kB	7.83 kB	+0.26%	3.08 kB	3.09 kB
oss-experimental/use-sync-external-store/cjs/use-sync-external-store-shim.development.js	+0.55%	8.37 kB	8.42 kB	+0.28%	3.18 kB	3.19 kB
oss-stable-semver/use-sync-external-store/cjs/use-sync-external-store-shim.development.js	+0.55%	8.37 kB	8.42 kB	+0.28%	3.18 kB	3.19 kB
oss-stable/use-sync-external-store/cjs/use-sync-external-store-shim.development.js	+0.55%	8.37 kB	8.42 kB	+0.28%	3.18 kB	3.19 kB

Generated by 🚫 dangerJS against 35a7af8

[acdlite]

While I was testing the code, the unit test doesn't use the functions declared in useSyncExternalStoreShimClient.js but packages/react-reconciler/src/ReactFiberHooks.new.js.

I guess it was due to that the shim could be bypassed if the React has a native version of the function.
I fixed both but a former does not have any unit test as it was (I checked it in my local env).

We have a feature flag system that runs the tests in different configurations. You can test the shimmed version by passing the --no-variant option to the test command:

# Runs using built-in useSyncExternalStore implementation
yarn test

# Runs using shimmed implementation
yarn test --no-variant

Both variants run automatically in CI.

Your changes look good. (I pushed some minor nits.) Thanks for submitting your first React PR!

[hachibeeDI]

Thanks!