Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to disable double useEffect in legacy strict mode #26914

Merged
merged 3 commits into from
Jun 21, 2023
Merged

Add an option to disable double useEffect in legacy strict mode #26914

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20b0c01
Add an option in StrictMode to disable double useEffect in legacy str…
tyao1 Jun 9, 2023
10ce45a
Rename to DO_NOT_USE_disableStrictPassiveEffect
tyao1 Jun 20, 2023
659af98
Add a feature flag to avoid leaking
tyao1 Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add an option in StrictMode to disable double useEffect in legacy str… 
…ict mode
  • Loading branch information
@tyao1
tyao1 committed Jun 9, 2023
commit 20b0c0139f0569c71d52815bf6efd52a8a6e7027
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import {
StrictLegacyMode,
StrictEffectsMode,
ConcurrentUpdatesByDefaultMode,
NoStrictPassiveEffectsMode,
} from './ReactTypeOfMode';
import {
REACT_FORWARD_REF_TYPE,
Expand Down Expand Up @@ -539,6 +540,9 @@ export function createFiberFromTypeAndProps(
if ((mode & ConcurrentMode) !== NoMode) {
// Strict effects should never run on legacy roots
mode |= StrictEffectsMode;
if (pendingProps.unstable_disableStrictPassiveEffect) {
mode |= NoStrictPassiveEffectsMode;
}
}
break;
case REACT_PROFILER_TYPE:
Expand Down Expand Up @@ -752,6 +756,10 @@ export function createFiberFromOffscreen(
lanes: Lanes,
key: null | string,
): Fiber {
if (__DEV__) {
// StrictMode in Offscreen should always run double passive effects
mode &= ~NoStrictPassiveEffectsMode;
}
const fiber = createFiber(OffscreenComponent, pendingProps, key, mode);
fiber.elementType = REACT_OFFSCREEN_TYPE;
fiber.lanes = lanes;
Expand Down
4 changes: 3 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
DebugTracingMode,
StrictEffectsMode,
StrictLegacyMode,
NoStrictPassiveEffectsMode,
} from './ReactTypeOfMode';
import {
NoLane,
Expand Down Expand Up @@ -2257,7 +2258,8 @@ function mountEffect(
): void {
if (
__DEV__ &&
(currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode
(currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode &&
(currentlyRenderingFiber.mode & NoStrictPassiveEffectsMode) === NoMode
) {
mountEffectImpl(
MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect,
Expand Down
15 changes: 8 additions & 7 deletions packages/react-reconciler/src/ReactTypeOfMode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@

export type TypeOfMode = number;

export const NoMode = /* */ 0b000000;
export const NoMode = /* */ 0b0000000;
// TODO: Remove ConcurrentMode by reading from the root tag instead
export const ConcurrentMode = /* */ 0b000001;
export const ProfileMode = /* */ 0b000010;
export const DebugTracingMode = /* */ 0b000100;
export const StrictLegacyMode = /* */ 0b001000;
export const StrictEffectsMode = /* */ 0b010000;
export const ConcurrentUpdatesByDefaultMode = /* */ 0b100000;
export const ConcurrentMode = /* */ 0b0000001;
export const ProfileMode = /* */ 0b0000010;
export const DebugTracingMode = /* */ 0b0000100;
export const StrictLegacyMode = /* */ 0b0001000;
export const StrictEffectsMode = /* */ 0b0010000;
export const ConcurrentUpdatesByDefaultMode = /* */ 0b0100000;
export const NoStrictPassiveEffectsMode = /* */ 0b1000000;
24 changes: 24 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,30 @@ describe('ReactOffscreenStrictMode', () => {
]);
});

// @gate __DEV__ && enableOffscreen
it('should trigger strict effects when disableStrictPassiveEffect is presented on StrictMode', async () => {
await act(() => {
ReactNoop.render(
<React.StrictMode unstable_disableStrictPassiveEffect={true}>
<Offscreen>
<Component label="A" />
</Offscreen>
</React.StrictMode>,
);
});

expect(log).toEqual([
'A: render',
'A: render',
'A: useLayoutEffect mount',
'A: useEffect mount',
'A: useLayoutEffect unmount',
'A: useEffect unmount',
'A: useLayoutEffect mount',
'A: useEffect mount',
]);
});

// @gate __DEV__ && enableOffscreen && useModernStrictMode
it('should not trigger strict effects when offscreen is hidden', async () => {
await act(() => {
Expand Down
21 changes: 21 additions & 0 deletions packages/react/src/__tests__/ReactStrictMode-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,27 @@ describe('ReactStrictMode', () => {
]);
});

it('should include legacy + strict effects mode, but not strict passive effect with disableStrictPassiveEffect', async () => {
await act(() => {
const container = document.createElement('div');
const root = ReactDOMClient.createRoot(container);
root.render(
<React.StrictMode unstable_disableStrictPassiveEffect={true}>
<Component label="A" />
</React.StrictMode>,
);
});

expect(log).toEqual([
'A: render',
'A: render',
'A: useLayoutEffect mount',
'A: useEffect mount',
'A: useLayoutEffect unmount',
'A: useLayoutEffect mount',
]);
});

it('should allow level to be increased with nesting', async () => {
await act(() => {
const container = document.createElement('div');
Expand Down