Skip to content

Commit

Permalink
[Fiber] Use Owner/JSX Stack When Appending Stacks to Console (#29206)
Browse files Browse the repository at this point in the history
This one should be fully behind the `enableOwnerStacks` flag.

Instead of printing the parent Component stack all the way to the root,
this now prints the owner stack of every JSX callsite. It also includes
intermediate callsites between the Component and the JSX call so it has
potentially more frames. Mainly it provides the line number of the JSX
callsite. In terms of the number of components is a subset of the parent
component stack so it's less information in that regard. This is usually
better since it's more focused on components that might affect the
output but if it's contextual based on rendering it's still good to have
parent stack. Therefore, I still use the parent stack when printing DOM
nesting warnings but I plan on switching that format to a diff view
format instead (Next.js already reformats the parent stack like this).

__Follow ups__

- Server Components show up in the owner stack for client logs but logs
done by Server Components don't yet get their owner stack printed as
they're replayed. They're also not yet printed in the server logs of the
RSC server.

- Server Component stack frames are formatted as the server and added to
the end but this might be a different format than the browser. E.g. if
server is running V8 and browser is running JSC or vice versa. Ideally
we can reformat them in terms of the client formatting.

- This doesn't yet update Fizz or DevTools. Those will be follow ups.
Fizz still prints parent stacks in the server side logs. The stacks
added to user space `console.error` calls by DevTools still get the
parent stacks instead.

- It also doesn't yet expose these to user space so there's no way to
get them inside `onCaughtError` for example or inside a custom
`console.error` override.

- In another follow up I'll use `console.createTask` instead and
completely remove these stacks if it's available.

DiffTrain build for commit d6cfa0f.
  • Loading branch information
sebmarkbage committed May 25, 2024
1 parent fc578ff commit 43adf5d
Show file tree
Hide file tree
Showing 13 changed files with 318 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<8b9b9798442c5728adfabb7cd8ca5f24>>
* @generated SignedSource<<955b120cad9193034a86d0bde3df7d2a>>
*/

'use strict';
Expand Down Expand Up @@ -949,6 +949,32 @@ function describeFunctionComponentFrame(fn) {
}
}

// TODO: Consider marking the whole bundle instead of these boundaries.

/** @noinline */

function callComponentInDEV(Component, props, secondArg) {
setIsRendering(true);
var result = Component(props, secondArg);
setIsRendering(false);
return result;
}
/** @noinline */

function callRenderInDEV(instance) {
setIsRendering(true);
var result = instance.render();
setIsRendering(false);
return result;
}
/** @noinline */

function callLazyInitInDEV(lazy) {
var payload = lazy._payload;
var init = lazy._init;
return init(payload);
}

function describeFiber(fiber) {
switch (fiber.tag) {
case HostHoistable:
Expand Down Expand Up @@ -1021,8 +1047,6 @@ function getCurrentFiberStackInDev() {
if (current === null) {
return '';
} // Safe because if current fiber exists, we are reconciling,
// and it is guaranteed to be the work-in-progress version.


return getStackByFiberInDevAndProd(current);
}
Expand Down Expand Up @@ -5812,9 +5836,9 @@ function warnOnSymbolType(returnFiber, invalidChild) {
}

function resolveLazy(lazyType) {
var payload = lazyType._payload;
var init = lazyType._init;
return init(payload);
{
return callLazyInitInDEV(lazyType);
}
} // This wrapper function exists because I expect to clone the code in each path
// to be able to optimize each path individually by branching early. This needs
// a compiler or we can do it manually. Helpers that don't need this branching
Expand Down Expand Up @@ -6086,9 +6110,13 @@ function createChildReconciler(shouldTrackSideEffects) {

case REACT_LAZY_TYPE:
{
var payload = newChild._payload;
var init = newChild._init;
return createChild(returnFiber, init(payload), lanes, mergeDebugInfo(debugInfo, newChild._debugInfo) // call merge after init
var resolvedChild;

{
resolvedChild = callLazyInitInDEV(newChild);
}

return createChild(returnFiber, resolvedChild, lanes, mergeDebugInfo(debugInfo, newChild._debugInfo) // call merge after init
);
}
}
Expand Down Expand Up @@ -6172,9 +6200,13 @@ function createChildReconciler(shouldTrackSideEffects) {

case REACT_LAZY_TYPE:
{
var payload = newChild._payload;
var init = newChild._init;
return updateSlot(returnFiber, oldFiber, init(payload), lanes, mergeDebugInfo(debugInfo, newChild._debugInfo));
var resolvedChild;

{
resolvedChild = callLazyInitInDEV(newChild);
}

return updateSlot(returnFiber, oldFiber, resolvedChild, lanes, mergeDebugInfo(debugInfo, newChild._debugInfo));
}
}

Expand Down Expand Up @@ -6241,9 +6273,15 @@ function createChildReconciler(shouldTrackSideEffects) {
}

case REACT_LAZY_TYPE:
var payload = newChild._payload;
var init = newChild._init;
return updateFromMap(existingChildren, returnFiber, newIdx, init(payload), lanes, mergeDebugInfo(debugInfo, newChild._debugInfo));
{
var resolvedChild;

{
resolvedChild = callLazyInitInDEV(newChild);
}

return updateFromMap(existingChildren, returnFiber, newIdx, resolvedChild, lanes, mergeDebugInfo(debugInfo, newChild._debugInfo));
}
}

if (isArray(newChild) || getIteratorFn(newChild) || enableAsyncIterableChildren ) {
Expand Down Expand Up @@ -6317,10 +6355,16 @@ function createChildReconciler(shouldTrackSideEffects) {
break;

case REACT_LAZY_TYPE:
var payload = child._payload;
var init = child._init;
warnOnInvalidKey(init(payload), knownKeys, returnFiber);
break;
{
var resolvedChild;

{
resolvedChild = callLazyInitInDEV(child);
}

warnOnInvalidKey(resolvedChild, knownKeys, returnFiber);
break;
}
}
}

Expand Down Expand Up @@ -7444,7 +7488,7 @@ function renderWithHooks(current, workInProgress, Component, props, secondArg, n

var shouldDoubleRenderDEV = debugRenderPhaseSideEffectsForStrictMode ;
shouldDoubleInvokeUserFnsInHooksDEV = shouldDoubleRenderDEV;
var children = Component(props, secondArg);
var children = callComponentInDEV(Component, props, secondArg) ;
shouldDoubleInvokeUserFnsInHooksDEV = false; // Check if there was a render phase update

if (didScheduleRenderPhaseUpdateDuringThisPass) {
Expand Down Expand Up @@ -7584,7 +7628,7 @@ function renderWithHooksAgain(workInProgress, Component, props, secondArg) {
}

ReactSharedInternals.H = HooksDispatcherOnRerenderInDEV ;
children = Component(props, secondArg);
children = callComponentInDEV(Component, props, secondArg) ;
} while (didScheduleRenderPhaseUpdateDuringThisPass);

return children;
Expand Down Expand Up @@ -12300,9 +12344,7 @@ function updateForwardRef(current, workInProgress, Component, nextProps, renderL
}

{
setIsRendering(true);
nextChildren = renderWithHooks(current, workInProgress, render, propsWithoutRef, ref, renderLanes);
setIsRendering(false);
}

{
Expand Down Expand Up @@ -12769,9 +12811,7 @@ function updateFunctionComponent(current, workInProgress, Component, nextProps,
}

{
setIsRendering(true);
nextChildren = renderWithHooks(current, workInProgress, Component, nextProps, context, renderLanes);
setIsRendering(false);
}

{
Expand Down Expand Up @@ -12942,10 +12982,7 @@ function finishClassComponent(current, workInProgress, Component, shouldUpdate,
}

{
setIsRendering(true);
nextChildren = instance.render();

setIsRendering(false);
nextChildren = callRenderInDEV(instance);
}

{
Expand Down Expand Up @@ -13106,9 +13143,12 @@ function mountLazyComponent(_current, workInProgress, elementType, renderLanes)
resetSuspendedCurrentOnMountInLegacyMode(_current, workInProgress);
var props = workInProgress.pendingProps;
var lazyComponent = elementType;
var payload = lazyComponent._payload;
var init = lazyComponent._init;
var Component = init(payload); // Store the unwrapped component in the type.
var Component;

{
Component = callLazyInitInDEV(lazyComponent);
} // Store the unwrapped component in the type.


workInProgress.type = Component;

Expand Down Expand Up @@ -14216,9 +14256,7 @@ function updateContextConsumer(current, workInProgress, renderLanes) {
var newChildren;

{
setIsRendering(true);
newChildren = render(newValue);
setIsRendering(false);
newChildren = callComponentInDEV(render, newValue, undefined);
}

{
Expand Down Expand Up @@ -14558,7 +14596,9 @@ function beginWork(current, workInProgress, renderLanes) {
{
if (workInProgress._debugNeedsRemount && current !== null) {
// This will restart the begin phase with a new fiber.
return remountFiber(current, workInProgress, createFiberFromTypeAndProps(workInProgress.type, workInProgress.key, workInProgress.pendingProps, workInProgress._debugOwner || null, workInProgress.mode, workInProgress.lanes));
var copiedFiber = createFiberFromTypeAndProps(workInProgress.type, workInProgress.key, workInProgress.pendingProps, workInProgress._debugOwner || null, workInProgress.mode, workInProgress.lanes);

return remountFiber(current, workInProgress, copiedFiber);
}
}

Expand Down Expand Up @@ -22900,6 +22940,7 @@ function FiberNode(tag, pendingProps, key, mode) {
// This isn't directly used but is handy for debugging internals:
this._debugInfo = null;
this._debugOwner = null;

this._debugNeedsRemount = false;
this._debugHookTypes = null;

Expand Down Expand Up @@ -22956,6 +22997,7 @@ function createWorkInProgress(current, pendingProps) {
{
// DEV-only fields
workInProgress._debugOwner = current._debugOwner;

workInProgress._debugHookTypes = current._debugHookTypes;
}

Expand Down Expand Up @@ -23463,7 +23505,7 @@ identifierPrefix, onUncaughtError, onCaughtError, onRecoverableError, transition
return root;
}

var ReactVersion = '19.0.0-rc-5a18a922';
var ReactVersion = '19.0.0-rc-999503d6';

/*
* The `'' + value` pattern (used in perf-sensitive code) throws for Symbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<e0bd67bfbe6ced8b6e410eb3974b80d7>>
* @generated SignedSource<<e2dd990512e58f23e127175cf13f6c55>>
*/

"use strict";
Expand Down Expand Up @@ -1723,7 +1723,8 @@ function createChildReconciler(shouldTrackSideEffects) {
);
case REACT_LAZY_TYPE:
var init = newChild._init;
return createChild(returnFiber, init(newChild._payload), lanes);
newChild = init(newChild._payload);
return createChild(returnFiber, newChild, lanes);
}
if (isArrayImpl(newChild) || getIteratorFn(newChild))
return (
Expand Down Expand Up @@ -1771,7 +1772,8 @@ function createChildReconciler(shouldTrackSideEffects) {
case REACT_LAZY_TYPE:
return (
(key = newChild._init),
updateSlot(returnFiber, oldFiber, key(newChild._payload), lanes)
(newChild = key(newChild._payload)),
updateSlot(returnFiber, oldFiber, newChild, lanes)
);
}
if (isArrayImpl(newChild) || getIteratorFn(newChild))
Expand Down Expand Up @@ -1832,11 +1834,12 @@ function createChildReconciler(shouldTrackSideEffects) {
);
case REACT_LAZY_TYPE:
var init = newChild._init;
newChild = init(newChild._payload);
return updateFromMap(
existingChildren,
returnFiber,
newIdx,
init(newChild._payload),
newChild,
lanes
);
}
Expand Down Expand Up @@ -9300,7 +9303,7 @@ var devToolsConfig$jscomp$inline_1042 = {
throw Error("TestRenderer does not support findFiberByHostInstance()");
},
bundleType: 0,
version: "19.0.0-rc-9c2862e7",
version: "19.0.0-rc-c9fc27c4",
rendererPackageName: "react-test-renderer"
};
var internals$jscomp$inline_1229 = {
Expand Down Expand Up @@ -9331,7 +9334,7 @@ var internals$jscomp$inline_1229 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-rc-9c2862e7"
reconcilerVersion: "19.0.0-rc-c9fc27c4"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1230 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<cd5cd2d51aad2dcc6d485f9a9bc90b90>>
* @generated SignedSource<<f18d6d4dff134392d4dfcaa89af16bb0>>
*/

"use strict";
Expand Down Expand Up @@ -1811,7 +1811,8 @@ function createChildReconciler(shouldTrackSideEffects) {
);
case REACT_LAZY_TYPE:
var init = newChild._init;
return createChild(returnFiber, init(newChild._payload), lanes);
newChild = init(newChild._payload);
return createChild(returnFiber, newChild, lanes);
}
if (isArrayImpl(newChild) || getIteratorFn(newChild))
return (
Expand Down Expand Up @@ -1859,7 +1860,8 @@ function createChildReconciler(shouldTrackSideEffects) {
case REACT_LAZY_TYPE:
return (
(key = newChild._init),
updateSlot(returnFiber, oldFiber, key(newChild._payload), lanes)
(newChild = key(newChild._payload)),
updateSlot(returnFiber, oldFiber, newChild, lanes)
);
}
if (isArrayImpl(newChild) || getIteratorFn(newChild))
Expand Down Expand Up @@ -1920,11 +1922,12 @@ function createChildReconciler(shouldTrackSideEffects) {
);
case REACT_LAZY_TYPE:
var init = newChild._init;
newChild = init(newChild._payload);
return updateFromMap(
existingChildren,
returnFiber,
newIdx,
init(newChild._payload),
newChild,
lanes
);
}
Expand Down Expand Up @@ -9943,7 +9946,7 @@ var devToolsConfig$jscomp$inline_1105 = {
throw Error("TestRenderer does not support findFiberByHostInstance()");
},
bundleType: 0,
version: "19.0.0-rc-6cf53e9c",
version: "19.0.0-rc-77c04414",
rendererPackageName: "react-test-renderer"
};
(function (internals) {
Expand Down Expand Up @@ -9987,7 +9990,7 @@ var devToolsConfig$jscomp$inline_1105 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-rc-6cf53e9c"
reconcilerVersion: "19.0.0-rc-77c04414"
});
exports._Scheduler = Scheduler;
exports.act = act;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<f3f0a5ed50b6bab3eafa955dec73aece>>
* @generated SignedSource<<498b38ab8fbbf44800642c8dc0009e44>>
*/

'use strict';
Expand Down Expand Up @@ -970,6 +970,10 @@ var didWarnAboutKeySpread = {};
*/

function jsxDEV$1(type, config, maybeKey, isStaticChildren, source, self) {
return jsxDEVImpl(type, config, maybeKey, isStaticChildren, source, self);
}

function jsxDEVImpl(type, config, maybeKey, isStaticChildren, source, self, debugStack, debugTask) {
{
if (!isValidElementType(type)) {
// This is an invalid element type.
Expand Down
Loading

0 comments on commit 43adf5d

Please sign in to comment.