Skip to content

Commit

Permalink
Update on "[compiler] Fix issue with macro arguments being outlined"
Browse files Browse the repository at this point in the history
Summary:
Fixes issue documented by #30435. We change the pipeline order so that outlining comes after tracking macro operands, and any function that is referenced in a macro will now not be outlined.

[ghstack-poisoned]
  • Loading branch information
mvitousek committed Aug 2, 2024
2 parents 5302bbb + e29a0e7 commit 9f61c18
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 23 deletions.
51 changes: 37 additions & 14 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2695,11 +2695,11 @@ export function attach(
// If we're tracing updates and we've bailed out before reaching a host node,
// we should fall back to recursively marking the nearest host descendants for highlight.
if (traceNearestHostComponentUpdate) {
const hostFibers = findAllCurrentHostFibers(
const hostInstances = findAllCurrentHostInstances(
getFiberInstanceThrows(nextFiber),
);
hostFibers.forEach(hostFiber => {
traceUpdatesForNodes.add(hostFiber.stateNode);
hostInstances.forEach(hostInstance => {
traceUpdatesForNodes.add(hostInstance);
});
}
}
Expand Down Expand Up @@ -2943,31 +2943,54 @@ export function attach(
currentRootID = -1;
}

function findAllCurrentHostFibers(
function getResourceInstance(fiber: Fiber): HostInstance | null {
if (fiber.tag === HostHoistable) {
const resource = fiber.memoizedState;
// Feature Detect a DOM Specific Instance of a Resource
if (
typeof resource === 'object' &&
resource !== null &&
resource.instance != null
) {
return resource.instance;
}
}
return null;
}

function findAllCurrentHostInstances(
fiberInstance: FiberInstance,
): $ReadOnlyArray<Fiber> {
const fibers = [];
): $ReadOnlyArray<HostInstance> {
const hostInstances = [];
const fiber = findCurrentFiberUsingSlowPathByFiberInstance(fiberInstance);
if (!fiber) {
return fibers;
return hostInstances;
}

// Next we'll drill down this component to find all HostComponent/Text.
let node: Fiber = fiber;
while (true) {
if (node.tag === HostComponent || node.tag === HostText) {
fibers.push(node);
if (
node.tag === HostComponent ||
node.tag === HostText ||
node.tag === HostSingleton ||
node.tag === HostHoistable
) {
const hostInstance = node.stateNode || getResourceInstance(node);
if (hostInstance) {
hostInstances.push(hostInstance);
}
} else if (node.child) {
node.child.return = node;
node = node.child;
continue;
}
if (node === fiber) {
return fibers;
return hostInstances;
}
while (!node.sibling) {
if (!node.return || node.return === fiber) {
return fibers;
return hostInstances;
}
node = node.return;
}
Expand All @@ -2976,7 +2999,7 @@ export function attach(
}
// Flow needs the return here, but ESLint complains about it.
// eslint-disable-next-line no-unreachable
return fibers;
return hostInstances;
}

function findHostInstancesForElementID(id: number) {
Expand All @@ -2996,8 +3019,8 @@ export function attach(
return null;
}

const hostFibers = findAllCurrentHostFibers(devtoolsInstance);
return hostFibers.map(hostFiber => hostFiber.stateNode).filter(Boolean);
const hostInstances = findAllCurrentHostInstances(devtoolsInstance);
return hostInstances;
} catch (err) {
// The fiber might have unmounted by now.
return null;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export type GetElementIDForHostInstance = (
) => number | null;
export type FindHostInstancesForElementID = (
id: number,
) => ?Array<HostInstance>;
) => null | $ReadOnlyArray<HostInstance>;

export type ReactProviderType<T> = {
$$typeof: symbol | number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@ export function hideOverlay(agent: Agent): void {
: hideOverlayWeb();
}

function showOverlayNative(elements: Array<HostInstance>, agent: Agent): void {
function showOverlayNative(
elements: $ReadOnlyArray<HostInstance>,
agent: Agent,
): void {
agent.emit('showNativeHighlight', elements);
}

function showOverlayWeb(
elements: Array<HTMLElement>,
elements: $ReadOnlyArray<HTMLElement>,
componentName: string | null,
agent: Agent,
hideAfterTimeout: boolean,
Expand All @@ -64,12 +67,17 @@ function showOverlayWeb(
}

export function showOverlay(
elements: Array<HostInstance>,
elements: $ReadOnlyArray<HostInstance>,
componentName: string | null,
agent: Agent,
hideAfterTimeout: boolean,
): void {
return isReactNativeEnvironment()
? showOverlayNative(elements, agent)
: showOverlayWeb((elements: any), componentName, agent, hideAfterTimeout);
: showOverlayWeb(
(elements: $ReadOnlyArray<any>),
componentName,
agent,
hideAfterTimeout,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export default class Overlay {
}
}

inspect(nodes: Array<HTMLElement>, name?: ?string) {
inspect(nodes: $ReadOnlyArray<HTMLElement>, name?: ?string) {
// We can't get the size of text nodes or comment nodes. React as of v15
// heavily uses comment nodes to delimit text.
const elements = nodes.filter(node => node.nodeType === Node.ELEMENT_NODE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import Agent from 'react-devtools-shared/src/backend/agent';
import {hideOverlay, showOverlay} from './Highlighter';

import type {BackendBridge} from 'react-devtools-shared/src/bridge';
import type {HostInstance} from '../../types';

// This plug-in provides in-page highlighting of the selected element.
// It is used by the browser extension and the standalone DevTools shell (when connected to a browser).
Expand Down Expand Up @@ -113,8 +112,7 @@ export default function setupHighlighter(
return;
}

const nodes: ?Array<HostInstance> =
renderer.findHostInstancesForElementID(id);
const nodes = renderer.findHostInstancesForElementID(id);

if (nodes != null && nodes[0] != null) {
const node = nodes[0];
Expand Down

0 comments on commit 9f61c18

Please sign in to comment.