Point useSubscription to useSyncExternalStore shim

[gaearon]

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.

[sizebot]

Comparing: 4bc465a...41c3664

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	=	131.40 kB	131.40 kB	=	41.98 kB	41.98 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	136.45 kB	136.45 kB	=	43.43 kB	43.43 kB
facebook-www/ReactDOM-prod.classic.js	=	433.07 kB	433.07 kB	=	79.60 kB	79.60 kB
facebook-www/ReactDOM-prod.modern.js	=	418.07 kB	418.07 kB	=	77.24 kB	77.24 kB
facebook-www/ReactDOMForked-prod.classic.js	=	433.07 kB	433.07 kB	=	79.61 kB	79.61 kB
oss-experimental/use-subscription/cjs/use-subscription.production.min.js	=	0.80 kB	0.41 kB	=	0.44 kB	0.29 kB
oss-stable-semver/use-subscription/cjs/use-subscription.production.min.js	=	0.80 kB	0.41 kB	=	0.44 kB	0.29 kB
oss-stable/use-subscription/cjs/use-subscription.production.min.js	=	0.80 kB	0.41 kB	=	0.44 kB	0.29 kB
oss-experimental/use-subscription/cjs/use-subscription.development.js	=	5.61 kB	1.50 kB	=	2.08 kB	0.66 kB
oss-stable-semver/use-subscription/cjs/use-subscription.development.js	=	5.61 kB	1.50 kB	=	2.08 kB	0.66 kB
oss-stable/use-subscription/cjs/use-subscription.development.js	=	5.61 kB	1.50 kB	=	2.08 kB	0.66 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-subscription/cjs/use-subscription.production.min.js	=	0.80 kB	0.41 kB	=	0.44 kB	0.29 kB
oss-stable-semver/use-subscription/cjs/use-subscription.production.min.js	=	0.80 kB	0.41 kB	=	0.44 kB	0.29 kB
oss-stable/use-subscription/cjs/use-subscription.production.min.js	=	0.80 kB	0.41 kB	=	0.44 kB	0.29 kB
oss-experimental/use-subscription/cjs/use-subscription.development.js	=	5.61 kB	1.50 kB	=	2.08 kB	0.66 kB
oss-stable-semver/use-subscription/cjs/use-subscription.development.js	=	5.61 kB	1.50 kB	=	2.08 kB	0.66 kB
oss-stable/use-subscription/cjs/use-subscription.development.js	=	5.61 kB	1.50 kB	=	2.08 kB	0.66 kB

Generated by 🚫 dangerJS against 41c3664

[rickhanlonii]

@gaearon the failures are because useSubscription schedules updates from mutations inside startTransition as transitions, but useSyncExternalStore forces a sync render even when inside startTransition.

Specifically, useSubscription schedules the update here (so if startTransition is anywhere on the stack, this update will be a transition):

react/packages/use-subscription/src/useSubscription.js

Lines 88 to 108 in d5f1b06

	setState(prevState => {
	// Ignore values from stale sources!
	// Since we subscribe an unsubscribe in a passive effect,
	// it's possible that this callback will be invoked for a stale (previous) subscription.
	// This check avoids scheduling an update for that stale subscription.
	if (
	prevState.getCurrentValue !== getCurrentValue ||
	prevState.subscribe !== subscribe
	) {
	return prevState;
	}
	// Some subscriptions will auto-invoke the handler, even if the value hasn't changed.
	// If the value hasn't changed, no update is needed.
	// Return state as-is so React can bail out and avoid an unnecessary render.
	if (prevState.value === value) {
	return prevState;
	}
	return {...prevState, value};
	});

And useSyncExternalStore schedules it here:

react/packages/react-reconciler/src/ReactFiberHooks.new.js

Lines 1477 to 1480 in d5f1b06

	if (checkIfSnapshotChanged(inst)) {
	// Force a re-render.
	forceStoreRerender(fiber);
	}

Which schedules a sync lane update here (instead of calling ensureRootIsScheduled which would check the priority):

react/packages/react-reconciler/src/ReactFiberHooks.new.js

Lines 1497 to 1499 in d5f1b06

	function forceStoreRerender(fiber) {
	scheduleUpdateOnFiber(fiber, SyncLane, NoTimestamp);
	}

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, toFlushAndYieldThrough is going to show both of the updates because there's no yield between.

[gaearon]

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.

[rickhanlonii]

Yes, I think that's fair. The test don't fail because of broken behavior, but different semantics and the new semantics are better.