Skip to content

Commit 6f52afb

Browse files
author
Brian Vaughn
committed
Warn if MutableSource snapshot is a function
1 parent 8dba669 commit 6f52afb

File tree

3 files changed

+66
-2
lines changed

3 files changed

+66
-2
lines changed

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,16 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
913913
}
914914

915915
if (isSafeToReadFromSource) {
916-
return getSnapshot(source._source);
916+
const snapshot = getSnapshot(source._source);
917+
if (__DEV__) {
918+
if (typeof snapshot === 'function') {
919+
console.warn(
920+
'Mutable source should not return a functions as the snapshot value. ' +
921+
'Functions may close over mutable values and cause tearing.',
922+
);
923+
}
924+
}
925+
return snapshot;
917926
} else {
918927
// This handles the special case of a mutable source being shared beween renderers.
919928
// In that case, if the source is mutated between the first and second renderer,
@@ -991,6 +1000,15 @@ function useMutableSource<Source, Snapshot>(
9911000
const maybeNewVersion = getVersion(source._source);
9921001
if (!is(version, maybeNewVersion)) {
9931002
const maybeNewSnapshot = getSnapshot(source._source);
1003+
if (__DEV__) {
1004+
if (typeof maybeNewSnapshot === 'function') {
1005+
console.warn(
1006+
'Mutable source should not return a functions as the snapshot value. ' +
1007+
'Functions may close over mutable values and cause tearing.',
1008+
);
1009+
}
1010+
}
1011+
9941012
if (!is(snapshot, maybeNewSnapshot)) {
9951013
setSnapshot(maybeNewSnapshot);
9961014

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,16 @@ function readFromUnsubcribedMutableSource<Source, Snapshot>(
897897
}
898898

899899
if (isSafeToReadFromSource) {
900-
return getSnapshot(source._source);
900+
const snapshot = getSnapshot(source._source);
901+
if (__DEV__) {
902+
if (typeof snapshot === 'function') {
903+
console.warn(
904+
'Mutable source should not return a functions as the snapshot value. ' +
905+
'Functions may close over mutable values and cause tearing.',
906+
);
907+
}
908+
}
909+
return snapshot;
901910
} else {
902911
// This handles the special case of a mutable source being shared beween renderers.
903912
// In that case, if the source is mutated between the first and second renderer,
@@ -975,6 +984,15 @@ function useMutableSource<Source, Snapshot>(
975984
const maybeNewVersion = getVersion(source._source);
976985
if (!is(version, maybeNewVersion)) {
977986
const maybeNewSnapshot = getSnapshot(source._source);
987+
if (__DEV__) {
988+
if (typeof maybeNewSnapshot === 'function') {
989+
console.warn(
990+
'Mutable source should not return a functions as the snapshot value. ' +
991+
'Functions may close over mutable values and cause tearing.',
992+
);
993+
}
994+
}
995+
978996
if (!is(snapshot, maybeNewSnapshot)) {
979997
setSnapshot(maybeNewSnapshot);
980998

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1493,6 +1493,34 @@ describe('useMutableSource', () => {
14931493
},
14941494
);
14951495

1496+
// @gate experimental
1497+
it('warns about functions being used as snapshot values', async () => {
1498+
const source = createSource(() => 'a');
1499+
const mutableSource = createMutableSource(source);
1500+
1501+
const getSnapshot = () => source.value;
1502+
1503+
function Read() {
1504+
const fn = useMutableSource(mutableSource, getSnapshot, defaultSubscribe);
1505+
const value = fn();
1506+
Scheduler.unstable_yieldValue(value);
1507+
return value;
1508+
}
1509+
1510+
const root = ReactNoop.createRoot();
1511+
await act(async () => {
1512+
root.render(
1513+
<>
1514+
<Read />
1515+
</>,
1516+
);
1517+
expect(() => expect(Scheduler).toFlushAndYield(['a'])).toWarnDev(
1518+
'Mutable source should not return a functions as the snapshot value.',
1519+
);
1520+
});
1521+
expect(root).toMatchRenderedOutput('a');
1522+
});
1523+
14961524
// @gate experimental
14971525
it('getSnapshot changes and then source is mutated during interleaved event', async () => {
14981526
const {useEffect} = React;

0 commit comments

Comments
 (0)