Skip to content

Commit

Permalink
Revert "Track Owner for Server Components in DEV (#28753)"
Browse files Browse the repository at this point in the history
This reverts commit e0455fe.
  • Loading branch information
kassens committed Apr 11, 2024
1 parent e0455fe commit 87b495f
Show file tree
Hide file tree
Showing 20 changed files with 158 additions and 291 deletions.
25 changes: 7 additions & 18 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,6 @@ function createElement(
type: mixed,
key: mixed,
props: mixed,
owner: null | ReactComponentInfo, // DEV-only
): React$Element<any> {
let element: any;
if (__DEV__ && enableRefAsProp) {
Expand All @@ -493,7 +492,7 @@ function createElement(
type,
key,
props,
_owner: owner,
_owner: null,
}: any);
Object.defineProperty(element, 'ref', {
enumerable: false,
Expand All @@ -510,7 +509,7 @@ function createElement(
props,

// Record the component responsible for creating this element.
_owner: owner,
_owner: null,
}: any);
}

Expand Down Expand Up @@ -844,12 +843,7 @@ function parseModelTuple(
if (tuple[0] === REACT_ELEMENT_TYPE) {
// TODO: Consider having React just directly accept these arrays as elements.
// Or even change the ReactElement type to be an array.
return createElement(
tuple[1],
tuple[2],
tuple[3],
__DEV__ ? (tuple: any)[4] : null,
);
return createElement(tuple[1], tuple[2], tuple[3]);
}
return value;
}
Expand Down Expand Up @@ -1127,14 +1121,12 @@ function resolveConsoleEntry(
);
}

const payload: [string, string, null | ReactComponentInfo, string, mixed] =
parseModel(response, value);
const payload: [string, string, string, mixed] = parseModel(response, value);
const methodName = payload[0];
// TODO: Restore the fake stack before logging.
// const stackTrace = payload[1];
// const owner = payload[2];
const env = payload[3];
const args = payload.slice(4);
const env = payload[2];
const args = payload.slice(3);
printToConsole(methodName, args, env);
}

Expand Down Expand Up @@ -1283,10 +1275,7 @@ function processFullRow(
}
case 68 /* "D" */: {
if (__DEV__) {
const debugInfo: ReactComponentInfo | ReactAsyncInfo = parseModel(
response,
row,
);
const debugInfo = JSON.parse(row);
resolveDebugInfo(response, id, debugInfo);
return;
}
Expand Down
58 changes: 5 additions & 53 deletions packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ describe('ReactFlight', () => {
const rootModel = await ReactNoopFlightClient.read(transport);
const greeting = rootModel.greeting;
expect(greeting._debugInfo).toEqual(
__DEV__ ? [{name: 'Greeting', env: 'Server', owner: null}] : undefined,
__DEV__ ? [{name: 'Greeting', env: 'Server'}] : undefined,
);
ReactNoop.render(greeting);
});
Expand All @@ -241,7 +241,7 @@ describe('ReactFlight', () => {
await act(async () => {
const promise = ReactNoopFlightClient.read(transport);
expect(promise._debugInfo).toEqual(
__DEV__ ? [{name: 'Greeting', env: 'Server', owner: null}] : undefined,
__DEV__ ? [{name: 'Greeting', env: 'Server'}] : undefined,
);
ReactNoop.render(await promise);
});
Expand Down Expand Up @@ -2072,21 +2072,19 @@ describe('ReactFlight', () => {
await act(async () => {
const promise = ReactNoopFlightClient.read(transport);
expect(promise._debugInfo).toEqual(
__DEV__
? [{name: 'ServerComponent', env: 'Server', owner: null}]
: undefined,
__DEV__ ? [{name: 'ServerComponent', env: 'Server'}] : undefined,
);
const result = await promise;
const thirdPartyChildren = await result.props.children[1];
// We expect the debug info to be transferred from the inner stream to the outer.
expect(thirdPartyChildren[0]._debugInfo).toEqual(
__DEV__
? [{name: 'ThirdPartyComponent', env: 'third-party', owner: null}]
? [{name: 'ThirdPartyComponent', env: 'third-party'}]
: undefined,
);
expect(thirdPartyChildren[1]._debugInfo).toEqual(
__DEV__
? [{name: 'ThirdPartyLazyComponent', env: 'third-party', owner: null}]
? [{name: 'ThirdPartyLazyComponent', env: 'third-party'}]
: undefined,
);
ReactNoop.render(result);
Expand Down Expand Up @@ -2147,50 +2145,4 @@ describe('ReactFlight', () => {
expect(loggedFn).not.toBe(foo);
expect(loggedFn.toString()).toBe(foo.toString());
});

it('uses the server component debug info as the element owner in DEV', async () => {
function Container({children}) {
return children;
}

function Greeting({firstName}) {
// We can't use JSX here because it'll use the Client React.
return ReactServer.createElement(
Container,
null,
ReactServer.createElement('span', null, 'Hello, ', firstName),
);
}

const model = {
greeting: ReactServer.createElement(Greeting, {firstName: 'Seb'}),
};

const transport = ReactNoopFlightServer.render(model);

await act(async () => {
const rootModel = await ReactNoopFlightClient.read(transport);
const greeting = rootModel.greeting;
// We've rendered down to the span.
expect(greeting.type).toBe('span');
if (__DEV__) {
const greetInfo = {name: 'Greeting', env: 'Server', owner: null};
expect(greeting._debugInfo).toEqual([
greetInfo,
{name: 'Container', env: 'Server', owner: greetInfo},
]);
// The owner that created the span was the outer server component.
// We expect the debug info to be referentially equal to the owner.
expect(greeting._owner).toBe(greeting._debugInfo[0]);
} else {
expect(greeting._debugInfo).toBe(undefined);
expect(greeting._owner).toBe(
gate(flags => flags.disableStringRefs) ? undefined : null,
);
}
ReactNoop.render(greeting);
});

expect(ReactNoop).toMatchRenderedOutput(<span>Hello, Seb</span>);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ describe('component stack', () => {
{
name: 'ServerComponent',
env: 'Server',
owner: null,
},
];
const Parent = () => ChildPromise;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ import {
import {disableLogs, reenableLogs} from './DevToolsConsolePatching';

let prefix;
export function describeBuiltInComponentFrame(name: string): string {
export function describeBuiltInComponentFrame(
name: string,
ownerFn: void | null | Function,
): string {
if (prefix === undefined) {
// Extract the VM specific prefix used by each line.
try {
Expand All @@ -48,7 +51,10 @@ export function describeBuiltInComponentFrame(name: string): string {
}

export function describeDebugInfoFrame(name: string, env: ?string): string {
return describeBuiltInComponentFrame(name + (env ? ' (' + env + ')' : ''));
return describeBuiltInComponentFrame(
name + (env ? ' (' + env + ')' : ''),
null,
);
}

let reentry = false;
Expand Down Expand Up @@ -286,13 +292,15 @@ export function describeNativeComponentFrame(

export function describeClassComponentFrame(
ctor: Function,
ownerFn: void | null | Function,
currentDispatcherRef: CurrentDispatcherRef,
): string {
return describeNativeComponentFrame(ctor, true, currentDispatcherRef);
}

export function describeFunctionComponentFrame(
fn: Function,
ownerFn: void | null | Function,
currentDispatcherRef: CurrentDispatcherRef,
): string {
return describeNativeComponentFrame(fn, false, currentDispatcherRef);
Expand All @@ -305,6 +313,7 @@ function shouldConstruct(Component: Function) {

export function describeUnknownElementTypeFrameInDEV(
type: any,
ownerFn: void | null | Function,
currentDispatcherRef: CurrentDispatcherRef,
): string {
if (!__DEV__) {
Expand All @@ -321,29 +330,31 @@ export function describeUnknownElementTypeFrameInDEV(
);
}
if (typeof type === 'string') {
return describeBuiltInComponentFrame(type);
return describeBuiltInComponentFrame(type, ownerFn);
}
switch (type) {
case SUSPENSE_NUMBER:
case SUSPENSE_SYMBOL_STRING:
return describeBuiltInComponentFrame('Suspense');
return describeBuiltInComponentFrame('Suspense', ownerFn);
case SUSPENSE_LIST_NUMBER:
case SUSPENSE_LIST_SYMBOL_STRING:
return describeBuiltInComponentFrame('SuspenseList');
return describeBuiltInComponentFrame('SuspenseList', ownerFn);
}
if (typeof type === 'object') {
switch (type.$$typeof) {
case FORWARD_REF_NUMBER:
case FORWARD_REF_SYMBOL_STRING:
return describeFunctionComponentFrame(
type.render,
ownerFn,
currentDispatcherRef,
);
case MEMO_NUMBER:
case MEMO_SYMBOL_STRING:
// Memo may contain any component type so we recursively resolve it.
return describeUnknownElementTypeFrameInDEV(
type.type,
ownerFn,
currentDispatcherRef,
);
case LAZY_NUMBER:
Expand All @@ -355,6 +366,7 @@ export function describeUnknownElementTypeFrameInDEV(
// Lazy may contain any component type so we recursively resolve it.
return describeUnknownElementTypeFrameInDEV(
init(payload),
ownerFn,
currentDispatcherRef,
);
} catch (x) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,30 +39,38 @@ export function describeFiber(
ClassComponent,
} = workTagMap;

const owner: null | Function = __DEV__
? workInProgress._debugOwner
? workInProgress._debugOwner.type
: null
: null;
switch (workInProgress.tag) {
case HostComponent:
return describeBuiltInComponentFrame(workInProgress.type);
return describeBuiltInComponentFrame(workInProgress.type, owner);
case LazyComponent:
return describeBuiltInComponentFrame('Lazy');
return describeBuiltInComponentFrame('Lazy', owner);
case SuspenseComponent:
return describeBuiltInComponentFrame('Suspense');
return describeBuiltInComponentFrame('Suspense', owner);
case SuspenseListComponent:
return describeBuiltInComponentFrame('SuspenseList');
return describeBuiltInComponentFrame('SuspenseList', owner);
case FunctionComponent:
case IndeterminateComponent:
case SimpleMemoComponent:
return describeFunctionComponentFrame(
workInProgress.type,
owner,
currentDispatcherRef,
);
case ForwardRef:
return describeFunctionComponentFrame(
workInProgress.type.render,
owner,
currentDispatcherRef,
);
case ClassComponent:
return describeClassComponentFrame(
workInProgress.type,
owner,
currentDispatcherRef,
);
default:
Expand Down
53 changes: 18 additions & 35 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1952,24 +1952,15 @@ export function attach(
const {key} = fiber;
const displayName = getDisplayNameForFiber(fiber);
const elementType = getElementTypeForFiber(fiber);
const debugOwner = fiber._debugOwner;
const {_debugOwner} = fiber;

// Ideally we should call getFiberIDThrows() for _debugOwner,
// since owners are almost always higher in the tree (and so have already been processed),
// but in some (rare) instances reported in open source, a descendant mounts before an owner.
// Since this is a DEV only field it's probably okay to also just lazily generate and ID here if needed.
// See https://github.com/facebook/react/issues/21445
let ownerID: number;
if (debugOwner != null) {
if (typeof debugOwner.tag === 'number') {
ownerID = getOrGenerateFiberID((debugOwner: any));
} else {
// TODO: Track Server Component Owners.
ownerID = 0;
}
} else {
ownerID = 0;
}
const ownerID =
_debugOwner != null ? getOrGenerateFiberID(_debugOwner) : 0;
const parentID = parentFiber ? getFiberIDThrows(parentFiber) : 0;

const displayNameStringID = getStringID(displayName);
Expand Down Expand Up @@ -3113,17 +3104,15 @@ export function attach(
return null;
}

const {_debugOwner} = fiber;

const owners: Array<SerializedElement> = [fiberToSerializedElement(fiber)];

let owner = fiber._debugOwner;
while (owner != null) {
if (typeof owner.tag === 'number') {
const ownerFiber: Fiber = (owner: any); // Refined
owners.unshift(fiberToSerializedElement(ownerFiber));
owner = ownerFiber._debugOwner;
} else {
// TODO: Track Server Component Owners.
break;
if (_debugOwner) {
let owner: null | Fiber = _debugOwner;
while (owner !== null) {
owners.unshift(fiberToSerializedElement(owner));
owner = owner._debugOwner || null;
}
}

Expand Down Expand Up @@ -3184,7 +3173,7 @@ export function attach(
}

const {
_debugOwner: debugOwner,
_debugOwner,
stateNode,
key,
memoizedProps,
Expand Down Expand Up @@ -3311,19 +3300,13 @@ export function attach(
context = {value: context};
}

let owners: null | Array<SerializedElement> = null;
let owner = debugOwner;
while (owner != null) {
if (typeof owner.tag === 'number') {
const ownerFiber: Fiber = (owner: any); // Refined
if (owners === null) {
owners = [];
}
owners.push(fiberToSerializedElement(ownerFiber));
owner = ownerFiber._debugOwner;
} else {
// TODO: Track Server Component Owners.
break;
let owners = null;
if (_debugOwner) {
owners = ([]: Array<SerializedElement>);
let owner: null | Fiber = _debugOwner;
while (owner !== null) {
owners.push(fiberToSerializedElement(owner));
owner = owner._debugOwner || null;
}
}

Expand Down
Loading

0 comments on commit 87b495f

Please sign in to comment.