Skip to content

Commit 44b566b

Browse files
committed
Omit the component stack from the replaying
Since getCurrentStack is now implemented, React appends component stacks to its own logs. However, at the same time we've instrumented the console so we need to strip them back out before sending to the client. This is another reason we shouldn't append them from React.
1 parent 0a9111f commit 44b566b

File tree

3 files changed

+60
-2
lines changed

3 files changed

+60
-2
lines changed

Diff for: packages/react-client/src/__tests__/ReactFlight-test.js

+36
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ let ErrorBoundary;
8181
let NoErrorExpected;
8282
let Scheduler;
8383
let assertLog;
84+
let assertConsoleErrorDev;
8485

8586
describe('ReactFlight', () => {
8687
beforeEach(() => {
@@ -102,6 +103,7 @@ describe('ReactFlight', () => {
102103
Scheduler = require('scheduler');
103104
const InternalTestUtils = require('internal-test-utils');
104105
assertLog = InternalTestUtils.assertLog;
106+
assertConsoleErrorDev = InternalTestUtils.assertConsoleErrorDev;
105107

106108
ErrorBoundary = class extends React.Component {
107109
state = {hasError: false, error: null};
@@ -2758,4 +2760,38 @@ describe('ReactFlight', () => {
27582760
'\n in Bar (at **)' + '\n in Foo (at **)',
27592761
);
27602762
});
2763+
2764+
it('should not include component stacks in replayed logs (unless DevTools add them)', () => {
2765+
function Foo() {
2766+
return 'hi';
2767+
}
2768+
2769+
function Bar({children}) {
2770+
const array = [];
2771+
// Trigger key warning
2772+
array.push(<Foo />);
2773+
return <div>{array}</div>;
2774+
}
2775+
2776+
function App() {
2777+
return <Bar />;
2778+
}
2779+
2780+
const transport = ReactNoopFlightServer.render(<App />);
2781+
assertConsoleErrorDev([
2782+
'Each child in a list should have a unique "key" prop. See https://react.dev/link/warning-keys for more information.\n' +
2783+
' in Foo (at **)',
2784+
]);
2785+
2786+
// Replay logs on the client
2787+
ReactNoopFlightClient.read(transport);
2788+
assertConsoleErrorDev(
2789+
[
2790+
'Each child in a list should have a unique "key" prop. See https://react.dev/link/warning-keys for more information.',
2791+
],
2792+
// We should not have a stack in the replay because that should be added either by console.createTask
2793+
// or React DevTools on the client. Neither of which we do here.
2794+
{withoutStack: true},
2795+
);
2796+
});
27612797
});

Diff for: packages/react-server/src/ReactFlightServer.js

+20-2
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ import {resolveOwner, setCurrentOwner} from './flight/ReactFlightCurrentOwner';
9999

100100
import {getOwnerStackByComponentInfoInDev} from './flight/ReactFlightComponentStack';
101101

102+
import {isWritingAppendedStack} from 'shared/consoleWithStackDev';
103+
102104
import {
103105
getIteratorFn,
104106
REACT_ELEMENT_TYPE,
@@ -265,8 +267,9 @@ function patchConsole(consoleInst: typeof console, methodName: string) {
265267
'name',
266268
);
267269
const wrapperMethod = function (this: typeof console) {
270+
let args = arguments;
268271
const request = resolveRequest();
269-
if (methodName === 'assert' && arguments[0]) {
272+
if (methodName === 'assert' && args[0]) {
270273
// assert doesn't emit anything unless first argument is falsy so we can skip it.
271274
} else if (request !== null) {
272275
// Extract the stack. Not all console logs print the full stack but they have at
@@ -278,7 +281,22 @@ function patchConsole(consoleInst: typeof console, methodName: string) {
278281
// refer to previous logs in debug info to associate them with a component.
279282
const id = request.nextChunkId++;
280283
const owner: null | ReactComponentInfo = resolveOwner();
281-
emitConsoleChunk(request, id, methodName, owner, stack, arguments);
284+
if (
285+
isWritingAppendedStack &&
286+
(methodName === 'error' || methodName === 'warn') &&
287+
args.length > 1 &&
288+
typeof args[0] === 'string' &&
289+
args[0].endsWith('%s')
290+
) {
291+
// This looks like we've appended the component stack to the error from our own logs.
292+
// We don't want those added to the replayed logs since those have the opportunity to add
293+
// their own stacks or use console.createTask on the client as needed.
294+
// TODO: Remove this special case once we remove consoleWithStackDev.
295+
// $FlowFixMe[method-unbinding]
296+
args = Array.prototype.slice.call(args, 0, args.length - 1);
297+
args[0] = args[0].slice(0, args[0].length - 2);
298+
}
299+
emitConsoleChunk(request, id, methodName, owner, stack, args);
282300
}
283301
// $FlowFixMe[prop-missing]
284302
return originalMethod.apply(this, arguments);

Diff for: packages/shared/consoleWithStackDev.js

+4
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ export function error(format, ...args) {
4040
// eslint-disable-next-line react-internal/no-production-logging
4141
const supportsCreateTask = __DEV__ && enableOwnerStacks && !!console.createTask;
4242

43+
export let isWritingAppendedStack = false;
44+
4345
function printWarning(level, format, args, currentStack) {
4446
// When changing this logic, you might want to also
4547
// update consoleWithStackDev.www.js as well.
@@ -50,6 +52,7 @@ function printWarning(level, format, args, currentStack) {
5052
// can be lost while DevTools isn't open but we can't detect this.
5153
const stack = ReactSharedInternals.getCurrentStack(currentStack);
5254
if (stack !== '') {
55+
isWritingAppendedStack = true;
5356
format += '%s';
5457
args = args.concat([stack]);
5558
}
@@ -60,5 +63,6 @@ function printWarning(level, format, args, currentStack) {
6063
// breaks IE9: https://github.com/facebook/react/issues/13610
6164
// eslint-disable-next-line react-internal/no-production-logging
6265
Function.prototype.apply.call(console[level], console, args);
66+
isWritingAppendedStack = false;
6367
}
6468
}

0 commit comments

Comments
 (0)