Skip to content

Commit 082f49f

Browse files
committed
Add test that triggers infinite update loop
In 18, passive effects are flushed synchronously if they are the result of a synchronous update. We have a guard for infinite update loops that occur in the layout phase, but it doesn't currently work for synchronous updates from a passive effect. The reason this probably hasn't come up yet is because synchronous updates inside the passive effect phase are relatively rare: you either have to imperatively dispatch a discrete event, like `el.focus`, or you have to call `ReactDOM.flushSync`, which triggers a warning. (In general, updates inside a passive effect are not encouraged.) I discovered this because `useSyncExternalStore` does sometimes trigger updates inside the passive effect phase. This commit adds a failing test to prove the issue exists. I will fix it in the next commit.
1 parent 4ce89a5 commit 082f49f

File tree

1 file changed

+32
-0
lines changed

1 file changed

+32
-0
lines changed

packages/react-dom/src/__tests__/ReactUpdates-test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,6 +1594,7 @@ describe('ReactUpdates', () => {
15941594
});
15951595
});
15961596

1597+
// TODO: Replace this branch with @gate pragmas
15971598
if (__DEV__) {
15981599
it('warns about a deferred infinite update loop with useEffect', () => {
15991600
function NonTerminating() {
@@ -1684,4 +1685,35 @@ describe('ReactUpdates', () => {
16841685
expect(container.textContent).toBe('1000');
16851686
});
16861687
}
1688+
1689+
it('prevents infinite update loop triggered by synchronous updates in useEffect', () => {
1690+
// Ignore flushSync warning
1691+
spyOnDev(console, 'error');
1692+
1693+
function NonTerminating() {
1694+
const [step, setStep] = React.useState(0);
1695+
React.useEffect(() => {
1696+
// Other examples of synchronous updates in useEffect are imperative
1697+
// event dispatches like `el.focus`, or `useSyncExternalStore`, which
1698+
// may schedule a synchronous update upon subscribing if it detects
1699+
// that the store has been mutated since the initial render.
1700+
//
1701+
// (Originally I wrote this test using `el.focus` but those errors
1702+
// get dispatched in a JSDOM event and I don't know how to "catch" those
1703+
// so that they don't fail the test.)
1704+
ReactDOM.flushSync(() => {
1705+
setStep(step + 1);
1706+
});
1707+
}, [step]);
1708+
return step;
1709+
}
1710+
1711+
const container = document.createElement('div');
1712+
const root = ReactDOM.createRoot(container);
1713+
expect(() => {
1714+
ReactDOM.flushSync(() => {
1715+
root.render(<NonTerminating />);
1716+
});
1717+
}).toThrow('Maximum update depth exceeded');
1718+
});
16871719
});

0 commit comments

Comments
 (0)