Skip to content

Commit

Permalink
refactor[react-devtools]: move console patching to global hook (#30596)
Browse files Browse the repository at this point in the history
Stacked on #30566 and whats under
it. See [this
commit](374fd73).

It is mostly copying code from one place to another and updating tests.
With these changes, for every console method that we patch, there is
going to be a single applied patch:
- For `error`, `warn`, and `trace` we are patching when hook is
installed. This guarantees that component stacks are going to be
appended even if browser DevTools are not opened. We pay some price for
it, though: if user has browser DevTools closed and if at this point
some warning or error is emitted (logged), the next time user opens
browser DevTools, they are going to see `hook.js` as the source frame.
Unfortunately, ignore listing from source maps is not applied
retroactively, and I don't know if its a bug or just a design
limitations. Once browser DevTools are opened, source maps will be
loaded and ignore listing will be applied for all emitted logs in the
future.
- For `log`, `info`, `group`, `groupCollapsed` we are only patching when
React notifies React DevTools about running in StrictMode. We unpatch
the methods right after it.
  • Loading branch information
hoxyq authored Sep 18, 2024
1 parent b521ef8 commit 3cac087
Show file tree
Hide file tree
Showing 13 changed files with 680 additions and 1,383 deletions.
4 changes: 0 additions & 4 deletions packages/react-devtools-core/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import Agent from 'react-devtools-shared/src/backend/agent';
import Bridge from 'react-devtools-shared/src/bridge';
import {installHook} from 'react-devtools-shared/src/hook';
import {initBackend} from 'react-devtools-shared/src/backend';
import {installConsoleFunctionsToWindow} from 'react-devtools-shared/src/backend/console';
import {__DEBUG__} from 'react-devtools-shared/src/constants';
import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor';
import {getDefaultComponentFilters} from 'react-devtools-shared/src/utils';
Expand Down Expand Up @@ -41,9 +40,6 @@ type ConnectOptions = {
devToolsSettingsManager: ?DevToolsSettingsManager,
};

// Install a global variable to allow patching console early (during injection).
// This provides React Native developers with components stacks even if they don't run DevTools.
installConsoleFunctionsToWindow();
installHook(window);

const hook: ?DevToolsHook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
4 changes: 0 additions & 4 deletions packages/react-devtools-inline/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import Agent from 'react-devtools-shared/src/backend/agent';
import Bridge from 'react-devtools-shared/src/bridge';
import {initBackend} from 'react-devtools-shared/src/backend';
import {installConsoleFunctionsToWindow} from 'react-devtools-shared/src/backend/console';
import {installHook} from 'react-devtools-shared/src/hook';
import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor';

Expand Down Expand Up @@ -120,8 +119,5 @@ export function createBridge(contentWindow: any, wall?: Wall): BackendBridge {
}

export function initialize(contentWindow: any): void {
// Install a global variable to allow patching console early (during injection).
// This provides React Native developers with components stacks even if they don't run DevTools.
installConsoleFunctionsToWindow();
installHook(contentWindow);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,17 @@
* @flow
*/

import {getVersionedRenderImplementation, normalizeCodeLocInfo} from './utils';
import {
getVersionedRenderImplementation,
normalizeCodeLocInfo,
} from 'react-devtools-shared/src/__tests__/utils';

describe('component stack', () => {
let React;
let act;
let mockError;
let mockWarn;
let supportsOwnerStacks;

beforeEach(() => {
// Intercept native console methods before DevTools bootstraps.
// Normalize component stack locations.
mockError = jest.fn();
mockWarn = jest.fn();
console.error = (...args) => {
mockError(...args.map(normalizeCodeLocInfo));
};
console.warn = (...args) => {
mockWarn(...args.map(normalizeCodeLocInfo));
};

const utils = require('./utils');
act = utils.act;

Expand All @@ -54,18 +44,22 @@ describe('component stack', () => {

act(() => render(<Grandparent />));

expect(mockError).toHaveBeenCalledWith(
expect(
global.consoleErrorMock.mock.calls[0].map(normalizeCodeLocInfo),
).toEqual([
'Test error.',
'\n in Child (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
);
expect(mockWarn).toHaveBeenCalledWith(
]);
expect(
global.consoleWarnMock.mock.calls[0].map(normalizeCodeLocInfo),
).toEqual([
'Test warning.',
'\n in Child (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
);
]);
});

// This test should have caught #19911
Expand All @@ -89,13 +83,15 @@ describe('component stack', () => {

expect(useEffectCount).toBe(1);

expect(mockWarn).toHaveBeenCalledWith(
expect(
global.consoleWarnMock.mock.calls[0].map(normalizeCodeLocInfo),
).toEqual([
'Warning to trigger appended component stacks.',
'\n in Example (at **)',
);
]);
});

// @reactVersion >=18.3
// @reactVersion >= 18.3
it('should log the current component stack with debug info from promises', () => {
const Child = () => {
console.error('Test error.');
Expand All @@ -117,23 +113,27 @@ describe('component stack', () => {

act(() => render(<Grandparent />));

expect(mockError).toHaveBeenCalledWith(
expect(
global.consoleErrorMock.mock.calls[0].map(normalizeCodeLocInfo),
).toEqual([
'Test error.',
supportsOwnerStacks
? '\n in Child (at **)'
: '\n in Child (at **)' +
'\n in ServerComponent (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
);
expect(mockWarn).toHaveBeenCalledWith(
'\n in ServerComponent (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
]);
expect(
global.consoleWarnMock.mock.calls[0].map(normalizeCodeLocInfo),
).toEqual([
'Test warning.',
supportsOwnerStacks
? '\n in Child (at **)'
: '\n in Child (at **)' +
'\n in ServerComponent (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
);
'\n in ServerComponent (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
]);
});
});
Loading

0 comments on commit 3cac087

Please sign in to comment.