Improve React error message when mutable sources are mutated during render (#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.
Brian Vaughn committed
Jan 29, 2021 at 10:22 UTC
766a7a28a9a233b236a97166411a9242376df7a6
6 files changed
+140
-4
packages/react-reconciler/src/ReactFiberHooks.new.js
+24
-1
@@ -904,6 +904,18 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
904
const getVersion = source._getVersion;
905
const version = getVersion(source._source);
906
907
+ let mutableSourceSideEffectDetected = false;
908
+ if (__DEV__) {
909
+ // Detect side effects that update a mutable source during render.
910
+ // See https://github.com/facebook/react/issues/19948
911
+ if (source._currentlyRenderingFiber !== currentlyRenderingFiber) {
912
+ source._currentlyRenderingFiber = currentlyRenderingFiber;
913
+ source._initialVersionAsOfFirstRender = version;
914
+ } else if (source._initialVersionAsOfFirstRender !== version) {
915
+ mutableSourceSideEffectDetected = true;
916
+ }
917
+ }
918
+
919
// Is it safe for this component to read from this source during the current render?
920
let isSafeToReadFromSource = false;
921
@@ -966,9 +978,20 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
978
// but there's nothing we can do about that (short of throwing here and refusing to continue the render).
979
markSourceAsDirty(source);
980
981
+ if (__DEV__) {
982
+ if (mutableSourceSideEffectDetected) {
983
+ const componentName = getComponentName(currentlyRenderingFiber.type);
984
+ console.warn(
985
+ 'A mutable source was mutated while the %s component was rendering. This is not supported. ' +
986
+ 'Move any mutations into event handlers or effects.',
987
+ componentName,
988
+ );
989
+ }
990
+ }
991
+
992
invariant(
993
false,
971
- 'Cannot read from mutable source during the current render without tearing. This is a bug in React. Please file an issue.',
994
+ 'Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue.',
995
);
996
}
997
}
packages/react-reconciler/src/ReactFiberHooks.old.js
+24
-1
@@ -885,6 +885,18 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
885
const getVersion = source._getVersion;
886
const version = getVersion(source._source);
887
888
+ let mutableSourceSideEffectDetected = false;
889
+ if (__DEV__) {
890
+ // Detect side effects that update a mutable source during render.
891
+ // See https://github.com/facebook/react/issues/19948
892
+ if (source._currentlyRenderingFiber !== currentlyRenderingFiber) {
893
+ source._currentlyRenderingFiber = currentlyRenderingFiber;
894
+ source._initialVersionAsOfFirstRender = version;
895
+ } else if (source._initialVersionAsOfFirstRender !== version) {
896
+ mutableSourceSideEffectDetected = true;
897
+ }
898
+ }
899
+
900
// Is it safe for this component to read from this source during the current render?
901
let isSafeToReadFromSource = false;
902
@@ -947,9 +959,20 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
959
// but there's nothing we can do about that (short of throwing here and refusing to continue the render).
960
markSourceAsDirty(source);
961
962
+ if (__DEV__) {
963
+ if (mutableSourceSideEffectDetected) {
964
+ const componentName = getComponentName(currentlyRenderingFiber.type);
965
+ console.warn(
966
+ 'A mutable source was mutated while the %s component was rendering. This is not supported. ' +
967
+ 'Move any mutations into event handlers or effects.',
968
+ componentName,
969
+ );
970
+ }
971
+ }
972
+
973
invariant(
974
false,
952
- 'Cannot read from mutable source during the current render without tearing. This is a bug in React. Please file an issue.',
975
+ 'Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue.',
976
);
977
}
978
}
packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js
+79
-1
@@ -25,9 +25,9 @@ function loadModules() {
25
jest.useFakeTimers();
26
27
ReactFeatureFlags = require('shared/ReactFeatureFlags');
28
-
28
ReactFeatureFlags.enableSchedulerTracing = true;
29
ReactFeatureFlags.enableProfilerTimer = true;
30
+
31
React = require('react');
32
ReactNoop = require('react-noop-renderer');
33
Scheduler = require('scheduler');
@@ -1720,6 +1720,84 @@ describe('useMutableSource', () => {
1720
});
1721
1722
if (__DEV__) {
1723
+ // See https://github.com/facebook/react/issues/19948
1724
+ describe('side effecte detection', () => {
1725
+ // @gate experimental
1726
+ it('should throw if a mutable source is mutated during render', () => {
1727
+ const source = createSource('initial');
1728
+ const mutableSource = createMutableSource(
1729
+ source,
1730
+ param => param.version,
1731
+ );
1732
+
1733
+ function MutateDuringRead() {
1734
+ const value = useMutableSource(
1735
+ mutableSource,
1736
+ defaultGetSnapshot,
1737
+ defaultSubscribe,
1738
+ );
1739
+ Scheduler.unstable_yieldValue('MutateDuringRead:' + value);
1740
+ // Note that mutating an exeternal value during render is a side effect and is not supported.
1741
+ if (value === 'initial') {
1742
+ source.value = 'updated';
1743
+ }
1744
+ return null;
1745
+ }
1746
+
1747
+ expect(() => {
1748
+ expect(() => {
1749
+ act(() => {
1750
+ ReactNoop.renderLegacySyncRoot(
1751
+ <React.StrictMode>
1752
+ <MutateDuringRead />
1753
+ </React.StrictMode>,
1754
+ );
1755
+ });
1756
+ }).toThrow(
1757
+ 'Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue.',
1758
+ );
1759
+ }).toWarnDev(
1760
+ 'A mutable source was mutated while the MutateDuringRead component was rendering. This is not supported. ' +
1761
+ 'Move any mutations into event handlers or effects.\n' +
1762
+ ' in MutateDuringRead (at **)',
1763
+ );
1764
+
1765
+ expect(Scheduler).toHaveYielded(['MutateDuringRead:initial']);
1766
+ });
1767
+
1768
+ // @gate experimental
1769
+ it('should not misidentify mutations after render as side effects', () => {
1770
+ const source = createSource('initial');
1771
+ const mutableSource = createMutableSource(
1772
+ source,
1773
+ param => param.version,
1774
+ );
1775
+
1776
+ function MutateDuringRead() {
1777
+ const value = useMutableSource(
1778
+ mutableSource,
1779
+ defaultGetSnapshot,
1780
+ defaultSubscribe,
1781
+ );
1782
+ Scheduler.unstable_yieldValue('MutateDuringRead:' + value);
1783
+ return null;
1784
+ }
1785
+
1786
+ act(() => {
1787
+ ReactNoop.renderLegacySyncRoot(
1788
+ <React.StrictMode>
1789
+ <MutateDuringRead />
1790
+ </React.StrictMode>,
1791
+ );
1792
+ expect(Scheduler).toFlushAndYieldThrough([
1793
+ 'MutateDuringRead:initial',
1794
+ ]);
1795
+ source.value = 'updated';
1796
+ });
1797
+ expect(Scheduler).toHaveYielded(['MutateDuringRead:updated']);
1798
+ });
1799
+ });
1800
+
1801
describe('dev warnings', () => {
1802
// @gate experimental
1803
it('should warn if the subscribe function does not return an unsubscribe function', () => {
packages/react/src/ReactMutableSource.js
+5
@@ -23,6 +23,11 @@ export function createMutableSource<Source: $NonMaybeType<mixed>>(
23
if (__DEV__) {
24
mutableSource._currentPrimaryRenderer = null;
25
mutableSource._currentSecondaryRenderer = null;
26
+
27
+ // Used to detect side effects that update a mutable source during render.
28
+ // See https://github.com/facebook/react/issues/19948
29
+ mutableSource._currentlyRenderingFiber = null;
30
+ mutableSource._initialVersionAsOfFirstRender = null;
31
}
32
33
return mutableSource;
packages/shared/ReactTypes.js
+6
@@ -197,6 +197,12 @@ export type MutableSource<Source: $NonMaybeType<mixed>> = {|
197
// Used to detect multiple renderers using the same mutable source.
198
_currentPrimaryRenderer?: Object | null,
199
_currentSecondaryRenderer?: Object | null,
200
+
201
+ // DEV only
202
+ // Used to detect side effects that update a mutable source during render.
203
+ // See https://github.com/facebook/react/issues/19948
204
+ _currentlyRenderingFiber?: Fiber | null,
205
+ _initialVersionAsOfFirstRender?: MutableSourceVersion | null,
206
|};
207
208
// The subset of a Thenable required by things thrown by Suspense.
scripts/error-codes/codes.json
+2
-1
@@ -372,5 +372,6 @@
372
"381": "This feature is not supported by ReactSuspenseTestUtils.",
373
"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.",
374
"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.",
375
- "384": "Refreshing the cache is not supported in Server Components."
375
+ "384": "Refreshing the cache is not supported in Server Components.",
376
+ "385": "Cannot read from mutable source during the current render without tearing. This may be a bug in React. Please file an issue."
377
}