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.

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

feat[react-devtools]: add settings to global hook object #30564

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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,
});
Comment on lines +811 to +816
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this typed or is it easy to forget to add/remove fields here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is typed, and will be updated to a shorter form in 248ddba


// 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,
};
Comment on lines +542 to +547
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a successor of ConsolePatchSettings, the only difference is browserTheme field, but looks like it is not used. I will do it later in this stack.

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
Loading