Skip to content

Commit

Permalink
feat[react-devtools]: add settings to global hook object (#30564)
Browse files Browse the repository at this point in the history
Right now we are patching console 2 times: when hook is installed
(before page is loaded) and when backend is connected. Because of this,
even if user had `appendComponentStack` setting enabled, all emitted
error and warning logs are not going to have component stacks appended.
They also won't have component stacks appended retroactively when user
opens browser DevTools (this is when frontend is initialized and
connects to backend).

This behavior adds potential race conditions with LogBox in React
Native, and also unpredictable to the user, because in order to get
component stacks logged you have to open browser DevTools, but by the
time you do it, error or warning log was already emitted.

To solve this, we are going to only patch console in the hook object,
because it is guaranteed to load even before React. Settings are going
to be synchronized with the hook via Bridge, and React DevTools Backend
Host (React Native or browser extension shell) will be responsible for
persisting these settings across the session, this is going to be
implemented in a separate PR.
  • Loading branch information
hoxyq committed Sep 18, 2024
1 parent 5dcb009 commit 5e83d9a
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 26 deletions.
39 changes: 15 additions & 24 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,9 @@ import type {
RendererID,
RendererInterface,
ConsolePatchSettings,
DevToolsHookSettings,
} from './types';
import type {
ComponentFilter,
BrowserTheme,
} from 'react-devtools-shared/src/frontend/types';
import type {ComponentFilter} from 'react-devtools-shared/src/frontend/types';
import {isSynchronousXHRSupported, isReactNativeEnvironment} from './utils';

const debug = (methodName: string, ...args: Array<string>) => {
Expand Down Expand Up @@ -153,6 +151,7 @@ export default class Agent extends EventEmitter<{
drawTraceUpdates: [Array<HostInstance>],
disableTraceUpdates: [],
getIfHasUnsupportedRendererVersion: [],
updateHookSettings: [DevToolsHookSettings],
}> {
_bridge: BackendBridge;
_isProfiling: boolean = false;
Expand Down Expand Up @@ -805,30 +804,22 @@ export default class Agent extends EventEmitter<{
}
};

updateConsolePatchSettings: ({
appendComponentStack: boolean,
breakOnConsoleErrors: boolean,
browserTheme: BrowserTheme,
hideConsoleLogsInStrictMode: boolean,
showInlineWarningsAndErrors: boolean,
}) => void = ({
appendComponentStack,
breakOnConsoleErrors,
showInlineWarningsAndErrors,
hideConsoleLogsInStrictMode,
browserTheme,
}: ConsolePatchSettings) => {
updateConsolePatchSettings: (
settings: $ReadOnly<ConsolePatchSettings>,
) => void = settings => {
// Propagate the settings, so Backend can subscribe to it and modify hook
this.emit('updateHookSettings', {
appendComponentStack: settings.appendComponentStack,
breakOnConsoleErrors: settings.breakOnConsoleErrors,
showInlineWarningsAndErrors: settings.showInlineWarningsAndErrors,
hideConsoleLogsInStrictMode: settings.hideConsoleLogsInStrictMode,
});

// If the frontend preferences have changed,
// or in the case of React Native- if the backend is just finding out the preferences-
// then reinstall the console overrides.
// It's safe to call `patchConsole` multiple times.
patchConsole({
appendComponentStack,
breakOnConsoleErrors,
showInlineWarningsAndErrors,
hideConsoleLogsInStrictMode,
browserTheme,
});
patchConsole(settings);
};

updateComponentFilters: (componentFilters: Array<ComponentFilter>) => void =
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export function patch({
showInlineWarningsAndErrors,
hideConsoleLogsInStrictMode,
browserTheme,
}: ConsolePatchSettings): void {
}: $ReadOnly<ConsolePatchSettings>): void {
// Settings may change after we've patched the console.
// Using a shared ref allows the patch function to read the latest values.
consoleSettingsRef.appendComponentStack = appendComponentStack;
Expand Down
4 changes: 4 additions & 0 deletions packages/react-devtools-shared/src/backend/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ export function initBackend(
agent.removeListener('shutdown', onAgentShutdown);
});

agent.addListener('updateHookSettings', settings => {
hook.settings = settings;
});

return () => {
subs.forEach(fn => fn());
};
Expand Down
8 changes: 8 additions & 0 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,7 @@ export type DevToolsHook = {
// Testing
dangerous_setTargetConsoleForTesting?: (fakeConsole: Object) => void,

settings?: DevToolsHookSettings,
...
};

Expand All @@ -537,3 +538,10 @@ export type ConsolePatchSettings = {
hideConsoleLogsInStrictMode: boolean,
browserTheme: BrowserTheme,
};

export type DevToolsHookSettings = {
appendComponentStack: boolean,
breakOnConsoleErrors: boolean,
showInlineWarningsAndErrors: boolean,
hideConsoleLogsInStrictMode: boolean,
};
28 changes: 27 additions & 1 deletion packages/react-devtools-shared/src/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
RendererID,
RendererInterface,
DevToolsBackend,
DevToolsHookSettings,
} from './backend/types';

import {
Expand All @@ -25,7 +26,12 @@ import attachRenderer from './attachRenderer';

declare var window: any;

export function installHook(target: any): DevToolsHook | null {
export function installHook(
target: any,
maybeSettingsOrSettingsPromise?:
| DevToolsHookSettings
| Promise<DevToolsHookSettings>,
): DevToolsHook | null {
if (target.hasOwnProperty('__REACT_DEVTOOLS_GLOBAL_HOOK__')) {
return null;
}
Expand Down Expand Up @@ -566,6 +572,26 @@ export function installHook(target: any): DevToolsHook | null {
registerInternalModuleStop,
};

if (maybeSettingsOrSettingsPromise == null) {
// Set default settings
hook.settings = {
appendComponentStack: true,
breakOnConsoleErrors: false,
showInlineWarningsAndErrors: true,
hideConsoleLogsInStrictMode: false,
};
} else {
Promise.resolve(maybeSettingsOrSettingsPromise)
.then(settings => {
hook.settings = settings;
})
.catch(() => {
targetConsole.error(
"React DevTools failed to get Console Patching settings. Console won't be patched and some console features will not work.",
);
});
}

if (__TEST__) {
hook.dangerous_setTargetConsoleForTesting =
dangerous_setTargetConsoleForTesting;
Expand Down

0 comments on commit 5e83d9a

Please sign in to comment.