hanzoai/react
2
0
Fork 0
mirror of https://github.com/hanzoai/react.git synced 2026-09-24 17:02:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

[DevTools] Validate Store operation invariants (#37050)

Browse Source 
Builds on #37049 by validating Store operation invariants before
mutation. Missing nodes, invalid element types, inconsistent
parent-child relationships, and invalid reorder operations now emit and
throw explicit errors instead of silently continuing with corrupted
state.

Adds a canonical-render regression test for invalid child removal.
This commit is contained in:
Ruslan Lesiutin
2026-07-23 10:39:16 +01:00
committed by GitHub
parent 3025d58379
commit 0cbb5c38d5
2 changed files with 272 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-devtools-shared/src/__tests__/store-test.js
Vendored
+70
View File
@@ -129,6 +129,76 @@ describe('Store', () => {
expect(store).toMatchInlineSnapshot(`[root]`);
});
it('throws when a transition timeline is requested during initial paint', () => {
const errorListener = jest.fn();
store.addListener('error', errorListener);
expect(() =>
store.getSuspendableDocumentOrderSuspenseTransition(false, 1),
).toThrow(
'Cannot get a transition timeline during the initial paint. This is a bug in React DevTools.',
);
expect(errorListener).toHaveBeenCalledTimes(1);
store.removeListener('error', errorListener);
});
// @reactVersion >= 18.0
it('throws before removing a node that is not a child of its parent', () => {
function FirstChild() {
return null;
}
function SecondChild() {
return null;
}
function Parent({showFirstChild}) {
return (
<>
{showFirstChild && <FirstChild />}
<SecondChild />
</>
);
}
act(() => render(<Parent showFirstChild={true} />));
const parent = store.getElementAtIndex(0);
expect(parent.displayName).toBe('Parent');
const firstChildIndex = parent.children.findIndex(id => {
const child = store.getElementByID(id);
return child !== null && child.displayName === 'FirstChild';
});
expect(firstChildIndex).not.toBe(-1);
const firstChildID = parent.children[firstChildIndex];
// Corrupt only the frontend relationship. The removal operation below is
// still produced canonically by rendering React.
parent.children.splice(firstChildIndex, 1);
const errorListener = jest.fn();
store.addListener('error', errorListener);
let caughtError = null;
try {
act(() => render(<Parent showFirstChild={false} />));
} catch (error) {
caughtError = error;
} finally {
// The test Bridge invokes listeners synchronously, so discard the batch
// whose Store listener intentionally threw.
bridge._messageQueue.length = 0;
}
const expectedMessage =
`Cannot remove node "${firstChildID}" from parent "${parent.id}" ` +
`because it is not a child of the parent.`;
expect(caughtError).toMatchObject({message: expectedMessage});
expect(errorListener).toHaveBeenCalledWith(caughtError);
expect(store.containsElement(firstChildID)).toBe(true);
parent.children.splice(firstChildIndex, 0, firstChildID);
store.removeListener('error', errorListener);
});
// This test is not the same cause as what's reported on GitHub,
// but the resulting behavior (owner mounting after descendant) is the same.
// Thec ase below is admittedly contrived and relies on side effects.
packages/react-devtools-shared/src/devtools/store.js
Vendored
+202 -118
View File
@@ -28,7 +28,20 @@ import {
SUSPENSE_TREE_OPERATION_SUSPENDERS,
} from '../constants';
import {
ElementTypeClass,
ElementTypeContext,
ElementTypeFunction,
ElementTypeForwardRef,
ElementTypeHostComponent,
ElementTypeMemo,
ElementTypeOtherOrUnknown,
ElementTypeProfiler,
ElementTypeRoot,
ElementTypeSuspense,
ElementTypeSuspenseList,
ElementTypeTracingMarker,
ElementTypeVirtual,
ElementTypeViewTransition,
ElementTypeActivity,
ComponentFilterActivitySlice,
} from '../frontend/types';
@@ -140,6 +153,32 @@ function isNonZeroRect(rect: Rect) {
return rect.width > 0 || rect.height > 0 || rect.x > 0 || rect.y > 0;
}
function parseElementType(value: number): ElementType | null {
// Cast before switching so Flow checks exhaustiveness while the default rejects unknown bridge values.
const type = value as any as ElementType;
switch (type) {
case ElementTypeClass:
case ElementTypeContext:
case ElementTypeFunction:
case ElementTypeForwardRef:
case ElementTypeHostComponent:
case ElementTypeMemo:
case ElementTypeOtherOrUnknown:
case ElementTypeProfiler:
case ElementTypeRoot:
case ElementTypeSuspense:
case ElementTypeSuspenseList:
case ElementTypeTracingMarker:
case ElementTypeVirtual:
case ElementTypeViewTransition:
case ElementTypeActivity:
return type;
default:
(type) as empty;
return null;
}
}
/**
* The store is the single source of truth for updates from the backend.
* ContextProviders can subscribe to the Store for specific things they want to provide.
@@ -366,7 +405,7 @@ export default class Store extends EventEmitter<{
}
// This is only used in tests to avoid memory leaks.
assertMapSizeMatchesRootCount(map: Map<any, any>, mapName: string) {
assertMapSizeMatchesRootCount<K, V>(map: Map<K, V>, mapName: string) {
const expectedSize = this.roots.length;
if (map.size !== expectedSize) {
this._throwAndEmitError(
@@ -609,13 +648,11 @@ export default class Store extends EventEmitter<{
if (root === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Couldn't find root with id "${rootID}": no matching node was found in the Store.`,
),
);
return null;
}
if (root.children.length === 0) {
@@ -630,7 +667,9 @@ export default class Store extends EventEmitter<{
}
if (root === undefined) {
return null;
return this._throwAndEmitError(
Error(`Could not find an element at index "${index}" in the Store.`),
);
}
// Find the element in the tree using the weight of each node...
@@ -640,19 +679,18 @@ export default class Store extends EventEmitter<{
while (index !== currentWeight) {
const numChildren = currentElement.children.length;
let didFindChild = false;
for (let i = 0; i < numChildren; i++) {
const childID = currentElement.children[i];
const child = this._idToElement.get(childID);
if (child === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Couldn't child element with id "${childID}": no matching node was found in the Store.`,
),
);
return null;
}
const childWeight = child.isCollapsed ? 1 : child.weight;
@@ -660,14 +698,23 @@ export default class Store extends EventEmitter<{
if (index <= currentWeight + childWeight) {
currentWeight++;
currentElement = child;
didFindChild = true;
break;
} else {
currentWeight += childWeight;
}
}
if (!didFindChild) {
return this._throwAndEmitError(
Error(
`Could not find an element at index "${index}" because the Store tree weights are invalid.`,
),
);
}
}
return currentElement || null;
return currentElement;
}
getElementIDAtIndex(index: number): number | null {
@@ -685,6 +732,26 @@ export default class Store extends EventEmitter<{
return element;
}
_getElementByIDOrThrow(id: Element['id']): Element {
const element = this._idToElement.get(id);
if (element === undefined) {
return this._throwAndEmitError(
Error(
`Could not find element with id "${id}": no matching node was found in the Store.`,
),
);
}
return element;
}
_recalculateWeightAcrossRoots(): void {
let weightAcrossRoots = 0;
this._roots.forEach(rootID => {
weightAcrossRoots += this._getElementByIDOrThrow(rootID).weight;
});
this._weightAcrossRoots = weightAcrossRoots;
}
containsSuspense(id: SuspenseNode['id']): boolean {
return this._idToSuspense.has(id);
}
@@ -893,8 +960,15 @@ export default class Store extends EventEmitter<{
let depth = 0;
while (parentID > 0) {
if (parentID === ownerID || unsortedIDs.has(parentID)) {
// $FlowFixMe[unsafe-addition] addition with possible null/undefined value
depth = depthMap.get(parentID) + 1;
const parentDepth = depthMap.get(parentID);
if (parentDepth === undefined) {
return this._throwAndEmitError(
Error(
`Invalid owners list: owner depth for element "${parentID}" was not found.`,
),
);
}
depth = parentDepth + 1;
depthMap.set(id, depth);
break;
}
@@ -961,14 +1035,13 @@ export default class Store extends EventEmitter<{
let rootStep: null | SuspenseTimelineStep = null;
for (let i = 0; i < roots.length; i++) {
const rootID = roots[i];
const root = this.getElementByID(rootID);
if (root === null) {
continue;
}
this._getElementByIDOrThrow(rootID);
const rendererID = this._rootIDToRendererID.get(rootID);
if (rendererID === undefined) {
throw new Error(
'Failed to find renderer ID for root. This is a bug in React DevTools.',
return this._throwAndEmitError(
Error(
'Failed to find renderer ID for root. This is a bug in React DevTools.',
),
);
}
// TODO: This includes boundaries that can't be suspended due to no support from the renderer.
@@ -1059,9 +1132,12 @@ export default class Store extends EventEmitter<{
): Array<SuspenseTimelineStep> {
const target: Array<SuspenseTimelineStep> = [];
const focusedTransitionID = this._focusedTransition;
// $FlowFixMe[invalid-compare]
if (focusedTransitionID === null) {
return target;
if (focusedTransitionID === 0) {
return this._throwAndEmitError(
Error(
'Cannot get a transition timeline during the initial paint. This is a bug in React DevTools.',
),
);
}
target.push({
@@ -1155,14 +1231,18 @@ export default class Store extends EventEmitter<{
this._focusedTransition,
);
if (focusedTransitionRootID === null) {
throw new Error(
'Failed to find root ID for focused transition. This is a bug in React DevTools.',
return this._throwAndEmitError(
Error(
'Failed to find root ID for focused transition. This is a bug in React DevTools.',
),
);
}
const rendererID = this._rootIDToRendererID.get(focusedTransitionRootID);
if (rendererID === undefined) {
throw new Error(
'Failed to find renderer ID for focused transition root. This is a bug in React DevTools.',
return this._throwAndEmitError(
Error(
'Failed to find renderer ID for focused transition root. This is a bug in React DevTools.',
),
);
}
timeline = this.getSuspendableDocumentOrderSuspenseTransition(
@@ -1313,12 +1393,7 @@ export default class Store extends EventEmitter<{
// Only re-calculate weights and emit an "update" event if the store was mutated.
if (didMutate) {
let weightAcrossRoots = 0;
this._roots.forEach(rootID => {
const {weight} = this.getElementByID(rootID) as any as Element;
weightAcrossRoots += weight;
});
this._weightAcrossRoots = weightAcrossRoots;
this._recalculateWeightAcrossRoots();
// The Tree context's search reducer expects an explicit list of ids for nodes that were added or removed.
// In this case, we can pass it empty arrays since nodes in a collapsed tree are still there (just hidden).
@@ -1428,13 +1503,22 @@ export default class Store extends EventEmitter<{
switch (operation) {
case TREE_OPERATION_ADD: {
const id = operations[i + 1];
const type = operations[i + 2] as any as ElementType;
const rawType = operations[i + 2];
const type = parseElementType(rawType);
if (type === null) {
return this._throwAndEmitError(
Error(
`Cannot add node "${id}" because "${rawType}" is not a valid element type.`,
),
);
}
i += 3;
if (this._idToElement.has(id)) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot add node "${id}" because a node with that id is already in the Store.`,
),
@@ -1545,13 +1629,11 @@ export default class Store extends EventEmitter<{
const parentElement = this._idToElement.get(parentID);
if (parentElement === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot add child "${id}" to parent "${parentID}" because parent node was not found in the Store.`,
),
);
break;
}
parentElement.children.push(id);
@@ -1622,13 +1704,11 @@ export default class Store extends EventEmitter<{
if (element === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot remove node "${id}" because no matching node was found in the Store.`,
),
);
break;
}
i += 1;
@@ -1636,13 +1716,11 @@ export default class Store extends EventEmitter<{
const {children, ownerID, parentID, weight} = element;
if (children.length > 0) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(`Node "${id}" was removed before its children.`),
);
}
this._idToElement.delete(id);
let parentElement: ?Element = null;
if (parentID === 0) {
// $FlowFixMe[constant-condition]
@@ -1664,19 +1742,26 @@ export default class Store extends EventEmitter<{
parentElement = this._idToElement.get(parentID);
if (parentElement === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot remove node "${id}" from parent "${parentID}" because no matching node was found in the Store.`,
),
);
break;
}
const index = parentElement.children.indexOf(id);
if (index === -1) {
return this._throwAndEmitError(
Error(
`Cannot remove node "${id}" from parent "${parentID}" because it is not a child of the parent.`,
),
);
}
parentElement.children.splice(index, 1);
}
this._idToElement.delete(id);
this._adjustParentTreeWeight(parentElement, -weight);
removedElementIDs.set(id, parentID);
@@ -1704,37 +1789,42 @@ export default class Store extends EventEmitter<{
const element = this._idToElement.get(id);
if (element === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot reorder children for node "${id}" because no matching node was found in the Store.`,
),
);
break;
}
const children = element.children;
if (children.length !== numChildren) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Children cannot be added or removed during a reorder operation.`,
),
);
}
const reorderedChildIDs: Set<Element['id']> = new Set();
for (let j = 0; j < numChildren; j++) {
const childID = operations[i + j];
children[j] = childID;
if (__DEV__) {
// This check is more expensive so it's gated by __DEV__.
const childElement = this._idToElement.get(childID);
if (childElement == null || childElement.parentID !== id) {
console.error(
const childElement = this._idToElement.get(childID);
if (
childElement === undefined ||
childElement.parentID !== id ||
reorderedChildIDs.has(childID)
) {
return this._throwAndEmitError(
Error(
`Children cannot be added or removed during a reorder operation.`,
);
}
),
);
}
reorderedChildIDs.add(childID);
}
for (let j = 0; j < numChildren; j++) {
children[j] = operations[i + j];
}
i += numChildren;
@@ -1829,12 +1919,12 @@ export default class Store extends EventEmitter<{
const parentID = operations[i + 2];
const nameStringID = operations[i + 3];
const isSuspended = operations[i + 4] === 1;
const numRects = operations[i + 5] as any as number;
const numRects = operations[i + 5];
let name = stringTable[nameStringID];
if (this._idToSuspense.has(id)) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot add suspense node "${id}" because a suspense node with that id is already in the Store.`,
),
@@ -1887,13 +1977,11 @@ export default class Store extends EventEmitter<{
const parentSuspense = this._idToSuspense.get(parentID);
if (parentSuspense === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot add suspense child "${id}" to parent suspense "${parentID}" because parent suspense node was not found in the Store.`,
),
);
break;
}
parentSuspense.children.push(id);
@@ -1924,13 +2012,11 @@ export default class Store extends EventEmitter<{
if (suspense === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot remove suspense node "${id}" because no matching node was found in the Store.`,
),
);
break;
}
i += 1;
@@ -1938,11 +2024,33 @@ export default class Store extends EventEmitter<{
const {children, parentID, rects} = suspense;
if (children.length > 0) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(`Suspense node "${id}" was removed before its children.`),
);
}
let parentSuspense: SuspenseNode | null = null;
let parentIndex = -1;
if (parentID !== 0) {
parentSuspense = this._idToSuspense.get(parentID) || null;
if (parentSuspense === null) {
return this._throwAndEmitError(
Error(
`Cannot remove suspense node "${id}" from parent "${parentID}" because no matching node was found in the Store.`,
),
);
}
parentIndex = parentSuspense.children.indexOf(id);
if (parentIndex === -1) {
return this._throwAndEmitError(
Error(
`Cannot remove suspense node "${id}" from parent "${parentID}" because it is not a child of the parent.`,
),
);
}
}
if (rects !== null && parentID !== 0) {
// Delete all the existing rects from the R-tree
for (let j = 0; j < rects.length; j++) {
@@ -1953,8 +2061,7 @@ export default class Store extends EventEmitter<{
this._idToSuspense.delete(id);
removedSuspenseIDs.set(id, parentID);
let parentSuspense: ?SuspenseNode = null;
if (parentID === 0) {
if (parentSuspense === null) {
// $FlowFixMe[constant-condition]
if (__DEBUG__) {
debug('Suspense remove', `node ${id} root`);
@@ -1965,28 +2072,7 @@ export default class Store extends EventEmitter<{
debug('Suspense Remove', `node ${id} from parent ${parentID}`);
}
parentSuspense = this._idToSuspense.get(parentID);
if (parentSuspense === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
Error(
`Cannot remove suspense node "${id}" from parent "${parentID}" because no matching node was found in the Store.`,
),
);
break;
}
const index = parentSuspense.children.indexOf(id);
if (index === -1) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
Error(
`Cannot remove suspense node "${id}" from parent "${parentID}" because it is not a child of the parent.`,
),
);
}
parentSuspense.children.splice(index, 1);
parentSuspense.children.splice(parentIndex, 1);
}
}
@@ -2001,37 +2087,42 @@ export default class Store extends EventEmitter<{
const suspense = this._idToSuspense.get(id);
if (suspense === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot reorder children for suspense node "${id}" because no matching node was found in the Store.`,
),
);
break;
}
const children = suspense.children;
if (children.length !== numChildren) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Suspense children cannot be added or removed during a reorder operation.`,
),
);
}
const reorderedChildIDs: Set<SuspenseNode['id']> = new Set();
for (let j = 0; j < numChildren; j++) {
const childID = operations[i + j];
children[j] = childID;
if (__DEV__) {
// This check is more expensive so it's gated by __DEV__.
const childSuspense = this._idToSuspense.get(childID);
if (childSuspense == null || childSuspense.parentID !== id) {
console.error(
const childSuspense = this._idToSuspense.get(childID);
if (
childSuspense === undefined ||
childSuspense.parentID !== id ||
reorderedChildIDs.has(childID)
) {
return this._throwAndEmitError(
Error(
`Suspense children cannot be added or removed during a reorder operation.`,
);
}
),
);
}
reorderedChildIDs.add(childID);
}
for (let j = 0; j < numChildren; j++) {
children[j] = operations[i + j];
}
i += numChildren;
@@ -2047,20 +2138,18 @@ export default class Store extends EventEmitter<{
break;
}
case SUSPENSE_TREE_OPERATION_RESIZE: {
const id = operations[i + 1] as any as number;
const numRects = operations[i + 2] as any as number;
const id = operations[i + 1];
const numRects = operations[i + 2];
i += 3;
const suspense = this._idToSuspense.get(id);
if (suspense === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot set rects for suspense node "${id}" because no matching node was found in the Store.`,
),
);
break;
}
const prevRects = suspense.rects;
@@ -2142,13 +2231,11 @@ export default class Store extends EventEmitter<{
if (suspense === undefined) {
// We should never reach this. This is a bug in the backend renderer.
this._throwAndEmitError(
return this._throwAndEmitError(
Error(
`Cannot update suspenders of suspense node "${id}" because no matching node was found in the Store.`,
),
);
break;
}
// $FlowFixMe[constant-condition]
@@ -2178,7 +2265,7 @@ export default class Store extends EventEmitter<{
break;
}
default:
this._throwAndEmitError(
return this._throwAndEmitError(
new UnsupportedBridgeOperationError(
`Unsupported Bridge operation "${operation}"`,
),
@@ -2283,7 +2370,9 @@ export default class Store extends EventEmitter<{
// the Activities that are descendants of the next Activity slice.
const nextActivitySlice = this._idToElement.get(nextActivitySliceID);
if (nextActivitySlice === undefined) {
throw new Error('Next Activity slice not found in Store.');
return this._throwAndEmitError(
Error('Next Activity slice not found in Store.'),
);
}
for (let j = 0; j < nextActivitySlice.children.length; j++) {
@@ -2293,12 +2382,7 @@ export default class Store extends EventEmitter<{
}
if (didCollapse) {
let weightAcrossRoots = 0;
this._roots.forEach(rootID => {
const {weight} = this.getElementByID(rootID) as any as Element;
weightAcrossRoots += weight;
});
this._weightAcrossRoots = weightAcrossRoots;
this._recalculateWeightAcrossRoots();
}
}
@@ -2329,7 +2413,7 @@ export default class Store extends EventEmitter<{
let didMutate = false;
const element = this._idToElement.get(elementID);
if (element === undefined) {
throw new Error('Element not found in Store.');
return this._throwAndEmitError(Error('Element not found in Store.'));
}
if (element.type === ElementTypeActivity) {