Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DevTools] Rename NativeElement to HostInstance in the Bridge #30491

Merged
merged 9 commits into from
Jul 30, 2024
Merged

[DevTools] Rename NativeElement to HostInstance in the Bridge #30491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c78b88c
Rename NativeNode to HostInstance in the RendererInterface
sebmarkbage Jul 27, 2024
ef3d256
Rename NativeElement to HostInstance in the Bridgd
sebmarkbage Jul 27, 2024
f7a0adb
Rename start/stopInspectingNative to InspectingHost in the Bridge
sebmarkbage Jul 27, 2024
b3508a7
Rename NativeElementsPanel to BuiltinElementsPanel
sebmarkbage Jul 27, 2024
e2dd54a
Clarify types when they're React Native vs HTMLElement
sebmarkbage Jul 27, 2024
2eec856
Rename Native in legacy renderer
sebmarkbage Jul 27, 2024
5cf264a
supportsNativeConsoleTasks -> supportsConsoleTasks
sebmarkbage Jul 27, 2024
09068c3
Rename NativeElement terms to HostInstance in the frontend
sebmarkbage Jul 27, 2024
18f44c6
Fiber renderer
sebmarkbage Jul 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/react-devtools-extensions/src/main/elementSelection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function setReactSelectionFromBrowser(bridge) {
}

// Remember to sync the selection next time we show Components tab.
bridge.send('syncSelectionFromNativeElementsPanel');
bridge.send('syncSelectionFromBuiltinElementsPanel');
}
},
);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-extensions/src/main/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function createBridge() {
});

bridge.addListener(
'syncSelectionToNativeElementsPanel',
'syncSelectionToBuiltinElementsPanel',
setBrowserSelectionFromReact,
);

Expand Down
22 changes: 11 additions & 11 deletions packages/react-devtools-shared/src/backend/agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {currentBridgeProtocol} from 'react-devtools-shared/src/bridge';
import type {BackendBridge} from 'react-devtools-shared/src/bridge';
import type {
InstanceAndStyle,
NativeType,
HostInstance,
OwnersList,
PathFrame,
PathMatch,
Expand Down Expand Up @@ -146,12 +146,12 @@ type PersistedSelection = {

export default class Agent extends EventEmitter<{
hideNativeHighlight: [],
showNativeHighlight: [NativeType],
showNativeHighlight: [HostInstance],
startInspectingNative: [],
stopInspectingNative: [],
shutdown: [],
traceUpdates: [Set<NativeType>],
drawTraceUpdates: [Array<NativeType>],
traceUpdates: [Set<HostInstance>],
drawTraceUpdates: [Array<HostInstance>],
disableTraceUpdates: [],
}> {
_bridge: BackendBridge;
Expand Down Expand Up @@ -212,8 +212,8 @@ export default class Agent extends EventEmitter<{
bridge.addListener('stopProfiling', this.stopProfiling);
bridge.addListener('storeAsGlobal', this.storeAsGlobal);
bridge.addListener(
'syncSelectionFromNativeElementsPanel',
this.syncSelectionFromNativeElementsPanel,
'syncSelectionFromBuiltinElementsPanel',
this.syncSelectionFromBuiltinElementsPanel,
);
bridge.addListener('shutdown', this.shutdown);
bridge.addListener(
Expand Down Expand Up @@ -367,7 +367,7 @@ export default class Agent extends EventEmitter<{
const rendererInterface = this.getBestMatchingRendererInterface(node);
if (rendererInterface != null) {
try {
return rendererInterface.getElementIDForNative(node, true);
return rendererInterface.getElementIDForHostInstance(node, true);
} catch (error) {
// Some old React versions might throw if they can't find a match.
// If so we should ignore it...
Expand Down Expand Up @@ -439,7 +439,7 @@ export default class Agent extends EventEmitter<{
}

// TODO: If there was a way to change the selected DOM element
// in native Elements tab without forcing a switch to it, we'd do it here.
// in built-in Elements tab without forcing a switch to it, we'd do it here.
// For now, it doesn't seem like there is a way to do that:
// https://github.com/bvaughn/react-devtools-experimental/issues/102
// (Setting $0 doesn't work, and calling inspect() switches the tab.)
Expand Down Expand Up @@ -658,7 +658,7 @@ export default class Agent extends EventEmitter<{
}
};

syncSelectionFromNativeElementsPanel: () => void = () => {
syncSelectionFromBuiltinElementsPanel: () => void = () => {
const target = window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0;
if (target == null) {
return;
Expand Down Expand Up @@ -697,7 +697,7 @@ export default class Agent extends EventEmitter<{
};

stopInspectingNative: (selected: boolean) => void = selected => {
this._bridge.send('stopInspectingNative', selected);
this._bridge.send('stopInspectingHost', selected);
};

storeAsGlobal: StoreAsGlobalParams => void = ({
Expand Down Expand Up @@ -768,7 +768,7 @@ export default class Agent extends EventEmitter<{
}
};

onTraceUpdates: (nodes: Set<NativeType>) => void = nodes => {
onTraceUpdates: (nodes: Set<HostInstance>) => void = nodes => {
this.emit('traceUpdates', nodes);
};

Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/backend/console.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
getStackByFiberInDevAndProd,
getOwnerStackByFiberInDev,
supportsOwnerStacks,
supportsNativeConsoleTasks,
supportsConsoleTasks,
} from './fiber/DevToolsFiberComponentStack';
import {formatOwnerStack} from './shared/DevToolsOwnerStack';
import {castBool, castBrowserTheme} from '../utils';
Expand Down Expand Up @@ -251,7 +251,7 @@ export function patch({

if (
consoleSettingsRef.appendComponentStack &&
!supportsNativeConsoleTasks(current)
!supportsConsoleTasks(current)
) {
const enableOwnerStacks = supportsOwnerStacks(current);
let componentStack = '';
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export function getStackByFiberInDevAndProd(
}
}

export function supportsNativeConsoleTasks(fiber: Fiber): boolean {
export function supportsConsoleTasks(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.
Expand Down
22 changes: 11 additions & 11 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ import type {
InspectedElement,
InspectedElementPayload,
InstanceAndStyle,
NativeType,
HostInstance,
PathFrame,
PathMatch,
ProfilingDataBackend,
Expand Down Expand Up @@ -937,7 +937,7 @@ export function attach(

// Highlight updates
let traceUpdatesEnabled: boolean = false;
const traceUpdatesForNodes: Set<NativeType> = new Set();
const traceUpdatesForNodes: Set<HostInstance> = new Set();

function applyComponentFilters(componentFilters: Array<ComponentFilter>) {
hideElementsWithTypes.clear();
Expand Down Expand Up @@ -2862,7 +2862,7 @@ export function attach(
return fibers;
}

function findNativeNodesForElementID(id: number) {
function findHostInstancesForElementID(id: number) {
try {
const fiber = findCurrentFiberUsingSlowPathById(id);
if (fiber === null) {
Expand All @@ -2882,12 +2882,12 @@ export function attach(
return fiber != null ? getDisplayNameForFiber(fiber) : null;
}

function getFiberForNative(hostInstance: NativeType) {
function getFiberForNative(hostInstance: HostInstance) {
return renderer.findFiberByHostInstance(hostInstance);
}

function getElementIDForNative(
hostInstance: NativeType,
function getElementIDForHostInstance(
hostInstance: HostInstance,
findNearestUnfilteredAncestor: boolean = false,
) {
let fiber = renderer.findFiberByHostInstance(hostInstance);
Expand Down Expand Up @@ -3870,9 +3870,9 @@ export function attach(
if (result.hooks !== null) {
console.log('Hooks:', result.hooks);
}
const nativeNodes = findNativeNodesForElementID(id);
if (nativeNodes !== null) {
console.log('Nodes:', nativeNodes);
const hostInstances = findHostInstancesForElementID(id);
if (hostInstances !== null) {
console.log('Nodes:', hostInstances);
}
if (window.chrome || /firefox/i.test(navigator.userAgent)) {
console.log(
Expand Down Expand Up @@ -4655,12 +4655,12 @@ export function attach(
clearWarningsForElementID,
getSerializedElementValueByPath,
deletePath,
findNativeNodesForElementID,
findHostInstancesForElementID,
flushInitialOperations,
getBestMatchForTrackedPath,
getDisplayNameForElementID,
getFiberForNative,
getElementIDForNative,
getElementIDForHostInstance,
getInstanceAndStyle,
getOwnersList,
getPathForElement,
Expand Down
36 changes: 18 additions & 18 deletions packages/react-devtools-shared/src/backend/legacy/renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ import {decorateMany, forceUpdate, restoreMany} from './utils';

import type {
DevToolsHook,
GetElementIDForNative,
GetElementIDForHostInstance,
InspectedElementPayload,
InstanceAndStyle,
NativeType,
HostInstance,
PathFrame,
PathMatch,
RendererInterface,
Expand Down Expand Up @@ -142,33 +142,33 @@ export function attach(
const internalInstanceToRootIDMap: WeakMap<InternalInstance, number> =
new WeakMap();

let getInternalIDForNative: GetElementIDForNative =
((null: any): GetElementIDForNative);
let findNativeNodeForInternalID: (id: number) => ?NativeType;
let getFiberForNative = (node: NativeType) => {
let getElementIDForHostInstance: GetElementIDForHostInstance =
((null: any): GetElementIDForHostInstance);
let findHostInstanceForInternalID: (id: number) => ?HostInstance;
let getFiberForNative = (node: HostInstance) => {
// Not implemented.
return null;
};

if (renderer.ComponentTree) {
getInternalIDForNative = (node, findNearestUnfilteredAncestor) => {
getElementIDForHostInstance = (node, findNearestUnfilteredAncestor) => {
const internalInstance =
renderer.ComponentTree.getClosestInstanceFromNode(node);
return internalInstanceToIDMap.get(internalInstance) || null;
};
findNativeNodeForInternalID = (id: number) => {
findHostInstanceForInternalID = (id: number) => {
const internalInstance = idToInternalInstanceMap.get(id);
return renderer.ComponentTree.getNodeFromInstance(internalInstance);
};
getFiberForNative = (node: NativeType) => {
getFiberForNative = (node: HostInstance) => {
return renderer.ComponentTree.getClosestInstanceFromNode(node);
};
} else if (renderer.Mount.getID && renderer.Mount.getNode) {
getInternalIDForNative = (node, findNearestUnfilteredAncestor) => {
getElementIDForHostInstance = (node, findNearestUnfilteredAncestor) => {
// Not implemented.
return null;
};
findNativeNodeForInternalID = (id: number) => {
findHostInstanceForInternalID = (id: number) => {
// Not implemented.
return null;
};
Expand Down Expand Up @@ -884,9 +884,9 @@ export function attach(
if (result.context !== null) {
console.log('Context:', result.context);
}
const nativeNode = findNativeNodeForInternalID(id);
if (nativeNode !== null) {
console.log('Node:', nativeNode);
const hostInstance = findHostInstanceForInternalID(id);
if (hostInstance !== null) {
console.log('Node:', hostInstance);
}
if (window.chrome || /firefox/i.test(navigator.userAgent)) {
console.log(
Expand Down Expand Up @@ -1112,11 +1112,11 @@ export function attach(
getBestMatchForTrackedPath,
getDisplayNameForElementID,
getFiberForNative,
getElementIDForNative: getInternalIDForNative,
getElementIDForHostInstance,
getInstanceAndStyle,
findNativeNodesForElementID: (id: number) => {
const nativeNode = findNativeNodeForInternalID(id);
return nativeNode == null ? null : [nativeNode];
findHostInstancesForElementID: (id: number) => {
const hostInstance = findHostInstanceForInternalID(id);
return hostInstance == null ? null : [hostInstance];
},
getOwnersList,
getPathForElement,
Expand Down
18 changes: 10 additions & 8 deletions packages/react-devtools-shared/src/backend/types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export type WorkTagMap = {
Throw: WorkTag,
};

export type NativeType = Object;
export type HostInstance = Object;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it a bit confusing, because Host is essentially React DevTools backend, and this is HostInstance, which is not about React DevTools backend, but element instance.

Maybe something like InstanceForHost or ElementInstanceForHost?

Copy link
Collaborator Author

@sebmarkbage sebmarkbage Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I think of Host is the environment that the backend is running inside. E.g. the DOM. Not the backend itself.

It's also what we call this elsewhere in Fiber. It gets shortened into the name Instance inside the "Host Config" but the long name we refer it by is Host Instance. We could potentially rename it to HostInstance even in the Configs and then it would have the same name everywhere.

This name already shows up even in the DevTools interface too e.g. through the injected findFiberByHostInstance.

Also, not all Elements in the React DevTools refer to anything that has a Host Instance. Only a subset of DevTools Elements are represented by a Host Instance. HostText/HostComponent/HostSingleton/HostHoistable.

I have a follow up draft PR where I'm introducing an Instance to back stuff for the DevTools specifically which I'm calling a DevtoolsInstance which is more 1:1 with the Elements tab instances.

Although I think we should probably rename Element to Component in the Elements tab because in React an Element is what you return from JSX but a Component instance is the stateful thing you get as a result of rendering an Element.

Copy link
Collaborator Author

@sebmarkbage sebmarkbage Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated but ReactComponentInfo started out as an idea of being more like an instance of a Server Component but it's not actually a stateful thing and I wonder if it shouldn't just be modeled as a ReactElement. It already has mostly the same fields. It should have props too. ReactElement is missing env but technically it should have it:

react/packages/react-client/src/ReactFlightClient.js

Lines 699 to 703 in ec98d36

// Interestingly we don't actually have the environment name of where
// this JSX was created if it doesn't have an owner but if it does
// it must be the same environment as the owner. We could send it separately
// but it seems a bit unnecessary for this edge case.
env = owner.env;

In this case DevTools would have to deal with React Elements too which makes the Element naming even more confusing.

export type RendererID = number;

type Dispatcher = any;
Expand All @@ -91,11 +91,13 @@ export type GetDisplayNameForElementID = (
findNearestUnfilteredAncestor?: boolean,
) => string | null;

export type GetElementIDForNative = (
component: NativeType,
export type GetElementIDForHostInstance = (
component: HostInstance,
findNearestUnfilteredAncestor?: boolean,
) => number | null;
export type FindNativeNodesForFiberID = (id: number) => ?Array<NativeType>;
export type FindHostInstancesForElementID = (
id: number,
) => ?Array<HostInstance>;

export type ReactProviderType<T> = {
$$typeof: symbol | number,
Expand All @@ -107,7 +109,7 @@ export type Lane = number;
export type Lanes = number;

export type ReactRenderer = {
findFiberByHostInstance: (hostInstance: NativeType) => Fiber | null,
findFiberByHostInstance: (hostInstance: HostInstance) => Fiber | null,
version: string,
rendererPackageName: string,
bundleType: BundleType,
Expand Down Expand Up @@ -358,11 +360,11 @@ export type RendererInterface = {
hookID: ?number,
path: Array<string | number>,
) => void,
findNativeNodesForElementID: FindNativeNodesForFiberID,
findHostInstancesForElementID: FindHostInstancesForElementID,
flushInitialOperations: () => void,
getBestMatchForTrackedPath: () => PathMatch | null,
getFiberForNative: (component: NativeType) => Fiber | null,
getElementIDForNative: GetElementIDForNative,
getFiberForNative: (component: HostInstance) => Fiber | null,
getElementIDForHostInstance: GetElementIDForHostInstance,
getDisplayNameForElementID: GetDisplayNameForElementID,
getInstanceAndStyle(id: number): InstanceAndStyle,
getProfilingData(): ProfilingDataBackend,
Expand Down
7 changes: 4 additions & 3 deletions packages/react-devtools-shared/src/backend/views/Highlighter/Highlighter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type Agent from 'react-devtools-shared/src/backend/agent';
import type {HostInstance} from '../../types';

import {isReactNativeEnvironment} from 'react-devtools-shared/src/backend/utils';

Expand Down Expand Up @@ -37,7 +38,7 @@ export function hideOverlay(agent: Agent): void {
: hideOverlayWeb();
}

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

Expand All @@ -63,12 +64,12 @@ function showOverlayWeb(
}

export function showOverlay(
elements: Array<HTMLElement>,
elements: Array<HostInstance>,
componentName: string | null,
agent: Agent,
hideAfterTimeout: boolean,
): void {
return isReactNativeEnvironment()
? showOverlayNative(elements, agent)
: showOverlayWeb(elements, componentName, agent, hideAfterTimeout);
: showOverlayWeb((elements: any), componentName, agent, hideAfterTimeout);
}
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/views/Highlighter/Overlay.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export default class Overlay {
const rendererInterface =
this.agent.getBestMatchingRendererInterface(node);
if (rendererInterface) {
const id = rendererInterface.getElementIDForNative(node, true);
const id = rendererInterface.getElementIDForHostInstance(node, true);
if (id) {
const ownerName = rendererInterface.getDisplayNameForElementID(
id,
Expand Down
Loading
Loading