Skip to content

Commit

Permalink
Test for suspending with modern strict mode (facebook#28513)
Browse files Browse the repository at this point in the history
## Overview

Adds a test to show the cause of an infinite loop in Relay related to
[these effects in
Relay](https://github.com/facebook/relay/blob/448aa67d2a11e7d45cd7b4492b9f599b498cb39e/packages/react-relay/relay-hooks/useLazyLoadQueryNode.js#L77-L104)
and `useModernStrictMode`. The bug is related to effect behavior when
committing trees that re-suspend after initial mount.

With `useModernStrictEffect`, when you:
- initial mount
- update
- suspend (to fallbacks)
- resolve
- re-commit

We fire strict effects during the second mount, like it's a new tree.
This creates weird cases, where if there was an update while we
suspended, we'll first fire only the effects that changed dependencies,
and then fire strict effects.

Creating a test to demonstrate the behavior to see if it's a bug.
  • Loading branch information
rickhanlonii authored and AndyPengc12 committed Apr 15, 2024
1 parent 4184b80 commit 08682f0
Showing 1 changed file with 173 additions and 0 deletions.
173 changes: 173 additions & 0 deletions packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -772,4 +772,177 @@ describe('StrictEffectsMode', () => {
'useEffect unmount',
]);
});

// @gate __DEV__
it('should double invoke effects after a re-suspend', async () => {
// Not using Scheduler.log because it silences double render logs.
let log = [];
let shouldSuspend = true;
let resolve;
const suspensePromise = new Promise(_resolve => {
resolve = _resolve;
});
function Fallback() {
log.push('Fallback');
return 'Loading';
}

function Parent({prop}) {
log.push('Parent rendered');

React.useEffect(() => {
log.push('Parent create');
return () => {
log.push('Parent destroy');
};
}, []);

React.useEffect(() => {
log.push('Parent dep create');
return () => {
log.push('Parent dep destroy');
};
}, [prop]);

return (
<React.Suspense fallback={<Fallback />}>
<Child prop={prop} />
</React.Suspense>
);
}

function Child({prop}) {
const [count, forceUpdate] = React.useState(0);
const ref = React.useRef(null);
log.push('Child rendered');
React.useEffect(() => {
log.push('Child create');
return () => {
log.push('Child destroy');
ref.current = true;
};
}, []);
const key = `${prop}-${count}`;
React.useEffect(() => {
log.push('Child dep create');
if (ref.current === true) {
ref.current = false;
forceUpdate(c => c + 1);
log.push('-----------------------after setState');
return;
}

return () => {
log.push('Child dep destroy');
};
}, [key]);

if (shouldSuspend) {
log.push('Child suspended');
throw suspensePromise;
}
return null;
}

// Initial mount
shouldSuspend = false;
await act(() => {
ReactNoop.render(
<React.StrictMode>
<Parent />
</React.StrictMode>,
);
});

// Now re-suspend
shouldSuspend = true;
log = [];
await act(() => {
ReactNoop.render(
<React.StrictMode>
<Parent />
</React.StrictMode>,
);
});

// while suspended, update
log.push('-----------------------after update');
await act(() => {
ReactNoop.render(
<React.StrictMode>
<Parent prop={'bar'} />
</React.StrictMode>,
);
});

// Now resolve and commit
log.push('-----------------------after suspense');

await act(() => {
resolve();
shouldSuspend = false;
});

if (gate(flags => flags.useModernStrictMode)) {
expect(log).toEqual([
'Parent rendered',
'Parent rendered',
'Child rendered',
'Child suspended',
'Fallback',
'Fallback',
'-----------------------after update',
'Parent rendered',
'Parent rendered',
'Child rendered',
'Child suspended',
'Fallback',
'Fallback',
'Parent dep destroy',
'Parent dep create',
'-----------------------after suspense',
'Child rendered',
'Child rendered',
// !!! Committed, destroy and create effect.
// !!! The other effect is not destroyed and created
// !!! because the dep didn't change
'Child dep destroy',
'Child dep create',

// Double invoke both effects
'Child destroy',
'Child dep destroy',
'Child create',
'Child dep create',
// Fires setState
'-----------------------after setState',
'Child rendered',
'Child rendered',
'Child dep create',
]);
} else {
expect(log).toEqual([
'Parent rendered',
'Parent rendered',
'Child rendered',
'Child suspended',
'Fallback',
'Fallback',
'-----------------------after update',
'Parent rendered',
'Parent rendered',
'Child rendered',
'Child suspended',
'Fallback',
'Fallback',
'Parent dep destroy',
'Parent dep create',
'-----------------------after suspense',
'Child rendered',
'Child rendered',
'Child dep destroy',
'Child dep create',
]);
}
});
});

0 comments on commit 08682f0

Please sign in to comment.