Skip to content

Disable consoleWithStackDev Transform except in RN/WWW #30313

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 12, 2024
Merged

Disable consoleWithStackDev Transform except in RN/WWW #30313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 10 deletions packages/react-client/src/ReactClientConsoleConfigBrowser.js
View file
Open in desktop
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
Comment on lines -68 to +67
Copy link
Contributor

@hoxyq hoxyq Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Looking at changes in facebook-www/ReactDOM-dev.modern.js, this could call original console methods?

Or are they guaranteed to be transformed at this point? So we end up calling same warn and error from consoleWithStackDev?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are not guaranteed to be transformed. This affects not just this but the Flight log replaying. In other words, it won't go through the warning module in www/rn. But both of these are only affecting Flight so we just need to figure that out once Flight when added to www what to do avoid Flight logs in www.

}
12 changes: 2 additions & 10 deletions packages/react-client/src/ReactClientConsoleConfigPlain.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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