@samitouri / QOS-React / commits / 3ac551e855

Dim `console` calls on additional Effect invocations due to `StrictMode` (#29007)

Sebastian Silbermann committed May 22, 2024 at 11:39 UTC 3ac551e855f9bec3161da2fc8787958aa62113db

9 files changed +272 -30

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

+20 -1

@@ -146,6 +146,12 @@ describe('ReactInternalTestUtils', () => {

146 test('assertLog', async () => {

147 const Yield = ({id}) => {

148 Scheduler.log(id);

149 + React.useEffect(() => {

150 + Scheduler.log(`create effect ${id}`);

151 + return () => {

152 + Scheduler.log(`cleanup effect ${id}`);

153 + };

154 + });

155 return id;

156 };

157

@@ -167,7 +173,20 @@ describe('ReactInternalTestUtils', () => {

173 </React.StrictMode>

174 );

175 });

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']);

190 });

191 });

192

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

+162

@@ -625,6 +625,168 @@ describe('console', () => {

625 expect(mockGroupCollapsed.mock.calls[0][0]).toBe('groupCollapsed');

626 });

627

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 +

790 it('should double log from useMemo if hideConsoleLogsInStrictMode is disabled in Strict mode', () => {

791 global.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = false;

792 global.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ = false;

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

+2 -2

@@ -294,7 +294,7 @@ export function unpatch(): void {

294

295 let unpatchForStrictModeFn: null | (() => void) = null;

296

297 -// NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialRenderInStrictMode

297 +// NOTE: KEEP IN SYNC with src/hook.js:patchConsoleForInitialCommitInStrictMode

298 export function patchForStrictMode() {

299 if (consoleManagedByDevToolsDuringStrictMode) {

300 const overrideConsoleMethods = [

@@ -359,7 +359,7 @@ export function patchForStrictMode() {

359 }

360 }

361

362 -// NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialRenderInStrictMode

362 +// NOTE: KEEP IN SYNC with src/hook.js:unpatchConsoleForInitialCommitInStrictMode

363 export function unpatchForStrictMode(): void {

364 if (consoleManagedByDevToolsDuringStrictMode) {

365 if (unpatchForStrictModeFn !== null) {

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

+8 -1

@@ -75,7 +75,14 @@ export default function DebuggingSettings(_: {}): React.Node {

75 setHideConsoleLogsInStrictMode(currentTarget.checked)

76 }

77 />{' '}

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>

86 </label>

87 </div>

88 </div>

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

+2 -2

@@ -141,7 +141,7 @@

141 border-radius: 0.25rem;

142 }

143

144 -.ReleaseNotesLink {

144 +.ReleaseNotesLink, .StrictModeLink {

145 color: var(--color-button-active);

146 }

147

@@ -153,4 +153,4 @@

153 list-style: none;

154 padding: 0;

155 margin: 0;

156 -}

\ No newline at end of file

156 +}

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

+5 -5

@@ -225,7 +225,7 @@ export function installHook(target: any): DevToolsHook | null {

225 // React and DevTools are connecting and the renderer interface isn't avaiable

226 // and we want to be able to have consistent logging behavior for double logs

227 // during the initial renderer.

228 - function patchConsoleForInitialRenderInStrictMode({

228 + function patchConsoleForInitialCommitInStrictMode({

229 hideConsoleLogsInStrictMode,

230 browserTheme,

231 }: {

@@ -311,7 +311,7 @@ export function installHook(target: any): DevToolsHook | null {

311 }

312

313 // NOTE: KEEP IN SYNC with src/backend/console.js:unpatchForStrictMode

314 - function unpatchConsoleForInitialRenderInStrictMode() {

314 + function unpatchConsoleForInitialCommitInStrictMode() {

315 if (unpatchFn !== null) {

316 unpatchFn();

317 unpatchFn = null;

@@ -451,19 +451,19 @@ export function installHook(target: any): DevToolsHook | null {

451 rendererInterface.unpatchConsoleForStrictMode();

452 }

453 } 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

455 // finishes its handshake with the injected renderer

456 if (isStrictMode) {

457 const hideConsoleLogsInStrictMode =

458 window.__REACT_DEVTOOLS_HIDE_CONSOLE_LOGS_IN_STRICT_MODE__ === true;

459 const browserTheme = window.__REACT_DEVTOOLS_BROWSER_THEME__;

460

461 - patchConsoleForInitialRenderInStrictMode({

461 + patchConsoleForInitialCommitInStrictMode({

462 hideConsoleLogsInStrictMode,

463 browserTheme,

464 });

465 } else {

466 - unpatchConsoleForInitialRenderInStrictMode();

466 + unpatchConsoleForInitialCommitInStrictMode();

467 }

468 }

469 }

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -251,6 +251,7 @@ import {

251 markRenderStopped,

252 onCommitRoot as onCommitRootDevTools,

253 onPostCommitRoot as onPostCommitRootDevTools,

254 + setIsStrictModeForDevtools,

255 } from './ReactFiberDevToolsHook';

256 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';

257 import {releaseCache} from './ReactFiberCacheComponent';

@@ -3675,6 +3676,7 @@ function doubleInvokeEffectsOnFiber(

3676 fiber: Fiber,

3677 shouldDoubleInvokePassiveEffects: boolean = true,

3678 ) {

3679 + setIsStrictModeForDevtools(true);

3680 disappearLayoutEffects(fiber);

3681 if (shouldDoubleInvokePassiveEffects) {

3682 disconnectPassiveEffect(fiber);

@@ -3683,6 +3685,7 @@ function doubleInvokeEffectsOnFiber(

3685 if (shouldDoubleInvokePassiveEffects) {

3686 reconnectPassiveEffects(root, fiber, NoLanes, null, false);

3687 }

3688 + setIsStrictModeForDevtools(false);

3689 }

3690

3691 function doubleInvokeEffectsInDEVIfNecessary(

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

+1 -19

@@ -115,11 +115,7 @@ describe('StrictEffectsMode defaults', () => {

115 </React.StrictMode>,

116 );

117

118 - await waitForPaint([

119 - 'useLayoutEffect mount "one"',

120 - 'useLayoutEffect unmount "one"',

121 - 'useLayoutEffect mount "one"',

122 - ]);

118 + await waitForPaint(['useLayoutEffect mount "one"']);

119 expect(log).toEqual([

120 'useLayoutEffect mount "one"',

121 'useLayoutEffect unmount "one"',

@@ -142,10 +138,6 @@ describe('StrictEffectsMode defaults', () => {

138 'useLayoutEffect unmount "one"',

139 'useLayoutEffect mount "one"',

140 'useLayoutEffect mount "two"',

145 -

146 - // Since "two" is new, it should be double-invoked.

147 - 'useLayoutEffect unmount "two"',

148 - 'useLayoutEffect mount "two"',

141 ]);

142 expect(log).toEqual([

143 // Cleanup and re-run "one" (and "two") since there is no dependencies array.

@@ -196,10 +188,6 @@ describe('StrictEffectsMode defaults', () => {

188 await waitForAll([

189 'useLayoutEffect mount "one"',

190 'useEffect mount "one"',

199 - 'useLayoutEffect unmount "one"',

200 - 'useEffect unmount "one"',

201 - 'useLayoutEffect mount "one"',

202 - 'useEffect mount "one"',

191 ]);

192 expect(log).toEqual([

193 'useLayoutEffect mount "one"',

@@ -237,12 +225,6 @@ describe('StrictEffectsMode defaults', () => {

225 'useEffect unmount "one"',

226 'useEffect mount "one"',

227 '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"',

228 ]);

229 expect(log).toEqual([

230 'useEffect unmount "one"',

packages/react/src/__tests__/ReactStrictMode-test.js

+69

@@ -1343,6 +1343,42 @@ describe('context legacy', () => {

1343 // and on the next render they'd get deduplicated and ignored.

1344 expect(console.log).toBeCalledWith('foo 1');

1345 });

1346 +

1347 + it('does not disable logs for effect double invoke', async () => {

1348 + let create = 0;

1349 + let cleanup = 0;

1350 + function Foo() {

1351 + React.useEffect(() => {

1352 + create++;

1353 + console.log('foo create ' + create);

1354 + return () => {

1355 + cleanup++;

1356 + console.log('foo cleanup ' + cleanup);

1357 + };

1358 + });

1359 + return null;

1360 + }

1361 +

1362 + const container = document.createElement('div');

1363 + const root = ReactDOMClient.createRoot(container);

1364 + await act(() => {

1365 + root.render(

1366 + <React.StrictMode>

1367 + <Foo />

1368 + </React.StrictMode>,

1369 + );

1370 + });

1371 + expect(create).toBe(__DEV__ ? 2 : 1);

1372 + expect(cleanup).toBe(__DEV__ ? 1 : 0);

1373 + expect(console.log).toBeCalledTimes(__DEV__ ? 3 : 1);

1374 + // Note: we should display the first log because otherwise

1375 + // there is a risk of suppressing warnings when they happen,

1376 + // and on the next render they'd get deduplicated and ignored.

1377 + expect(console.log).toBeCalledWith('foo create 1');

1378 + if (__DEV__) {

1379 + expect(console.log).toBeCalledWith('foo cleanup 1');

1380 + }

1381 + });

1382 } else {

1383 it('disable logs for class double render', async () => {

1384 let count = 0;

@@ -1530,6 +1566,39 @@ describe('context legacy', () => {

1566 // and on the next render they'd get deduplicated and ignored.

1567 expect(console.log).toBeCalledWith('foo 1');

1568 });

1569 +

1570 + it('disable logs for effect double invoke', async () => {

1571 + let create = 0;

1572 + let cleanup = 0;

1573 + function Foo() {

1574 + React.useEffect(() => {

1575 + create++;

1576 + console.log('foo create ' + create);

1577 + return () => {

1578 + cleanup++;

1579 + console.log('foo cleanup ' + cleanup);

1580 + };

1581 + });

1582 + return null;

1583 + }

1584 +

1585 + const container = document.createElement('div');

1586 + const root = ReactDOMClient.createRoot(container);

1587 + await act(() => {

1588 + root.render(

1589 + <React.StrictMode>

1590 + <Foo />

1591 + </React.StrictMode>,

1592 + );

1593 + });

1594 + expect(create).toBe(__DEV__ ? 2 : 1);

1595 + expect(cleanup).toBe(__DEV__ ? 1 : 0);

1596 + expect(console.log).toBeCalledTimes(1);

1597 + // Note: we should display the first log because otherwise

1598 + // there is a risk of suppressing warnings when they happen,

1599 + // and on the next render they'd get deduplicated and ignored.

1600 + expect(console.log).toBeCalledWith('foo create 1');

1601 + });

1602 }

1603 });

1604 });