Skip to content

Commit 4169420

Browse files
authored
Refactor Component Stack Traces (#18495)
* Add feature flag * Split stack from current fiber You can get stack from any fiber, not just current. * Refactor description of component frames These should use fiber tags for switching. This also puts the relevant code behind DEV flags. * We no longer expose StrictMode in component stacks They're not super useful and will go away later anyway. * Update tests Context is no longer part of SSR stacks. This was already the case on the client. forwardRef no longer is wrapped on the stack. It's still in getComponentName but it's probably just noise in stacks. Eventually we'll remove the wrapper so it'll go away anyway. If we use native stack frames they won't have this extra wrapper. It also doesn't pick up displayName from the outer wrapper. We could maybe transfer it but this will also be fixed by removing the wrapper. * Forward displayName onto the inner function for forwardRef and memo in DEV This allows them to show up in stack traces. I'm not doing this for lazy because lazy is supposed to be called on the consuming side so you shouldn't assign it a name on that end. Especially not one that mutates the inner. * Use multiple instances of the fake component We mutate the inner component for its name so we need multiple copies.
1 parent a387566 commit 4169420

33 files changed

Lines changed: 327 additions & 138 deletions

packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -619,11 +619,11 @@ exports[`Store should show the right display names for special component types 1
619619
<MyComponent>
620620
<MyComponent> [ForwardRef]
621621
▾ <Anonymous> [ForwardRef]
622-
<MyComponent>
622+
<MyComponent2>
623623
<Custom> [ForwardRef]
624-
<MyComponent> [Memo]
624+
<MyComponent4> [Memo]
625625
▾ <MyComponent> [Memo]
626626
<MyComponent> [ForwardRef]
627627
▾ <Suspense>
628-
<MyComponent>
628+
<MyComponent5>
629629
`;

packages/react-devtools-shared/src/__tests__/store-test.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -856,16 +856,20 @@ describe('Store', () => {
856856

857857
const MyComponent = (props, ref) => null;
858858
const FowardRefComponent = React.forwardRef(MyComponent);
859+
const MyComponent2 = (props, ref) => null;
859860
const FowardRefComponentWithAnonymousFunction = React.forwardRef(() => (
860-
<MyComponent />
861+
<MyComponent2 />
861862
));
863+
const MyComponent3 = (props, ref) => null;
862864
const FowardRefComponentWithCustomDisplayName = React.forwardRef(
863-
MyComponent,
865+
MyComponent3,
864866
);
865867
FowardRefComponentWithCustomDisplayName.displayName = 'Custom';
866-
const MemoComponent = React.memo(MyComponent);
868+
const MyComponent4 = (props, ref) => null;
869+
const MemoComponent = React.memo(MyComponent4);
867870
const MemoForwardRefComponent = React.memo(FowardRefComponent);
868-
const LazyComponent = React.lazy(() => fakeImport(MyComponent));
871+
const MyComponent5 = (props, ref) => null;
872+
const LazyComponent = React.lazy(() => fakeImport(MyComponent5));
869873

870874
const App = () => (
871875
<React.Fragment>

packages/react-dom/src/__tests__/findDOMNode-test.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ describe('findDOMNode', () => {
124124
'Learn more about using refs safely here: ' +
125125
'https://fb.me/react-strict-mode-find-node' +
126126
'\n in div (at **)' +
127-
'\n in StrictMode (at **)' +
128127
'\n in ContainsStrictModeChild (at **)',
129128
]);
130129
expect(match).toBe(child);
@@ -154,8 +153,7 @@ describe('findDOMNode', () => {
154153
'Learn more about using refs safely here: ' +
155154
'https://fb.me/react-strict-mode-find-node' +
156155
'\n in div (at **)' +
157-
'\n in IsInStrictMode (at **)' +
158-
'\n in StrictMode (at **)',
156+
'\n in IsInStrictMode (at **)',
159157
]);
160158
expect(match).toBe(child);
161159
});

packages/react-dom/src/server/ReactPartialRenderer.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import type {ReactProvider, ReactContext} from 'shared/ReactTypes';
1515
import * as React from 'react';
1616
import invariant from 'shared/invariant';
1717
import getComponentName from 'shared/getComponentName';
18-
import describeComponentFrame from 'shared/describeComponentFrame';
18+
import {describeUnknownElementTypeFrameInDEV} from 'shared/ReactComponentStackFrame';
1919
import ReactSharedInternals from 'shared/ReactSharedInternals';
2020
import {
2121
warnAboutDeprecatedLifecycles,
@@ -112,11 +112,11 @@ if (__DEV__) {
112112
};
113113

114114
describeStackFrame = function(element): string {
115-
const source = element._source;
116-
const type = element.type;
117-
const name = getComponentName(type);
118-
const ownerName = null;
119-
return describeComponentFrame(name, source, ownerName);
115+
return describeUnknownElementTypeFrameInDEV(
116+
element.type,
117+
element._source,
118+
null,
119+
);
120120
};
121121

122122
pushCurrentDebugStack = function(stack: Array<Frame>) {

packages/react-native-renderer/src/ReactNativeRenderer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
getPublicRootInstance,
2626
} from 'react-reconciler/src/ReactFiberReconciler';
2727
// TODO: direct imports like some-package/src/* are bad. Fix me.
28-
import {getStackByFiberInDevAndProd} from 'react-reconciler/src/ReactCurrentFiber';
28+
import {getStackByFiberInDevAndProd} from 'react-reconciler/src/ReactFiberComponentStack';
2929
import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal';
3030
import {
3131
setBatchingImplementation,

packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,6 @@ describe('ReactFabric', () => {
773773
'Learn more about using refs safely here: ' +
774774
'https://fb.me/react-strict-mode-find-node' +
775775
'\n in RCTView (at **)' +
776-
'\n in StrictMode (at **)' +
777776
'\n in ContainsStrictModeChild (at **)',
778777
]);
779778
expect(match).toBe(child);
@@ -811,8 +810,7 @@ describe('ReactFabric', () => {
811810
'Learn more about using refs safely here: ' +
812811
'https://fb.me/react-strict-mode-find-node' +
813812
'\n in RCTView (at **)' +
814-
'\n in IsInStrictMode (at **)' +
815-
'\n in StrictMode (at **)',
813+
'\n in IsInStrictMode (at **)',
816814
]);
817815
expect(match).toBe(child);
818816
});
@@ -846,7 +844,6 @@ describe('ReactFabric', () => {
846844
'Learn more about using refs safely here: ' +
847845
'https://fb.me/react-strict-mode-find-node' +
848846
'\n in RCTView (at **)' +
849-
'\n in StrictMode (at **)' +
850847
'\n in ContainsStrictModeChild (at **)',
851848
]);
852849
expect(match).toBe(child._nativeTag);
@@ -882,8 +879,7 @@ describe('ReactFabric', () => {
882879
'Learn more about using refs safely here: ' +
883880
'https://fb.me/react-strict-mode-find-node' +
884881
'\n in RCTView (at **)' +
885-
'\n in IsInStrictMode (at **)' +
886-
'\n in StrictMode (at **)',
882+
'\n in IsInStrictMode (at **)',
887883
]);
888884
expect(match).toBe(child._nativeTag);
889885
});

packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,6 @@ describe('ReactNative', () => {
483483
'Learn more about using refs safely here: ' +
484484
'https://fb.me/react-strict-mode-find-node' +
485485
'\n in RCTView (at **)' +
486-
'\n in StrictMode (at **)' +
487486
'\n in ContainsStrictModeChild (at **)',
488487
]);
489488
expect(match).toBe(child);
@@ -521,8 +520,7 @@ describe('ReactNative', () => {
521520
'Learn more about using refs safely here: ' +
522521
'https://fb.me/react-strict-mode-find-node' +
523522
'\n in RCTView (at **)' +
524-
'\n in IsInStrictMode (at **)' +
525-
'\n in StrictMode (at **)',
523+
'\n in IsInStrictMode (at **)',
526524
]);
527525
expect(match).toBe(child);
528526
});
@@ -556,7 +554,6 @@ describe('ReactNative', () => {
556554
'Learn more about using refs safely here: ' +
557555
'https://fb.me/react-strict-mode-find-node' +
558556
'\n in RCTView (at **)' +
559-
'\n in StrictMode (at **)' +
560557
'\n in ContainsStrictModeChild (at **)',
561558
]);
562559
expect(match).toBe(child._nativeTag);
@@ -592,8 +589,7 @@ describe('ReactNative', () => {
592589
'Learn more about using refs safely here: ' +
593590
'https://fb.me/react-strict-mode-find-node' +
594591
'\n in RCTView (at **)' +
595-
'\n in IsInStrictMode (at **)' +
596-
'\n in StrictMode (at **)',
592+
'\n in IsInStrictMode (at **)',
597593
]);
598594
expect(match).toBe(child._nativeTag);
599595
});

packages/react-reconciler/src/ReactCapturedValue.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
import type {Fiber} from './ReactFiber';
1111

12-
import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
12+
import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';
1313

1414
export type CapturedValue<T> = {|
1515
value: T,

packages/react-reconciler/src/ReactChildFiber.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,8 @@ import {
4444
createFiberFromPortal,
4545
} from './ReactFiber';
4646
import {emptyRefsObject} from './ReactFiberClassComponent';
47-
import {
48-
getCurrentFiberStackInDev,
49-
getStackByFiberInDevAndProd,
50-
} from './ReactCurrentFiber';
47+
import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';
48+
import {getCurrentFiberStackInDev} from './ReactCurrentFiber';
5149
import {isCompatibleFamilyForHotReloading} from './ReactFiberHotReloading';
5250
import {StrictMode} from './ReactTypeOfMode';
5351

packages/react-reconciler/src/ReactCurrentFiber.js

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -10,50 +10,11 @@
1010
import type {Fiber} from './ReactFiber';
1111

1212
import ReactSharedInternals from 'shared/ReactSharedInternals';
13-
import {
14-
HostRoot,
15-
HostPortal,
16-
HostText,
17-
Fragment,
18-
ContextProvider,
19-
ContextConsumer,
20-
} from './ReactWorkTags';
21-
import describeComponentFrame from 'shared/describeComponentFrame';
13+
import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';
2214
import getComponentName from 'shared/getComponentName';
2315

2416
const ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;
2517

26-
function describeFiber(fiber: Fiber): string {
27-
switch (fiber.tag) {
28-
case HostRoot:
29-
case HostPortal:
30-
case HostText:
31-
case Fragment:
32-
case ContextProvider:
33-
case ContextConsumer:
34-
return '';
35-
default:
36-
const owner = fiber._debugOwner;
37-
const source = fiber._debugSource;
38-
const name = getComponentName(fiber.type);
39-
let ownerName = null;
40-
if (owner) {
41-
ownerName = getComponentName(owner.type);
42-
}
43-
return describeComponentFrame(name, source, ownerName);
44-
}
45-
}
46-
47-
export function getStackByFiberInDevAndProd(workInProgress: Fiber): string {
48-
let info = '';
49-
let node = workInProgress;
50-
do {
51-
info += describeFiber(node);
52-
node = node.return;
53-
} while (node);
54-
return info;
55-
}
56-
5718
export let current: Fiber | null = null;
5819
export let isRendering: boolean = false;
5920

0 commit comments

Comments
 (0)