Skip to content

Commit

Permalink
Disable consoleWithStackDev Transform except in RN/WWW (#30313)
Browse files Browse the repository at this point in the history
Stacked on #30308.

This is now a noop module so we can stop applying the transform of
console.error using the Babel plugin in the mainline builds. I'm keeping
the transform for RN/WWW for now although it might be nice if the
transform moved into those systems as it gets synced instead of keeping
it upstream.

In jest tests we're already not running the forks for RN/WWW so we don't
need it at all there.
  • Loading branch information
sebmarkbage authored Jul 12, 2024
1 parent 400e822 commit ff89ba7
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 95 deletions.
12 changes: 2 additions & 10 deletions packages/react-client/src/ReactClientConsoleConfigBrowser.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
* @flow
*/

import {warn, error} from 'shared/consoleWithStackDev';

const badgeFormat = '%c%s%c ';
// Same badge styling as DevTools.
const badgeStyle =
Expand Down Expand Up @@ -65,12 +63,6 @@ export function printToConsole(
);
}

if (methodName === 'error' && __DEV__) {
error.apply(console, newArgs);
} else if (methodName === 'warn' && __DEV__) {
warn.apply(console, newArgs);
} else {
// $FlowFixMe[invalid-computed-prop]
console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
}
// $FlowFixMe[invalid-computed-prop]
console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
}
12 changes: 2 additions & 10 deletions packages/react-client/src/ReactClientConsoleConfigPlain.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
* @flow
*/

import {warn, error} from 'shared/consoleWithStackDev';

const badgeFormat = '[%s] ';
const pad = ' ';

Expand Down Expand Up @@ -46,12 +44,6 @@ export function printToConsole(
newArgs.splice(offset, 0, badgeFormat, pad + badgeName + pad);
}

if (methodName === 'error' && __DEV__) {
error.apply(console, newArgs);
} else if (methodName === 'warn' && __DEV__) {
warn.apply(console, newArgs);
} else {
// $FlowFixMe[invalid-computed-prop]
console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
}
// $FlowFixMe[invalid-computed-prop]
console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
}
12 changes: 2 additions & 10 deletions packages/react-client/src/ReactClientConsoleConfigServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
* @flow
*/

import {warn, error} from 'shared/consoleWithStackDev';

// This flips color using ANSI, then sets a color styling, then resets.
const badgeFormat = '\x1b[0m\x1b[7m%c%s\x1b[0m%c ';
// Same badge styling as DevTools.
Expand Down Expand Up @@ -66,12 +64,6 @@ export function printToConsole(
);
}

if (methodName === 'error' && __DEV__) {
error.apply(console, newArgs);
} else if (methodName === 'warn' && __DEV__) {
warn.apply(console, newArgs);
} else {
// $FlowFixMe[invalid-computed-prop]
console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
}
// $FlowFixMe[invalid-computed-prop]
console[methodName].apply(console, newArgs); // eslint-disable-line react-internal/no-production-logging
}
2 changes: 1 addition & 1 deletion packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -2090,7 +2090,7 @@ function resolveConsoleEntry(
task.run(callStack);
return;
}
// TODO: Set the current owner so that consoleWithStackDev adds the component
// TODO: Set the current owner so that captureOwnerStack() adds the component
// stack during the replay - if needed.
}
const rootTask = response._debugRootTask;
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberErrorLogger.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export function defaultOnCaughtError(
error,
componentNameMessage,
recreateMessage,
// We let our consoleWithStackDev wrapper add the component stack to the end.
// We let DevTools or console.createTask add the component stack to the end.
],
error.environmentName,
);
Expand All @@ -134,7 +134,7 @@ export function defaultOnCaughtError(
error,
componentNameMessage,
recreateMessage,
// We let our consoleWithStackDev wrapper add the component stack to the end.
// We let our DevTools or console.createTask add the component stack to the end.
);
}
} finally {
Expand Down
38 changes: 8 additions & 30 deletions packages/shared/consoleWithStackDev.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,14 @@
* LICENSE file in the root directory of this source tree.
*/

export function setSuppressWarning(newSuppressWarning) {
// TODO: Noop. Delete.
}

// In DEV, calls to console.warn and console.error get replaced
// by calls to these methods by a Babel plugin.
// We expect that our Rollup, Jest, and Flow configurations
// always shim this module with the corresponding environment
// (either rn or www).
//
// In PROD (or in packages without access to React internals),
// they are left as they are instead.

export function warn(format, ...args) {
if (__DEV__) {
printWarning('warn', format, args);
}
}

export function error(format, ...args) {
if (__DEV__) {
printWarning('error', format, args);
}
}
// We should never resolve to this file, but it exists to make
// sure that if we *do* accidentally break the configuration,
// the failure isn't silent.

function printWarning(level, format, args) {
// When changing this logic, you might want to also
// update consoleWithStackDev.www.js as well.
if (__DEV__) {
args.unshift(format);
// We intentionally don't use spread (or .apply) directly because it
// breaks IE9: https://github.com/facebook/react/issues/13610
// eslint-disable-next-line react-internal/no-production-logging
Function.prototype.apply.call(console[level], console, args);
}
export function setSuppressWarning() {
// TODO: Delete this and error when even importing this module.
}
12 changes: 1 addition & 11 deletions scripts/jest/preprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ const pathToBabel = path.join(
'../..',
'package.json'
);
const pathToBabelPluginReplaceConsoleCalls = require.resolve(
'../babel/transform-replace-console-calls'
);
const pathToTransformInfiniteLoops = require.resolve(
'../babel/transform-prevent-infinite-loops'
);
Expand Down Expand Up @@ -73,14 +70,7 @@ module.exports = {
const isInDevToolsPackages = !!filePath.match(
/\/packages\/react-devtools.*\//
);
const testOnlyPlugins = [];
const sourceOnlyPlugins = [];
if (process.env.NODE_ENV === 'development' && !isInDevToolsPackages) {
sourceOnlyPlugins.push(pathToBabelPluginReplaceConsoleCalls);
}
const plugins = (isTestFile ? testOnlyPlugins : sourceOnlyPlugins).concat(
babelOptions.plugins
);
const plugins = [].concat(babelOptions.plugins);
if (isTestFile && isInDevToolsPackages) {
plugins.push(pathToTransformReactVersionPragma);
}
Expand Down
1 change: 0 additions & 1 deletion scripts/print-warnings/print-warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ function transform(file, enc, cb) {
gs([
'packages/**/*.js',
'!packages/*/npm/**/*.js',
'!packages/shared/consoleWithStackDev.js',
'!packages/react-devtools*/**/*.js',
'!**/__tests__/**/*.js',
'!**/__mocks__/**/*.js',
Expand Down
26 changes: 16 additions & 10 deletions scripts/rollup/build-ghaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,22 @@ function getBabelConfig(
sourcemap: false,
};
if (isDevelopment) {
options.plugins.push(
...babelToES5Plugins,
// Turn console.error/warn() into a custom wrapper
[
require('../babel/transform-replace-console-calls'),
{
shouldError: !canAccessReactObject,
},
]
);
options.plugins.push(...babelToES5Plugins);
if (
bundleType === FB_WWW_DEV ||
bundleType === RN_OSS_DEV ||
bundleType === RN_FB_DEV
) {
options.plugins.push(
// Turn console.error/warn() into a custom wrapper
[
require('../babel/transform-replace-console-calls'),
{
shouldError: !canAccessReactObject,
},
]
);
}
}
if (updateBabelOptions) {
options = updateBabelOptions(options);
Expand Down
26 changes: 16 additions & 10 deletions scripts/rollup/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,22 @@ function getBabelConfig(
sourcemap: false,
};
if (isDevelopment) {
options.plugins.push(
...babelToES5Plugins,
// Turn console.error/warn() into a custom wrapper
[
require('../babel/transform-replace-console-calls'),
{
shouldError: !canAccessReactObject,
},
]
);
options.plugins.push(...babelToES5Plugins);
if (
bundleType === FB_WWW_DEV ||
bundleType === RN_OSS_DEV ||
bundleType === RN_FB_DEV
) {
options.plugins.push(
// Turn console.error/warn() into a custom wrapper
[
require('../babel/transform-replace-console-calls'),
{
shouldError: !canAccessReactObject,
},
]
);
}
}
if (updateBabelOptions) {
options = updateBabelOptions(options);
Expand Down

0 comments on commit ff89ba7

Please sign in to comment.