Skip to content

Commit 3ac551e

Browse files
eps1loneps1lon
authored
Dim console calls on additional Effect invocations due to StrictMode (#29007)
1 parent 81c5ff2 commit 3ac551e

File tree

9 files changed

+272
-30
lines changed

9 files changed

+272
-30
lines changed

‎packages/internal-test-utils/__tests__/ReactInternalTestUtils-test.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,12 @@ describe('ReactInternalTestUtils', () => {
146146
test('assertLog', async () => {
147147
const Yield = ({id}) => {
148148
Scheduler.log(id);
149+
React.useEffect(() => {
150+
Scheduler.log(`create effect ${id}`);
151+
return () => {
152+
Scheduler.log(`cleanup effect ${id}`);
153+
};
154+
});
149155
return id;
150156
};
151157

@@ -167,7 +173,20 @@ describe('ReactInternalTestUtils', () => {
167173
</React.StrictMode>
168174
);
169175
});
170-
assertLog(['A', 'B', 'C']);
176+
assertLog([
177+
'A',
178+
'B',
179+
'C',
180+
'create effect A',
181+
'create effect B',
182+
'create effect C',
183+
]);
184+
185+
await act(() => {
186+
root.render(null);
187+
});
188+
189+
assertLog(['cleanup effect A', 'cleanup effect B', 'cleanup effect C']);
171190
});
172191
});
173192

‎packages/react-devtools-shared/src/__tests__/console-test.js

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,168 @@ describe('console', () => {
625625
expect(mockGroupCollapsed.mock.calls[0][0]).toBe('groupCollapsed');
626626
});
627627

628+
it('should double log from Effects if hideConsoleLogsInStrictMode is disabled in Strict mode', () => {
629+
global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false;
630+
global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = false;
631+
632+
const container = document.createElement('div');
633+
const root = ReactDOMClient.createRoot(container);
634+
635+
function App() {
636+
React.useEffect(() => {
637+
fakeConsole.log('log effect create');
638+
fakeConsole.warn('warn effect create');
639+
fakeConsole.error('error effect create');
640+
fakeConsole.info('info effect create');
641+
fakeConsole.group('group effect create');
642+
fakeConsole.groupCollapsed('groupCollapsed effect create');
643+
644+
return () => {
645+
fakeConsole.log('log effect cleanup');
646+
fakeConsole.warn('warn effect cleanup');
647+
fakeConsole.error('error effect cleanup');
648+
fakeConsole.info('info effect cleanup');
649+
fakeConsole.group('group effect cleanup');
650+
fakeConsole.groupCollapsed('groupCollapsed effect cleanup');
651+
};
652+
});
653+
654+
return <div />;
655+
}
656+
657+
act(() =>
658+
root.render(
659+
<React.StrictMode>
660+
<App />
661+
</React.StrictMode>,
662+
),
663+
);
664+
expect(mockLog.mock.calls).toEqual([
665+
['log effect create'],
666+
[
667+
'%c%s',
668+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
669+
'log effect cleanup',
670+
],
671+
[
672+
'%c%s',
673+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
674+
'log effect create',
675+
],
676+
]);
677+
expect(mockWarn.mock.calls).toEqual([
678+
['warn effect create'],
679+
[
680+
'%c%s',
681+
`color: ${process.env.DARK_MODE_DIMMED_WARNING_COLOR}`,
682+
'warn effect cleanup',
683+
],
684+
[
685+
'%c%s',
686+
`color: ${process.env.DARK_MODE_DIMMED_WARNING_COLOR}`,
687+
'warn effect create',
688+
],
689+
]);
690+
expect(mockError.mock.calls).toEqual([
691+
['error effect create'],
692+
[
693+
'%c%s',
694+
`color: ${process.env.DARK_MODE_DIMMED_ERROR_COLOR}`,
695+
'error effect cleanup',
696+
],
697+
[
698+
'%c%s',
699+
`color: ${process.env.DARK_MODE_DIMMED_ERROR_COLOR}`,
700+
'error effect create',
701+
],
702+
]);
703+
expect(mockInfo.mock.calls).toEqual([
704+
['info effect create'],
705+
[
706+
'%c%s',
707+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
708+
'info effect cleanup',
709+
],
710+
[
711+
'%c%s',
712+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
713+
'info effect create',
714+
],
715+
]);
716+
expect(mockGroup.mock.calls).toEqual([
717+
['group effect create'],
718+
[
719+
'%c%s',
720+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
721+
'group effect cleanup',
722+
],
723+
[
724+
'%c%s',
725+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
726+
'group effect create',
727+
],
728+
]);
729+
expect(mockGroupCollapsed.mock.calls).toEqual([
730+
['groupCollapsed effect create'],
731+
[
732+
'%c%s',
733+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
734+
'groupCollapsed effect cleanup',
735+
],
736+
[
737+
'%c%s',
738+
`color: ${process.env.DARK_MODE_DIMMED_LOG_COLOR}`,
739+
'groupCollapsed effect create',
740+
],
741+
]);
742+
});
743+
744+
it('should not double log from Effects if hideConsoleLogsInStrictMode is enabled in Strict mode', () => {
745+
global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false;
746+
global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = true;
747+
748+
const container = document.createElement('div');
749+
const root = ReactDOMClient.createRoot(container);
750+
751+
function App() {
752+
React.useEffect(() => {
753+
fakeConsole.log('log effect create');
754+
fakeConsole.warn('warn effect create');
755+
fakeConsole.error('error effect create');
756+
fakeConsole.info('info effect create');
757+
fakeConsole.group('group effect create');
758+
fakeConsole.groupCollapsed('groupCollapsed effect create');
759+
760+
return () => {
761+
fakeConsole.log('log effect cleanup');
762+
fakeConsole.warn('warn effect cleanup');
763+
fakeConsole.error('error effect cleanup');
764+
fakeConsole.info('info effect cleanup');
765+
fakeConsole.group('group effect cleanup');
766+
fakeConsole.groupCollapsed('groupCollapsed effect cleanup');
767+
};
768+
});
769+
770+
return <div />;
771+
}
772+
773+
act(() =>
774+
root.render(
775+
<React.StrictMode>
776+
<App />
777+
</React.StrictMode>,
778+
),
779+
);
780+
expect(mockLog.mock.calls).toEqual([['log effect create']]);
781+
expect(mockWarn.mock.calls).toEqual([['warn effect create']]);
782+
expect(mockError.mock.calls).toEqual([['error effect create']]);
783+
expect(mockInfo.mock.calls).toEqual([['info effect create']]);
784+
expect(mockGroup.mock.calls).toEqual([['group effect create']]);
785+
expect(mockGroupCollapsed.mock.calls).toEqual([
786+
['groupCollapsed effect create'],
787+
]);
788+
});
789+
628790
it('should double log from useMemo if hideConsoleLogsInStrictMode is disabled in Strict mode', () => {
629791
global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false;
630792
global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = false;

‎packages/react-devtools-shared/src/backend/console.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ export function unpatch(): void {
294294

295295
let unpatchForStrictModeFn: null | (() => void) = null;
296296

297-
// NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialRenderInStrictMode
297+
// NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialCommitInStrictMode
298298
export function patchForStrictMode() {
299299
if (consoleManagedByDevToolsDuringStrictMode) {
300300
const overrideConsoleMethods = [
@@ -359,7 +359,7 @@ export function patchForStrictMode() {
359359
}
360360
}
361361

362-
// NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialRenderInStrictMode
362+
// NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialCommitInStrictMode
363363
export function unpatchForStrictMode(): void {
364364
if (consoleManagedByDevToolsDuringStrictMode) {
365365
if (unpatchForStrictModeFn !== null) {

‎packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,14 @@ export default function DebuggingSettings(_: {}): React.Node {
7575
setHideConsoleLogsInStrictMode(currentTarget.checked)
7676
}
7777
/>{' '}
78-
Hide logs during second render in Strict Mode
78+
Hide logs during additional invocations in{' '}
79+
<a
80+
className={styles.StrictModeLink}
81+
target="_blank"
82+
rel="noopener noreferrer"
83+
href="https://react.dev/reference/react/StrictMode">
84+
Strict Mode
85+
</a>
7986
</label>
8087
</div>
8188
</div>

‎packages/react-devtools-shared/src/devtools/views/Settings/SettingsShared.css

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@
141141
border-radius: 0.25rem;
142142
}
143143

144-
.ReleaseNotesLink {
144+
.ReleaseNotesLink, .StrictModeLink {
145145
color: var(--color-button-active);
146146
}
147147

@@ -153,4 +153,4 @@
153153
list-style: none;
154154
padding: 0;
155155
margin: 0;
156-
}
156+
}

‎packages/react-devtools-shared/src/hook.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ export function installHook(target: any): DevToolsHook | null {
225225
// React and DevTools are connecting and the renderer interface isn't avaiable
226226
// and we want to be able to have consistent logging behavior for double logs
227227
// during the initial renderer.
228-
function patchConsoleForInitialRenderInStrictMode({
228+
function patchConsoleForInitialCommitInStrictMode({
229229
hideConsoleLogsInStrictMode,
230230
browserTheme,
231231
}: {
@@ -311,7 +311,7 @@ export function installHook(target: any): DevToolsHook | null {
311311
}
312312

313313
// NOTE: KEEP IN SYNC with src/backend/console.js:unpatchForStrictMode
314-
function unpatchConsoleForInitialRenderInStrictMode() {
314+
function unpatchConsoleForInitialCommitInStrictMode() {
315315
if (unpatchFn !== null) {
316316
unpatchFn();
317317
unpatchFn = null;
@@ -451,19 +451,19 @@ export function installHook(target: any): DevToolsHook | null {
451451
rendererInterface.unpatchConsoleForStrictMode();
452452
}
453453
} else {
454-
// This should only happen during initial render in the extension before DevTools
454+
// This should only happen during initial commit in the extension before DevTools
455455
// finishes its handshake with the injected renderer
456456
if (isStrictMode) {
457457
const hideConsoleLogsInStrictMode =
458458
window.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ === true;
459459
const browserTheme = window.__REACT_DEVTOOLS_BROWSER_THEME__;
460460

461-
patchConsoleForInitialRenderInStrictMode({
461+
patchConsoleForInitialCommitInStrictMode({
462462
hideConsoleLogsInStrictMode,
463463
browserTheme,
464464
});
465465
} else {
466-
unpatchConsoleForInitialRenderInStrictMode();
466+
unpatchConsoleForInitialCommitInStrictMode();
467467
}
468468
}
469469
}

‎packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ import {
251251
markRenderStopped,
252252
onCommitRoot as onCommitRootDevTools,
253253
onPostCommitRoot as onPostCommitRootDevTools,
254+
setIsStrictModeForDevtools,
254255
} from './ReactFiberDevToolsHook';
255256
import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
256257
import {releaseCache} from './ReactFiberCacheComponent';
@@ -3675,6 +3676,7 @@ function doubleInvokeEffectsOnFiber(
36753676
fiber: Fiber,
36763677
shouldDoubleInvokePassiveEffects: boolean = true,
36773678
) {
3679+
setIsStrictModeForDevtools(true);
36783680
disappearLayoutEffects(fiber);
36793681
if (shouldDoubleInvokePassiveEffects) {
36803682
disconnectPassiveEffect(fiber);
@@ -3683,6 +3685,7 @@ function doubleInvokeEffectsOnFiber(
36833685
if (shouldDoubleInvokePassiveEffects) {
36843686
reconnectPassiveEffects(root, fiber, NoLanes, null, false);
36853687
}
3688+
setIsStrictModeForDevtools(false);
36863689
}
36873690

36883691
function doubleInvokeEffectsInDEVIfNecessary(

‎packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,7 @@ describe('StrictEffectsMode defaults', () => {
115115
</React.StrictMode>,
116116
);
117117

118-
await waitForPaint([
119-
'useLayoutEffect mount "one"',
120-
'useLayoutEffect unmount "one"',
121-
'useLayoutEffect mount "one"',
122-
]);
118+
await waitForPaint(['useLayoutEffect mount "one"']);
123119
expect(log).toEqual([
124120
'useLayoutEffect mount "one"',
125121
'useLayoutEffect unmount "one"',
@@ -142,10 +138,6 @@ describe('StrictEffectsMode defaults', () => {
142138
'useLayoutEffect unmount "one"',
143139
'useLayoutEffect mount "one"',
144140
'useLayoutEffect mount "two"',
145-
146-
// Since "two" is new, it should be double-invoked.
147-
'useLayoutEffect unmount "two"',
148-
'useLayoutEffect mount "two"',
149141
]);
150142
expect(log).toEqual([
151143
// Cleanup and re-run "one" (and "two") since there is no dependencies array.
@@ -196,10 +188,6 @@ describe('StrictEffectsMode defaults', () => {
196188
await waitForAll([
197189
'useLayoutEffect mount "one"',
198190
'useEffect mount "one"',
199-
'useLayoutEffect unmount "one"',
200-
'useEffect unmount "one"',
201-
'useLayoutEffect mount "one"',
202-
'useEffect mount "one"',
203191
]);
204192
expect(log).toEqual([
205193
'useLayoutEffect mount "one"',
@@ -237,12 +225,6 @@ describe('StrictEffectsMode defaults', () => {
237225
'useEffect unmount "one"',
238226
'useEffect mount "one"',
239227
'useEffect mount "two"',
240-
241-
// Since "two" is new, it should be double-invoked.
242-
'useLayoutEffect unmount "two"',
243-
'useEffect unmount "two"',
244-
'useLayoutEffect mount "two"',
245-
'useEffect mount "two"',
246228
]);
247229
expect(log).toEqual([
248230
'useEffect unmount "one"',

0 commit comments

Comments
 (0)