From e04c66e02e5c8ebe8bdfce771e4eda77d7e7c2cf Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Sat, 22 Aug 2026 02:21:38 -0700 Subject: [PATCH 1/2] Keep the perf baseline from blocking dependency PRs (#628) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paired perf bench checked out base `packages/react-grab/src` while leaving the PR's installed dependencies in place, so a dependency bump left base src importing an export the new version dropped. All 15 shards died at fixture startup and the aggregate gate failed a PR whose own code was fine — bippy 0.7.2 removing `getNearestHostFibers` did this to #626 and #621. The swap now takes the base manifests and lockfile too whenever the PR touches one, which both makes the baseline buildable and makes it the comparison a dependency bump actually wants. Manifests the PR adds are dropped first so the base lockfile passes its frozen check, and a base install that still fails falls back to skipping the baseline. The baseline run is also no longer fatal. It is a measurement, not a gate: the base ref has its own CI, and diff-perf-runs.mjs already reports a missing comparison instead of failing. --- .github/workflows/test-perf.yml | 45 ++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-perf.yml b/.github/workflows/test-perf.yml index b359d5a40..efb2f8773 100644 --- a/.github/workflows/test-perf.yml +++ b/.github/workflows/test-perf.yml @@ -82,16 +82,44 @@ jobs: run: | BASE_SHA="${{ github.event.pull_request.base.sha }}" git fetch origin "$BASE_SHA" --depth=1 - if git cat-file -e "$BASE_SHA:packages/react-grab/src" 2>/dev/null; then - git checkout "$BASE_SHA" -- packages/react-grab/src - echo "swapped=true" >> "$GITHUB_OUTPUT" - echo "Swapped react-grab/src to base revision $BASE_SHA" - else + if ! git cat-file -e "$BASE_SHA:packages/react-grab/src" 2>/dev/null; then echo "Base ref lacks packages/react-grab/src; skipping baseline run." echo "swapped=false" >> "$GITHUB_OUTPUT" + exit 0 fi + git checkout "$BASE_SHA" -- packages/react-grab/src + echo "Swapped react-grab/src to base revision $BASE_SHA" + # Base src builds against whatever this job installed, so a dependency + # change leaves it importing exports the new version dropped and the + # fixture's dev server never starts. Swapping the manifests too keeps + # the baseline a measurement of the base ref instead of a mix that + # cannot build, and it is the comparison a dependency bump wants. + if git diff --quiet "$BASE_SHA" HEAD -- '*package.json' pnpm-lock.yaml; then + echo "swapped=true" >> "$GITHUB_OUTPUT" + echo "deps_swapped=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + # Manifests the PR adds have no base revision to restore, and leaving + # them would make the base lockfile fail its frozen check. + git diff --name-only --diff-filter=A "$BASE_SHA" HEAD -- '*package.json' | xargs -r rm -f + git checkout "$BASE_SHA" -- '*package.json' pnpm-lock.yaml + if pnpm install; then + echo "swapped=true" >> "$GITHUB_OUTPUT" + echo "deps_swapped=true" >> "$GITHUB_OUTPUT" + echo "Swapped dependencies to base revision $BASE_SHA" + exit 0 + fi + echo "Base dependencies failed to install; skipping baseline run." + git checkout HEAD -- packages/react-grab/src '*package.json' pnpm-lock.yaml + pnpm install + echo "swapped=false" >> "$GITHUB_OUTPUT" + + # The baseline is a measurement, not a gate: the base ref is already tested + # by its own CI, so a baseline that cannot build or run must not block this + # PR. diff-perf-runs.mjs reports the missing comparison instead. - name: Run paired shard against base ref src + continue-on-error: true if: github.event_name == 'pull_request' && steps.swap-base.outputs.swapped == 'true' env: PERF_LABEL: baseline @@ -108,7 +136,12 @@ jobs: - name: Restore react-grab src to HEAD if: github.event_name == 'pull_request' && steps.swap-base.outputs.swapped == 'true' - run: git checkout HEAD -- packages/react-grab/src + run: | + git checkout HEAD -- packages/react-grab/src + if [ "${{ steps.swap-base.outputs.deps_swapped }}" = "true" ]; then + git checkout HEAD -- '*package.json' pnpm-lock.yaml + pnpm install + fi - name: Run paired shard against HEAD src env: From 38d49956d56cb068061d5e39f2b5561665a43ca1 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Sat, 22 Aug 2026 02:31:24 -0700 Subject: [PATCH 2/2] chore: bump bippy to 0.7.2 (#626) * chore: bump bippy to 0.7.2 Co-authored-by: aiden * fix: migrate removed bippy fiber traversal Co-authored-by: aiden * fix: align with bippy 0.7 fiber types Co-authored-by: aiden --------- Co-authored-by: Cursor Agent --- packages/react-grab/package.json | 2 +- packages/react-grab/src/core/context.ts | 5 +- .../react-grab/src/core/element-anchors.ts | 46 +++++++++++++------ .../react-grab/src/utils/freeze-updates.ts | 33 ++++++------- pnpm-lock.yaml | 22 +++++++-- 5 files changed, 66 insertions(+), 42 deletions(-) diff --git a/packages/react-grab/package.json b/packages/react-grab/package.json index 2829bae3c..d7bff4e57 100644 --- a/packages/react-grab/package.json +++ b/packages/react-grab/package.json @@ -109,7 +109,7 @@ }, "dependencies": { "@react-grab/cli": "workspace:*", - "bippy": "^0.6.1" + "bippy": "^0.7.2" }, "devDependencies": { "@babel/core": "^7.29.0", diff --git a/packages/react-grab/src/core/context.ts b/packages/react-grab/src/core/context.ts index 27f3895b3..ab6150b3b 100644 --- a/packages/react-grab/src/core/context.ts +++ b/packages/react-grab/src/core/context.ts @@ -3,6 +3,7 @@ import { isInstrumentationActive, getDisplayName, getLatestFiber, + isFiber, isCompositeFiber, traverseFiber, type Fiber, @@ -259,10 +260,10 @@ export interface ResolvedSource extends SourceLocation { const pickNearestSourceFrame = (frames: StackFrame[]): StackFrame | null => frames[0] ?? null; const getSourceComponentName = ( - fiber: Fiber | undefined, + fiber: Fiber["_debugOwner"], isNextProject: boolean, ): string | null => { - if (!fiber || !isCompositeFiber(fiber)) return null; + if (!isFiber(fiber) || !isCompositeFiber(fiber)) return null; return toSourceComponentName(getDisplayName(fiber.type), isNextProject); }; diff --git a/packages/react-grab/src/core/element-anchors.ts b/packages/react-grab/src/core/element-anchors.ts index 481c7253e..6aa140360 100644 --- a/packages/react-grab/src/core/element-anchors.ts +++ b/packages/react-grab/src/core/element-anchors.ts @@ -1,10 +1,4 @@ -import { - getFiberFromHostInstance, - getLatestFiber, - getNearestHostFibers, - isHostFiber, - type Fiber, -} from "bippy"; +import { getFiberFromHostInstance, getLatestFiber, isHostFiber, type Fiber } from "bippy"; import { indexInParent } from "../utils/index-in-parent.js"; import { isShadowRoot } from "../utils/is-shadow-root.js"; import { isElementNode } from "../utils/is-element-node.js"; @@ -55,15 +49,37 @@ const resolveLiveAnchor = (anchor: ElementAnchor): Element | null => { return candidate && candidate.tagName === anchorTagName ? candidate : null; } - for (const hostFiber of getNearestHostFibers(latestParentFiber)) { - const node = hostFiber.stateNode; - // The tag match keeps recovery from latching onto an unrelated host when the - // original element type is gone. Same-tag siblings under a shared composite - // ancestor can't be told apart and resolve to the first match, which still - // leaves the selection on a valid node instead of dropping it. - if (isElementNode(node) && node.isConnected && node.tagName === anchorTagName) { - return node; + return findNearestHostElementWithTag(latestParentFiber, anchorTagName); +}; + +// Walks the nearest host fibers below `parentFiber` (stopping at each host +// boundary, like bippy's removed getNearestHostFibers) for a connected element +// with the anchor's tag. The tag match keeps recovery from latching onto an +// unrelated host when the original element type is gone. Same-tag siblings +// under a shared composite ancestor can't be told apart and resolve to the +// first match, which still leaves the selection on a valid node instead of +// dropping it. +const findNearestHostElementWithTag = ( + parentFiber: Fiber, + anchorTagName: string, +): Element | null => { + let fiber: Fiber | null = parentFiber.child; + while (fiber) { + if (isHostFiber(fiber)) { + const node = fiber.stateNode; + if (isElementNode(node) && node.isConnected && node.tagName === anchorTagName) { + return node; + } + } else if (fiber.child) { + fiber = fiber.child; + continue; + } + while (fiber !== parentFiber && !fiber.sibling) { + fiber = fiber.return; + if (!fiber) return null; } + if (fiber === parentFiber) return null; + fiber = fiber.sibling; } return null; }; diff --git a/packages/react-grab/src/utils/freeze-updates.ts b/packages/react-grab/src/utils/freeze-updates.ts index e69d9e4e5..d346c9553 100644 --- a/packages/react-grab/src/utils/freeze-updates.ts +++ b/packages/react-grab/src/utils/freeze-updates.ts @@ -19,10 +19,6 @@ import { RecoverableError } from "../errors.js"; import { reportRecoverableError } from "./report-recoverable-error.js"; import { IS_DEMO } from "./runtime-mode.js"; -interface FiberRootLike extends FiberRoot { - current: Fiber | null; -} - interface PendingUpdate { next: PendingUpdate | null; action: unknown; @@ -91,9 +87,8 @@ const pendingStateUpdates: Array<() => void> = []; const pausedQueueStates = new WeakMap(); const pausedContextStates = new WeakMap(); const renderersWithPatchedDispatcher = new WeakSet(); -const typedFiberRoots = _fiberRoots as Set; -const pausedFiberRoots = new Set(); -const fiberRootRenderers = new WeakMap(); +const pausedFiberRoots = new Set(); +const fiberRootRenderers = new WeakMap(); instrument({ name: "react-grab-freeze-updates", @@ -112,17 +107,17 @@ const isDomRenderer = (renderer: ReactRenderer): boolean => { } }; -const getFiberRoot = (fiber: Fiber): FiberRootLike | null => { +const getFiberRoot = (fiber: Fiber): FiberRoot | null => { let current: Fiber | null = fiber; while (current.return) { current = current.return; } - return (current.stateNode ?? null) as FiberRootLike | null; + return (current.stateNode ?? null) as FiberRoot | null; }; -const findHostInstance = (fiberRoot: FiberRootLike): object | null => { +const findHostInstance = (fiberRoot: FiberRoot): object | null => { const root = fiberRoot.current; - let fiber = root; + let fiber: Fiber | null = root; while (fiber) { const stateNode = fiber.stateNode; if ( @@ -146,7 +141,7 @@ const findHostInstance = (fiberRoot: FiberRootLike): object | null => { return null; }; -const resolveFiberRootRenderer = (fiberRoot: FiberRootLike): ReactRenderer | null => { +const resolveFiberRootRenderer = (fiberRoot: FiberRoot): ReactRenderer | null => { const renderer = fiberRootRenderers.get(fiberRoot); if (renderer) return isDomRenderer(renderer) ? renderer : null; @@ -172,7 +167,7 @@ const resolveFiberRootRenderer = (fiberRoot: FiberRootLike): ReactRenderer | nul return null; }; -const isDomFiberRoot = (fiberRoot: FiberRootLike): boolean => { +const isDomFiberRoot = (fiberRoot: FiberRoot): boolean => { const stateNode = fiberRoot.current?.stateNode; if (!stateNode || typeof stateNode !== "object") return false; const containerInfo = Reflect.get(stateNode, "containerInfo"); @@ -185,16 +180,16 @@ const isDomFiberRoot = (fiberRoot: FiberRootLike): boolean => { // Collects React fiber roots, preferring bippy's tracked set but falling back // to a DOM walk when the app mounted before bippy instrumented the renderers. -const collectFiberRoots = (): Set => { - if (typedFiberRoots.size > 0) { - const domFiberRoots = new Set(); - for (const fiberRoot of typedFiberRoots) { +const collectFiberRoots = (): Set => { + if (_fiberRoots.size > 0) { + const domFiberRoots = new Set(); + for (const fiberRoot of _fiberRoots) { if (isDomFiberRoot(fiberRoot)) domFiberRoots.add(fiberRoot); } return domFiberRoots; } - const collectedRoots = new Set(); + const collectedRoots = new Set(); const traverseDOM = (element: Element): void => { const fiber = getFiberFromHostInstance(element); @@ -601,7 +596,7 @@ const installDispatcherPatching = (renderer: ReactRenderer): void => { }; const scheduleReactUpdate = ( - fiberRoots: Set, + fiberRoots: Set, scheduledFreezeSessionId: number, ): void => { queueMicrotask(() => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cf9cfc01f..00827108d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -402,8 +402,8 @@ importers: specifier: workspace:* version: link:../cli bippy: - specifier: ^0.6.1 - version: 0.6.1(react@19.2.6) + specifier: ^0.7.2 + version: 0.7.2(@types/react@19.2.14)(react@19.2.6) react: specifier: 19.2.6 version: 19.2.6 @@ -3695,6 +3695,11 @@ packages: peerDependencies: '@types/react': '*' + '@types/react-reconciler@0.33.0': + resolution: {integrity: sha512-HZOXsKT0tGI9LlUw2LuedXsVeB88wFa536vVL0M6vE8zN63nI+sSr1ByxmPToP5K5bukaVscyeCJcF9guVNJ1g==} + peerDependencies: + '@types/react': '*' + '@types/react@19.2.14': resolution: {integrity: sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==} @@ -4357,8 +4362,8 @@ packages: bignumber.js@9.3.1: resolution: {integrity: sha512-Ko0uX15oIUS7wJ3Rb30Fs6SkVbLmPBAKdlm7q9+ak9bbIeFf0MwuBsQV6z7+X768/cHsfg+WlysDWJcmthjsjQ==} - bippy@0.6.1: - resolution: {integrity: sha512-ky4m94Y/KfsddjGkKTsV4uFjZqkJjpOjQ2t5gKPdX6XH1MNxMNX5FrVefsxV4lpjemEmEdwe0e0YbzAMNs3oUQ==} + bippy@0.7.2: + resolution: {integrity: sha512-h8NFpBuVix0iuih7o5eclUP7W3DJyRUQz4OojD/S1DhyBGk6TeDrICvsAc0VMdl8KrMCEAyT4JbkX9ON3cHejg==} peerDependencies: react: 19.2.6 @@ -11505,6 +11510,10 @@ snapshots: dependencies: '@types/react': 19.2.14 + '@types/react-reconciler@0.33.0(@types/react@19.2.14)': + dependencies: + '@types/react': 19.2.14 + '@types/react@19.2.14': dependencies: csstype: 3.2.3 @@ -12011,9 +12020,12 @@ snapshots: bignumber.js@9.3.1: {} - bippy@0.6.1(react@19.2.6): + bippy@0.7.2(@types/react@19.2.14)(react@19.2.6): dependencies: + '@types/react-reconciler': 0.33.0(@types/react@19.2.14) react: 19.2.6 + transitivePeerDependencies: + - '@types/react' bl@4.1.0: dependencies: