Skip to content

Commit cde16b5

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 cde16b5

File tree

1 file changed

+34
-0
lines changed

1 file changed

+34
-0
lines changed

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

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

0 commit comments

Comments
 (0)