diff --git a/packages/use-subscription/src/__tests__/useSubscription-test.internal.js b/packages/use-subscription/src/__tests__/useSubscription-test.internal.js index daacfb5cfe57..891a80ceb31d 100644 --- a/packages/use-subscription/src/__tests__/useSubscription-test.internal.js +++ b/packages/use-subscription/src/__tests__/useSubscription-test.internal.js @@ -22,6 +22,9 @@ describe('useSubscription', () => { jest.resetModules(); jest.mock('scheduler', () => require('scheduler/unstable_mock')); + const ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; + useSubscription = require('use-subscription').useSubscription; React = require('react'); ReactTestRenderer = require('react-test-renderer'); @@ -560,4 +563,65 @@ describe('useSubscription', () => { act(() => renderer.update()); Scheduler.unstable_flushAll(); }); + + it('should not tear if a mutation occurs during a concurrent update', () => { + const input = document.createElement('input'); + + const mutate = value => { + input.value = value; + input.dispatchEvent(new Event('change')); + }; + + const subscription = { + getCurrentValue: () => input.value, + subscribe: callback => { + input.addEventListener('change', callback); + return () => input.removeEventListener('change', callback); + }, + }; + + const Subscriber = ({id}) => { + const value = useSubscription(subscription); + Scheduler.unstable_yieldValue(`render:${id}:${value}`); + return value; + }; + + act(() => { + // Initial render of "A" + mutate('A'); + ReactTestRenderer.create( + + + + , + {unstable_isConcurrent: true}, + ); + expect(Scheduler).toFlushAndYield(['render:first:A', 'render:second:A']); + + // Update state "A" -> "B" + // This update will be eagerly evaluated, + // so the tearing case this test is guarding against would not happen. + mutate('B'); + expect(Scheduler).toFlushAndYield(['render:first:B', 'render:second:B']); + + // No more pending updates + jest.runAllTimers(); + + // Partial update "B" -> "C" + // Interrupt with a second mutation "C" -> "D". + // This update will not be eagerly evaluated, + // but useSubscription() should eagerly close over the updated value to avoid tearing. + mutate('C'); + expect(Scheduler).toFlushAndYieldThrough(['render:first:C']); + mutate('D'); + expect(Scheduler).toFlushAndYield([ + 'render:second:C', + 'render:first:D', + 'render:second:D', + ]); + + // No more pending updates + jest.runAllTimers(); + }); + }); }); diff --git a/packages/use-subscription/src/useSubscription.js b/packages/use-subscription/src/useSubscription.js index 1bb80e2d7b13..5d7b6500df99 100644 --- a/packages/use-subscription/src/useSubscription.js +++ b/packages/use-subscription/src/useSubscription.js @@ -80,6 +80,12 @@ export function useSubscription({ return; } + // We use a state updater function to avoid scheduling work for a stale source. + // However it's important to eagerly read the currently value, + // so that all scheduled work shares the same value (in the event of multiple subscriptions). + // This avoids visual "tearing" when a mutation happens during a (concurrent) render. + const value = getCurrentValue(); + setState(prevState => { // Ignore values from stale sources! // Since we subscribe an unsubscribe in a passive effect, @@ -95,7 +101,6 @@ export function useSubscription({ // 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. - const value = getCurrentValue(); if (prevState.value === value) { return prevState; }