Skip to content

Commit 67e6fa6

Browse files
authored
Test for suspending with modern strict mode (#28513)
## 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.
1 parent d303740 commit 67e6fa6

File tree

1 file changed

+173
-0
lines changed

1 file changed

+173
-0
lines changed

packages/react-reconciler/src/__tests__/StrictEffectsMode-test.js

+173
Original file line numberDiff line numberDiff line change
@@ -772,4 +772,177 @@ describe('StrictEffectsMode', () => {
772772
'useEffect unmount',
773773
]);
774774
});
775+
776+
// @gate __DEV__
777+
it('should double invoke effects after a re-suspend', async () => {
778+
// Not using Scheduler.log because it silences double render logs.
779+
let log = [];
780+
let shouldSuspend = true;
781+
let resolve;
782+
const suspensePromise = new Promise(_resolve => {
783+
resolve = _resolve;
784+
});
785+
function Fallback() {
786+
log.push('Fallback');
787+
return 'Loading';
788+
}
789+
790+
function Parent({prop}) {
791+
log.push('Parent rendered');
792+
793+
React.useEffect(() => {
794+
log.push('Parent create');
795+
return () => {
796+
log.push('Parent destroy');
797+
};
798+
}, []);
799+
800+
React.useEffect(() => {
801+
log.push('Parent dep create');
802+
return () => {
803+
log.push('Parent dep destroy');
804+
};
805+
}, [prop]);
806+
807+
return (
808+
<React.Suspense fallback={<Fallback />}>
809+
<Child prop={prop} />
810+
</React.Suspense>
811+
);
812+
}
813+
814+
function Child({prop}) {
815+
const [count, forceUpdate] = React.useState(0);
816+
const ref = React.useRef(null);
817+
log.push('Child rendered');
818+
React.useEffect(() => {
819+
log.push('Child create');
820+
return () => {
821+
log.push('Child destroy');
822+
ref.current = true;
823+
};
824+
}, []);
825+
const key = `${prop}-${count}`;
826+
React.useEffect(() => {
827+
log.push('Child dep create');
828+
if (ref.current === true) {
829+
ref.current = false;
830+
forceUpdate(c => c + 1);
831+
log.push('-----------------------after setState');
832+
return;
833+
}
834+
835+
return () => {
836+
log.push('Child dep destroy');
837+
};
838+
}, [key]);
839+
840+
if (shouldSuspend) {
841+
log.push('Child suspended');
842+
throw suspensePromise;
843+
}
844+
return null;
845+
}
846+
847+
// Initial mount
848+
shouldSuspend = false;
849+
await act(() => {
850+
ReactNoop.render(
851+
<React.StrictMode>
852+
<Parent />
853+
</React.StrictMode>,
854+
);
855+
});
856+
857+
// Now re-suspend
858+
shouldSuspend = true;
859+
log = [];
860+
await act(() => {
861+
ReactNoop.render(
862+
<React.StrictMode>
863+
<Parent />
864+
</React.StrictMode>,
865+
);
866+
});
867+
868+
// while suspended, update
869+
log.push('-----------------------after update');
870+
await act(() => {
871+
ReactNoop.render(
872+
<React.StrictMode>
873+
<Parent prop={'bar'} />
874+
</React.StrictMode>,
875+
);
876+
});
877+
878+
// Now resolve and commit
879+
log.push('-----------------------after suspense');
880+
881+
await act(() => {
882+
resolve();
883+
shouldSuspend = false;
884+
});
885+
886+
if (gate(flags => flags.useModernStrictMode)) {
887+
expect(log).toEqual([
888+
'Parent rendered',
889+
'Parent rendered',
890+
'Child rendered',
891+
'Child suspended',
892+
'Fallback',
893+
'Fallback',
894+
'-----------------------after update',
895+
'Parent rendered',
896+
'Parent rendered',
897+
'Child rendered',
898+
'Child suspended',
899+
'Fallback',
900+
'Fallback',
901+
'Parent dep destroy',
902+
'Parent dep create',
903+
'-----------------------after suspense',
904+
'Child rendered',
905+
'Child rendered',
906+
// !!! Committed, destroy and create effect.
907+
// !!! The other effect is not destroyed and created
908+
// !!! because the dep didn't change
909+
'Child dep destroy',
910+
'Child dep create',
911+
912+
// Double invoke both effects
913+
'Child destroy',
914+
'Child dep destroy',
915+
'Child create',
916+
'Child dep create',
917+
// Fires setState
918+
'-----------------------after setState',
919+
'Child rendered',
920+
'Child rendered',
921+
'Child dep create',
922+
]);
923+
} else {
924+
expect(log).toEqual([
925+
'Parent rendered',
926+
'Parent rendered',
927+
'Child rendered',
928+
'Child suspended',
929+
'Fallback',
930+
'Fallback',
931+
'-----------------------after update',
932+
'Parent rendered',
933+
'Parent rendered',
934+
'Child rendered',
935+
'Child suspended',
936+
'Fallback',
937+
'Fallback',
938+
'Parent dep destroy',
939+
'Parent dep create',
940+
'-----------------------after suspense',
941+
'Child rendered',
942+
'Child rendered',
943+
'Child dep destroy',
944+
'Child dep create',
945+
]);
946+
}
947+
});
775948
});

0 commit comments

Comments
 (0)