Improve React error message when mutable sources are mutated during r…

…ender (#20665)

Changed previous error message from:
> Cannot read from mutable source during the current render without tearing. This is a bug in React. Please file an issue.

To:
> Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue.

Also added a DEV only warning about the unsafe side effect:
> A mutable source was mutated while the %s component was rendering. This is not supported. Move any mutations into event handlers or effects.

I think this is the best we can do without adding production overhead that we'd probably prefer to avoid.

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

@@ -904,6 +904,18 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
  const getVersion = source._getVersion;
  const version = getVersion(source._source);
 
+ let mutableSourceSideEffectDetected = false;
+ if (__DEV__) {
+ // Detect side effects that update a mutable source during render.
+ // See https://github.com/facebook/react/issues/19948
+ if (source._currentlyRenderingFiber !== currentlyRenderingFiber) {
+ source._currentlyRenderingFiber = currentlyRenderingFiber;
+ source._initialVersionAsOfFirstRender = version;
+ } else if (source._initialVersionAsOfFirstRender !== version) {
+ mutableSourceSideEffectDetected = true;
+ }
+ }
+
  // Is it safe for this component to read from this source during the current render?
  let isSafeToReadFromSource = false;
 
@@ -966,9 +978,20 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
  // but there's nothing we can do about that (short of throwing here and refusing to continue the render).
  markSourceAsDirty(source);
 
+ if (__DEV__) {
+ if (mutableSourceSideEffectDetected) {
+ const componentName = getComponentName(currentlyRenderingFiber.type);
+ console.warn(
+ 'A mutable source was mutated while the %s component was rendering. This is not supported. ' +
+ 'Move any mutations into event handlers or effects.',
+ componentName,
+ );
+ }
+ }
+
  invariant(
  false,
- 'Cannot read from mutable source during the current render without tearing. This is a bug in React. Please file an issue.',
+ 'Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue.',
  );
  }
 }

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

@@ -885,6 +885,18 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
  const getVersion = source._getVersion;
  const version = getVersion(source._source);
 
+ let mutableSourceSideEffectDetected = false;
+ if (__DEV__) {
+ // Detect side effects that update a mutable source during render.
+ // See https://github.com/facebook/react/issues/19948
+ if (source._currentlyRenderingFiber !== currentlyRenderingFiber) {
+ source._currentlyRenderingFiber = currentlyRenderingFiber;
+ source._initialVersionAsOfFirstRender = version;
+ } else if (source._initialVersionAsOfFirstRender !== version) {
+ mutableSourceSideEffectDetected = true;
+ }
+ }
+
  // Is it safe for this component to read from this source during the current render?
  let isSafeToReadFromSource = false;
 
@@ -947,9 +959,20 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
  // but there's nothing we can do about that (short of throwing here and refusing to continue the render).
  markSourceAsDirty(source);
 
+ if (__DEV__) {
+ if (mutableSourceSideEffectDetected) {
+ const componentName = getComponentName(currentlyRenderingFiber.type);
+ console.warn(
+ 'A mutable source was mutated while the %s component was rendering. This is not supported. ' +
+ 'Move any mutations into event handlers or effects.',
+ componentName,
+ );
+ }
+ }
+
  invariant(
  false,
- 'Cannot read from mutable source during the current render without tearing. This is a bug in React. Please file an issue.',
+ 'Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue.',
  );
  }
 }

packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js

@@ -25,9 +25,9 @@ function loadModules() {
  jest.useFakeTimers();
 
  ReactFeatureFlags = require('shared/ReactFeatureFlags');
-
  ReactFeatureFlags.enableSchedulerTracing = true;
  ReactFeatureFlags.enableProfilerTimer = true;
+
  React = require('react');
  ReactNoop = require('react-noop-renderer');
  Scheduler = require('scheduler');
@@ -1720,6 +1720,84 @@ describe('useMutableSource', () => {
  });
 
  if (__DEV__) {
+ // See https://github.com/facebook/react/issues/19948
+ describe('side effecte detection', () => {
+ // @gate experimental
+ it('should throw if a mutable source is mutated during render', () => {
+ const source = createSource('initial');
+ const mutableSource = createMutableSource(
+ source,
+ param => param.version,
+ );
+
+ function MutateDuringRead() {
+ const value = useMutableSource(
+ mutableSource,
+ defaultGetSnapshot,
+ defaultSubscribe,
+ );
+ Scheduler.unstable_yieldValue('MutateDuringRead:' + value);
+ // Note that mutating an exeternal value during render is a side effect and is not supported.
+ if (value === 'initial') {
+ source.value = 'updated';
+ }
+ return null;
+ }
+
+ expect(() => {
+ expect(() => {
+ act(() => {
+ ReactNoop.renderLegacySyncRoot(
+ <React.StrictMode>
+ <MutateDuringRead />
+ </React.StrictMode>,
+ );
+ });
+ }).toThrow(
+ 'Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue.',
+ );
+ }).toWarnDev(
+ 'A mutable source was mutated while the MutateDuringRead component was rendering. This is not supported. ' +
+ 'Move any mutations into event handlers or effects.\n' +
+ ' in MutateDuringRead (at **)',
+ );
+
+ expect(Scheduler).toHaveYielded(['MutateDuringRead:initial']);
+ });
+
+ // @gate experimental
+ it('should not misidentify mutations after render as side effects', () => {
+ const source = createSource('initial');
+ const mutableSource = createMutableSource(
+ source,
+ param => param.version,
+ );
+
+ function MutateDuringRead() {
+ const value = useMutableSource(
+ mutableSource,
+ defaultGetSnapshot,
+ defaultSubscribe,
+ );
+ Scheduler.unstable_yieldValue('MutateDuringRead:' + value);
+ return null;
+ }
+
+ act(() => {
+ ReactNoop.renderLegacySyncRoot(
+ <React.StrictMode>
+ <MutateDuringRead />
+ </React.StrictMode>,
+ );
+ expect(Scheduler).toFlushAndYieldThrough([
+ 'MutateDuringRead:initial',
+ ]);
+ source.value = 'updated';
+ });
+ expect(Scheduler).toHaveYielded(['MutateDuringRead:updated']);
+ });
+ });
+
  describe('dev warnings', () => {
  // @gate experimental
  it('should warn if the subscribe function does not return an unsubscribe function', () => {

packages/react/src/ReactMutableSource.js

@@ -23,6 +23,11 @@ export function createMutableSource<Source: $NonMaybeType<mixed>>(
  if (__DEV__) {
  mutableSource._currentPrimaryRenderer = null;
  mutableSource._currentSecondaryRenderer = null;
+
+ // Used to detect side effects that update a mutable source during render.
+ // See https://github.com/facebook/react/issues/19948
+ mutableSource._currentlyRenderingFiber = null;
+ mutableSource._initialVersionAsOfFirstRender = null;
  }
 
  return mutableSource;

packages/shared/ReactTypes.js

@@ -197,6 +197,12 @@ export type MutableSource<Source: $NonMaybeType<mixed>> = {|
  // Used to detect multiple renderers using the same mutable source.
  _currentPrimaryRenderer?: Object | null,
  _currentSecondaryRenderer?: Object | null,
+
+ // DEV only
+ // Used to detect side effects that update a mutable source during render.
+ // See https://github.com/facebook/react/issues/19948
+ _currentlyRenderingFiber?: Fiber | null,
+ _initialVersionAsOfFirstRender?: MutableSourceVersion | null,
 |};
 
 // The subset of a Thenable required by things thrown by Suspense.

scripts/error-codes/codes.json

@@ -372,5 +372,6 @@
  "381": "This feature is not supported by ReactSuspenseTestUtils.",
  "382": "This query has received more parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.",
  "383": "This query has received fewer parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.",
- "384": "Refreshing the cache is not supported in Server Components."
+ "384": "Refreshing the cache is not supported in Server Components.",
+ "385": "Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue."
 }