Skip to content

Commit

Permalink
[Fiber] Enable Native console.createTask Stacks When Available (#29223)
Browse files Browse the repository at this point in the history
Stacked on #29206 and #29221.

This disables appending owner stacks to console when
`console.createTask` is available in the environment. Instead we rely on
native "async" stacks that end up looking like this with source maps and
ignore list enabled.

<img width="673" alt="Screenshot 2024-05-22 at 4 00 27 PM"
src="https://github.com/facebook/react/assets/63648/5313ed53-b298-4386-8f76-8eb85bdfbbc7">

Unfortunately Chrome requires a string name for each async stack and,
worse, a suffix of `(async)` is automatically added which is very
confusing since it seems like it might be an async component or
something which it is not.

In this case it's not so bad because it's nice to refer to the host
component which otherwise doesn't have a stack frame since it's
internal. However, if there were more owners here there would also be a
`<Counter> (async)` which ends up being kind of duplicative.

If the Chrome DevTools is not open from the start of the app, then
`console.createTask` is disabled and so you lose the stack for those
errors (or those parents if the devtools is opened later). Unlike our
appended ones that are always added. That's unfortunate and likely to be
a bit of a DX issue but it's also nice that it saves on perf in DEV mode
for those cases. Framework dialogs can still surface the stack since we
also track it in user space in parallel.

This currently doesn't track Server Components yet. We need a more
clever hack for that part in a follow up.

I think I probably need to also add something to React DevTools to
disable its stacks for this case too. Since it looks for stacks in the
console.error and adds a stack otherwise. Since we don't add them
anymore from the runtime, the DevTools adds them instead.
  • Loading branch information
sebmarkbage authored May 26, 2024
1 parent b078c81 commit ea6e059
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,10 @@ export function getStackByFiberInDevAndProd(
return '\nError generating stack: ' + x.message + '\n' + x.stack;
}
}

export function supportsNativeConsoleTasks(fiber: Fiber): boolean {
// If this Fiber supports native console.createTask then we are already running
// inside a native async stack trace if it's active - meaning the DevTools is open.
// Ideally we'd detect if this task was created while the DevTools was open or not.
return !!fiber._debugTask;
}
10 changes: 8 additions & 2 deletions packages/react-devtools-shared/src/backend/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ import type {
import {format, formatWithStyles} from './utils';

import {getInternalReactConstants, getDispatcherRef} from './renderer';
import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack';
import {
getStackByFiberInDevAndProd,
supportsNativeConsoleTasks,
} from './DevToolsFiberComponentStack';
import {consoleManagedByDevToolsDuringStrictMode} from 'react-devtools-feature-flags';
import {castBool, castBrowserTheme} from '../utils';

Expand Down Expand Up @@ -235,7 +238,10 @@ export function patch({
}
}

if (shouldAppendWarningStack) {
if (
shouldAppendWarningStack &&
!supportsNativeConsoleTasks(current)
) {
const componentStack = getStackByFiberInDevAndProd(
workTagMap,
current,
Expand Down
7 changes: 7 additions & 0 deletions packages/react-reconciler/src/ReactCurrentFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ export function runWithFiberInDEV<A0, A1, A2, A3, A4, T>(
const previousFiber = current;
setCurrentFiber(fiber);
try {
if (enableOwnerStacks) {
if (fiber !== null && fiber._debugTask) {
return fiber._debugTask.run(
callback.bind(null, arg0, arg1, arg2, arg3, arg4),
);
}
}
return callback(arg0, arg1, arg2, arg3, arg4);
} finally {
current = previousFiber;
Expand Down
9 changes: 8 additions & 1 deletion packages/shared/consoleWithStackDev.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import ReactSharedInternals from 'shared/ReactSharedInternals';
import {enableOwnerStacks} from 'shared/ReactFeatureFlags';

let suppressWarning = false;
export function setSuppressWarning(newSuppressWarning) {
Expand Down Expand Up @@ -36,14 +37,20 @@ export function error(format, ...args) {
}
}

// eslint-disable-next-line react-internal/no-production-logging
const supportsCreateTask = __DEV__ && enableOwnerStacks && !!console.createTask;

function printWarning(level, format, args) {
// When changing this logic, you might want to also
// update consoleWithStackDev.www.js as well.
if (__DEV__) {
const isErrorLogger =
format === '%s\n\n%s\n' || format === '%o\n\n%s\n\n%s\n';

if (ReactSharedInternals.getCurrentStack) {
if (!supportsCreateTask && ReactSharedInternals.getCurrentStack) {
// We only add the current stack to the console when createTask is not supported.
// Since createTask requires DevTools to be open to work, this means that stacks
// can be lost while DevTools isn't open but we can't detect this.
const stack = ReactSharedInternals.getCurrentStack();
if (stack !== '') {
format += '%s';
Expand Down

0 comments on commit ea6e059

Please sign in to comment.