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

DevTools: Add break-on-warn feature #19048

Merged
merged 1 commit into from
May 29, 2020
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
12 changes: 8 additions & 4 deletions packages/react-devtools-core/src/standalone.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import {
import Bridge from 'react-devtools-shared/src/bridge';
import Store from 'react-devtools-shared/src/devtools/store';
import {
getSavedComponentFilters,
getAppendComponentStack,
getBreakOnConsoleErrors,
getSavedComponentFilters,
} from 'react-devtools-shared/src/utils';
import {Server} from 'ws';
import {join} from 'path';
Expand Down Expand Up @@ -282,11 +283,14 @@ function startServer(port?: number = 8097) {
// Because of this it relies on the extension to pass filters, so include them wth the response here.
// This will ensure that saved filters are shared across different web pages.
const savedPreferencesString = `
window.__REACT_DEVTOOLS_COMPONENT_FILTERS__ = ${JSON.stringify(
getSavedComponentFilters(),
)};
window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = ${JSON.stringify(
getAppendComponentStack(),
)};
window.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ = ${JSON.stringify(
getBreakOnConsoleErrors(),
)};
window.__REACT_DEVTOOLS_COMPONENT_FILTERS__ = ${JSON.stringify(
getSavedComponentFilters(),
)};`;

response.end(
Expand Down
18 changes: 10 additions & 8 deletions packages/react-devtools-extensions/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import Store from 'react-devtools-shared/src/devtools/store';
import {getBrowserName, getBrowserTheme} from './utils';
import {LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY} from 'react-devtools-shared/src/constants';
import {
getSavedComponentFilters,
getAppendComponentStack,
getBreakOnConsoleErrors,
getSavedComponentFilters,
} from 'react-devtools-shared/src/utils';
import {
localStorageGetItem,
Expand All @@ -28,17 +29,18 @@ let panelCreated = false;
// because they are stored in localStorage within the context of the extension.
// Instead it relies on the extension to pass filters through.
function syncSavedPreferences() {
const componentFilters = getSavedComponentFilters();
chrome.devtools.inspectedWindow.eval(
`window.__REACT_DEVTOOLS_COMPONENT_FILTERS__ = ${JSON.stringify(
componentFilters,
)};`,
);

const appendComponentStack = getAppendComponentStack();
const breakOnConsoleErrors = getBreakOnConsoleErrors();
const componentFilters = getSavedComponentFilters();
chrome.devtools.inspectedWindow.eval(
`window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = ${JSON.stringify(
appendComponentStack,
)};
window.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ = ${JSON.stringify(
breakOnConsoleErrors,
)};
window.__REACT_DEVTOOLS_COMPONENT_FILTERS__ = ${JSON.stringify(
componentFilters,
)};`,
);
}
Expand Down
11 changes: 11 additions & 0 deletions packages/react-devtools-extensions/webpack.backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const {resolve} = require('path');
const {DefinePlugin} = require('webpack');
const TerserPlugin = require('terser-webpack-plugin');
const {GITHUB_URL, getVersionString} = require('./utils');

const NODE_ENV = process.env.NODE_ENV;
Expand Down Expand Up @@ -39,6 +40,16 @@ module.exports = {
scheduler: resolve(builtModulesDir, 'scheduler'),
},
},
optimization: {
minimizer: [
new TerserPlugin({
terserOptions: {
compress: {drop_debugger: false},
output: {comments: true},
},
}),
],
},
plugins: [
new DefinePlugin({
__DEV__: true,
Expand Down
8 changes: 7 additions & 1 deletion packages/react-devtools-inline/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,14 @@ function startActivation(contentWindow: window) {
// so it's safe to cleanup after we've received it.
contentWindow.removeEventListener('message', onMessage);

const {appendComponentStack, componentFilters} = data;
const {
appendComponentStack,
breakOnConsoleErrors,
componentFilters,
} = data;

contentWindow.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = appendComponentStack;
contentWindow.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ = breakOnConsoleErrors;
contentWindow.__REACT_DEVTOOLS_COMPONENT_FILTERS__ = componentFilters;

// TRICKY
Expand All @@ -33,6 +38,7 @@ function startActivation(contentWindow: window) {
// but it doesn't really hurt anything to store them there too.
if (contentWindow !== window) {
window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ = appendComponentStack;
window.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ = breakOnConsoleErrors;
window.__REACT_DEVTOOLS_COMPONENT_FILTERS__ = componentFilters;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/react-devtools-inline/src/frontend.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import Bridge from 'react-devtools-shared/src/bridge';
import Store from 'react-devtools-shared/src/devtools/store';
import DevTools from 'react-devtools-shared/src/devtools/views/DevTools';
import {
getSavedComponentFilters,
getAppendComponentStack,
getBreakOnConsoleErrors,
getSavedComponentFilters,
} from 'react-devtools-shared/src/utils';
import {
MESSAGE_TYPE_GET_SAVED_PREFERENCES,
Expand Down Expand Up @@ -38,6 +39,7 @@ export function initialize(
{
type: MESSAGE_TYPE_SAVED_PREFERENCES,
appendComponentStack: getAppendComponentStack(),
breakOnConsoleErrors: getBreakOnConsoleErrors(),
componentFilters: getSavedComponentFilters(),
},
'*',
Expand Down
11 changes: 11 additions & 0 deletions packages/react-devtools-inline/webpack.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const {resolve} = require('path');
const {DefinePlugin} = require('webpack');
const TerserPlugin = require('terser-webpack-plugin');
const {
GITHUB_URL,
getVersionString,
Expand Down Expand Up @@ -36,6 +37,16 @@ module.exports = {
'react-is': 'react-is',
scheduler: 'scheduler',
},
optimization: {
minimizer: [
new TerserPlugin({
terserOptions: {
compress: {drop_debugger: false},
output: {comments: true},
},
}),
],
},
plugins: [
new DefinePlugin({
__DEV__,
Expand Down
15 changes: 12 additions & 3 deletions packages/react-devtools-shared/src/__tests__/console-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ describe('console', () => {

// Note the Console module only patches once,
// so it's important to patch the test console before injection.
patchConsole();
patchConsole({
appendComponentStack: true,
breakOnWarn: false,
});

const inject = global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject;
global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => {
Expand Down Expand Up @@ -79,7 +82,10 @@ describe('console', () => {
it('should only patch the console once', () => {
const {error, warn} = fakeConsole;

patchConsole();
patchConsole({
appendComponentStack: true,
breakOnWarn: false,
});

expect(fakeConsole.error).toBe(error);
expect(fakeConsole.warn).toBe(warn);
Expand Down Expand Up @@ -330,7 +336,10 @@ describe('console', () => {
expect(mockError.mock.calls[0]).toHaveLength(1);
expect(mockError.mock.calls[0][0]).toBe('error');

patchConsole();
patchConsole({
appendComponentStack: true,
breakOnWarn: false,
});
act(() => ReactDOM.render(<Child />, document.createElement('div')));

expect(mockWarn).toHaveBeenCalledTimes(2);
Expand Down
16 changes: 11 additions & 5 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ export default class Agent extends EventEmitter<{|
);
bridge.addListener('shutdown', this.shutdown);
bridge.addListener(
'updateAppendComponentStack',
this.updateAppendComponentStack,
'updateConsolePatchSettings',
this.updateConsolePatchSettings,
);
bridge.addListener('updateComponentFilters', this.updateComponentFilters);
bridge.addListener('viewAttributeSource', this.viewAttributeSource);
Expand Down Expand Up @@ -443,13 +443,19 @@ export default class Agent extends EventEmitter<{|
}
};

updateAppendComponentStack = (appendComponentStack: boolean) => {
updateConsolePatchSettings = ({
appendComponentStack,
breakOnConsoleErrors,
}: {|
appendComponentStack: boolean,
breakOnConsoleErrors: boolean,
|}) => {
// If the frontend preference has change,
// or in the case of React Native- if the backend is just finding out the preference-
// then install or uninstall the console overrides.
// It's safe to call these methods multiple times, so we don't need to worry about that.
if (appendComponentStack) {
patchConsole();
if (appendComponentStack || breakOnConsoleErrors) {
patchConsole({appendComponentStack, breakOnConsoleErrors});
} else {
unpatchConsole();
}
Expand Down
94 changes: 63 additions & 31 deletions packages/react-devtools-shared/src/backend/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,25 @@ export function registerRenderer(renderer: ReactRenderer): void {
}
}

const consoleSettingsRef = {
appendComponentStack: false,
breakOnConsoleErrors: false,
};

// Patches whitelisted console methods to append component stack for the current fiber.
// Call unpatch() to remove the injected behavior.
export function patch(): void {
export function patch({
appendComponentStack,
breakOnConsoleErrors,
}: {
appendComponentStack: boolean,
breakOnConsoleErrors: boolean,
}): 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;
consoleSettingsRef.breakOnConsoleErrors = breakOnConsoleErrors;

if (unpatchFn !== null) {
// Don't patch twice.
return;
Expand All @@ -105,40 +121,56 @@ export function patch(): void {
targetConsole[method]);

const overrideMethod = (...args) => {
try {
// If we are ever called with a string that already has a component stack, e.g. a React error/warning,
// don't append a second stack.
const lastArg = args.length > 0 ? args[args.length - 1] : null;
const alreadyHasComponentStack =
lastArg !== null &&
(PREFIX_REGEX.test(lastArg) ||
ROW_COLUMN_NUMBER_REGEX.test(lastArg));

if (!alreadyHasComponentStack) {
// If there's a component stack for at least one of the injected renderers, append it.
// We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?)
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
for (const {
currentDispatcherRef,
getCurrentFiber,
workTagMap,
} of injectedRenderers.values()) {
const current: ?Fiber = getCurrentFiber();
if (current != null) {
const componentStack = getStackByFiberInDevAndProd(
workTagMap,
current,
currentDispatcherRef,
);
if (componentStack !== '') {
args.push(componentStack);
const latestAppendComponentStack =
consoleSettingsRef.appendComponentStack;
const latestBreakOnConsoleErrors =
consoleSettingsRef.breakOnConsoleErrors;

if (latestAppendComponentStack) {
try {
// If we are ever called with a string that already has a component stack, e.g. a React error/warning,
// don't append a second stack.
const lastArg = args.length > 0 ? args[args.length - 1] : null;
const alreadyHasComponentStack =
lastArg !== null &&
(PREFIX_REGEX.test(lastArg) ||
ROW_COLUMN_NUMBER_REGEX.test(lastArg));

if (!alreadyHasComponentStack) {
// If there's a component stack for at least one of the injected renderers, append it.
// We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?)
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
for (const {
currentDispatcherRef,
getCurrentFiber,
workTagMap,
} of injectedRenderers.values()) {
const current: ?Fiber = getCurrentFiber();
if (current != null) {
const componentStack = getStackByFiberInDevAndProd(
workTagMap,
current,
currentDispatcherRef,
);
if (componentStack !== '') {
args.push(componentStack);
}
break;
}
break;
}
}
} catch (error) {
// Don't let a DevTools or React internal error interfere with logging.
}
} catch (error) {
// Don't let a DevTools or React internal error interfere with logging.
}

if (latestBreakOnConsoleErrors) {
// --- Welcome to debugging with React DevTools ---
// This debugger statement means that you've enabled the "break on warnings" feature.
// Use the browser's Call Stack panel to step out of this override function-
// to where the original warning or error was logged.
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line no-debugger
debugger;
}

originalMethod(...args);
Expand Down
13 changes: 10 additions & 3 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,18 @@ export function attach(
if (process.env.NODE_ENV !== 'test') {
registerRendererWithConsole(renderer);

// The renderer interface can't read this preference directly,
// The renderer interface can't read these preferences directly,
// because it is stored in localStorage within the context of the extension.
// It relies on the extension to pass the preference through via the global.
if (window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ !== false) {
patchConsole();
const appendComponentStack =
window.__REACT_DEVTOOLS_APPEND_COMPONENT_STACK__ !== false;
const breakOnConsoleErrors =
window.__REACT_DEVTOOLS_BREAK_ON_CONSOLE_ERRORS__ === true;
if (appendComponentStack || breakOnConsoleErrors) {
patchConsole({
appendComponentStack,
breakOnConsoleErrors,
});
}
}

Expand Down
7 changes: 6 additions & 1 deletion packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ type NativeStyleEditor_SetValueParams = {|
value: string,
|};

type UpdateConsolePatchSettingsParams = {|
appendComponentStack: boolean,
breakOnConsoleErrors: boolean,
|};

type BackendEvents = {|
extensionBackendInitialized: [],
inspectedElement: [InspectedElementPayload],
Expand Down Expand Up @@ -133,8 +138,8 @@ type FrontendEvents = {|
stopInspectingNative: [boolean],
stopProfiling: [],
storeAsGlobal: [StoreAsGlobalParams],
updateAppendComponentStack: [boolean],
updateComponentFilters: [Array<ComponentFilter>],
updateConsolePatchSettings: [UpdateConsolePatchSettingsParams],
viewAttributeSource: [ViewAttributeSourceParams],
viewElementSource: [ElementAndRendererID],

Expand Down
3 changes: 3 additions & 0 deletions packages/react-devtools-shared/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export const SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY =
export const SESSION_STORAGE_RELOAD_AND_PROFILE_KEY =
'React::DevTools::reloadAndProfile';

export const LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS =
'React::DevTools::breakOnConsoleErrors';

export const LOCAL_STORAGE_SHOULD_PATCH_CONSOLE_KEY =
'React::DevTools::appendComponentStack';

Expand Down
Loading