From fceb4824c8c4536ec74d4abc507639367675dbff Mon Sep 17 00:00:00 2001 From: Tanner Linsley Date: Tue, 13 Oct 2026 00:03:09 -0600 Subject: [PATCH 1/3] fix: preserve hydration and suspended error recovery contracts --- .changeset/calm-hydration-recovery.md | 5 + .github/workflows/pr.yml | 4 + benchmarks/HYDRATION_SUSPENSE_RECOVERY.md | 60 +++++++++ package.json | 3 +- packages/redact/src/core/attributes.ts | 65 ++++++++++ .../redact/src/dom/features/class/full.ts | 29 ++++- .../redact/src/dom/features/hydration/full.ts | 83 ++++-------- .../redact/src/dom/features/hydration/stub.ts | 1 - .../redact/src/dom/features/suspense/full.ts | 2 + packages/redact/src/dom/reconcile.ts | 76 ++++++++--- scripts/hydration-recovery-browser.config.mjs | 7 ++ scripts/hydration-recovery.config.mjs | 5 + scripts/size-check.mjs | 40 +++--- tests/error-fallback-recovery.test.tsx | 111 ++++++++++++++++ tests/hydration-runtime-regressions.test.tsx | 118 ++++++++++++++++++ tests/hydration-scroll-ownership.test.tsx | 19 +++ tests/retained-store-departure.test.tsx | 50 ++++++++ 17 files changed, 580 insertions(+), 98 deletions(-) create mode 100644 .changeset/calm-hydration-recovery.md create mode 100644 benchmarks/HYDRATION_SUSPENSE_RECOVERY.md create mode 100644 scripts/hydration-recovery-browser.config.mjs create mode 100644 scripts/hydration-recovery.config.mjs create mode 100644 tests/error-fallback-recovery.test.tsx create mode 100644 tests/hydration-runtime-regressions.test.tsx create mode 100644 tests/hydration-scroll-ownership.test.tsx create mode 100644 tests/retained-store-departure.test.tsx diff --git a/.changeset/calm-hydration-recovery.md b/.changeset/calm-hydration-recovery.md new file mode 100644 index 0000000..398cfda --- /dev/null +++ b/.changeset/calm-hydration-recovery.md @@ -0,0 +1,5 @@ +--- +'@tanstack/redact': patch +--- + +Preserve SVG attribute casing, application scroll ownership, and matching head styles during hydration. Use inert hydration probes to avoid fetching lazy images or constructing custom elements. Recover document roots and suspended error fallbacks with the correct boundary ownership, reset behavior, lifecycle order, and retained external-store updates. diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index c98c49f..04446e7 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -25,3 +25,7 @@ jobs: uses: TanStack/config/.github/setup@e4b48f16568324f76f467aa4c2aac2f05db632c3 - name: Run Checks run: pnpm run test:pr + - name: Install regression test browser + run: pnpm exec playwright install --with-deps chromium + - name: Run browser recovery regressions + run: pnpm run test:recovery:browser diff --git a/benchmarks/HYDRATION_SUSPENSE_RECOVERY.md b/benchmarks/HYDRATION_SUSPENSE_RECOVERY.md new file mode 100644 index 0000000..534aeac --- /dev/null +++ b/benchmarks/HYDRATION_SUSPENSE_RECOVERY.md @@ -0,0 +1,60 @@ +# Hydration and Suspense recovery review + +The baseline is pristine main `becad73`, built separately with the same installed dependencies and Node 26.3.1. Measurements use `node scripts/size-check.mjs`, esbuild 0.28.0, and production entry bundles. These are compressed library sizes, not page load or runtime benchmarks. + +The SVG attribute table preserves the HTML parser's mixed-case names in both SSR and DOM updates. This must cover names beyond clipPathUnits because attributeName lowercases other names after its clip/fill/stroke conversion. + +Built output adds 574 gzip bytes to the combined client and 582 bytes with default Vite features. The budget change accepts this measured correctness cost explicitly. Only exceeded limits increase, to the measured value without extra margin. Other limits remain unchanged. + +| Tree | Entry | Baseline gzip bytes | Candidate gzip bytes | Change | +| --- | --- | ---: | ---: | ---: | +| src | redact | 2760 | 2760 | +0 | +| src | redact/jsx-runtime | 189 | 189 | +0 | +| src | redact/dom | 24931 | 25641 | +710 | +| src | redact/dom-client | 24564 | 25153 | +589 | +| src | redact/server | 8936 | 9318 | +382 | +| src | client total | 27761 | 28356 | +595 | +| src | client total (default Vite) | 22972 | 23558 | +586 | +| src | redact/dom-client (activity=stub) | 24032 | 24613 | +581 | +| src | redact/dom-client (fragmentRefs=stub) | 22405 | 22999 | +594 | +| src | redact/dom-client (viewTransitions=stub) | 19815 | 20414 | +599 | +| src | redact/dom-client (portal=stub) | 24537 | 25121 | +584 | +| src | redact/dom-client (context=stub) | 24282 | 24870 | +588 | +| src | redact/dom-client (suspense=stub) | 23072 | 23617 | +545 | +| src | redact/dom-client (memo=stub) | 24481 | 25073 | +592 | +| src | redact/dom-client (forwardRef=stub) | 24532 | 25118 | +586 | +| src | redact/dom-client (lazy=stub) | 24566 | 25154 | +588 | +| src | redact/dom-client (classComponents=stub) | 24097 | 24558 | +461 | +| src | redact/dom-client (hydration=stub) | 22101 | 22709 | +608 | +| src | redact/dom-client (nano) | 12212 | 12669 | +457 | +| dist | redact | 2759 | 2759 | +0 | +| dist | redact/jsx-runtime | 190 | 190 | +0 | +| dist | redact/dom | 25219 | 25949 | +730 | +| dist | redact/dom-client | 24879 | 25461 | +582 | +| dist | redact/server | 8936 | 9316 | +380 | +| dist | client total | 28060 | 28634 | +574 | +| dist | client total (default Vite) | 23232 | 23814 | +582 | +| dist | redact/dom-client (activity=stub) | 24329 | 24911 | +582 | +| dist | redact/dom-client (fragmentRefs=stub) | 22703 | 23273 | +570 | +| dist | redact/dom-client (viewTransitions=stub) | 20107 | 20674 | +567 | +| dist | redact/dom-client (portal=stub) | 24847 | 25428 | +581 | +| dist | redact/dom-client (context=stub) | 24593 | 25179 | +586 | +| dist | redact/dom-client (suspense=stub) | 23365 | 23907 | +542 | +| dist | redact/dom-client (memo=stub) | 24786 | 25371 | +585 | +| dist | redact/dom-client (forwardRef=stub) | 24835 | 25415 | +580 | +| dist | redact/dom-client (lazy=stub) | 24866 | 25449 | +583 | +| dist | redact/dom-client (classComponents=stub) | 24400 | 24857 | +457 | +| dist | redact/dom-client (hydration=stub) | 22368 | 22986 | +618 | +| dist | redact/dom-client (nano) | 12453 | 12905 | +452 | + +## Regression commands + +`pnpm test:ci` runs the full jsdom suite, types, built export verification, and source/dist size budgets. The four new test files are included by the existing test glob. The lazy-image resource assertion requires a real browser and is skipped in jsdom. + +`pnpm exec playwright install --with-deps chromium` then `pnpm test:recovery:browser` runs all 14 regression cases in Chromium. The PR workflow runs this command. For a locally installed Chrome, set `RECOVERY_CHROME=1`. + +`REACT_REFERENCE=1 pnpm test:recovery:browser` uses the existing `benchmarks/reference` React installation as a control. Three Redact-specific hydration policies (four cases) are excluded: inert custom-element validation and recoverable opaque-markup differences. React does not share those recovery policies. The remaining ten cases use the same public React APIs in both runtimes. + +`REDACT_TEST_SOURCE=/absolute/path/to/pristine/packages/redact/src pnpm exec vitest run --config scripts/hydration-recovery.config.mjs` runs the regressions against a separate pristine source checkout. The original jsdom run reproduced nine failures before the fixes. The full browser suite reproduced eleven failing cases and three passing controls on pristine main. The browser configuration supports the same source override. + +These tests cover library contracts. They do not claim complete React compatibility or whole-application parity. diff --git a/package.json b/package.json index ed4acfb..4b4aa37 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,8 @@ "changeset:version": "changeset version && pnpm install --no-frozen-lockfile", "changeset:publish": "changeset publish", "test:ci": "pnpm test && pnpm test:types && pnpm build && pnpm size:check", - "test:pr": "pnpm test:ci" + "test:pr": "pnpm test:ci", + "test:recovery:browser": "vitest run --config scripts/hydration-recovery-browser.config.mjs" }, "devDependencies": { "esbuild": "^0.28.0", diff --git a/packages/redact/src/core/attributes.ts b/packages/redact/src/core/attributes.ts index 66aaba3..77d2e8f 100644 --- a/packages/redact/src/core/attributes.ts +++ b/packages/redact/src/core/attributes.ts @@ -5,6 +5,7 @@ export const STRING_BOOLEAN_ATTRS = new Set([ ]) export function attributeName(name: string, isSvg = false): string { + if (isSvg && SVG_CASE_SENSITIVE_ATTRS.has(name)) return name if (isSvg && name !== 'viewBox' && SVG_KEBAB_PREFIX.test(name)) { return name.replace(/[A-Z]/g, (m) => '-' + m.toLowerCase()) } @@ -22,3 +23,67 @@ export function attributeName(name: string, isSvg = false): string { } const SVG_KEBAB_PREFIX = /^(?:clip|fill|stroke)/ + + +// SVG attributes adjusted to mixed case by the HTML parser must retain that +// spelling during hydration and DOM updates. clipPathUnits is not clip-path-units. +const SVG_CASE_SENSITIVE_ATTRS = new Set([ + 'attributeName', + 'attributeType', + 'baseFrequency', + 'baseProfile', + 'calcMode', + 'clipPathUnits', + 'diffuseConstant', + 'edgeMode', + 'filterUnits', + 'glyphRef', + 'gradientTransform', + 'gradientUnits', + 'kernelMatrix', + 'kernelUnitLength', + 'keyPoints', + 'keySplines', + 'keyTimes', + 'lengthAdjust', + 'limitingConeAngle', + 'markerHeight', + 'markerUnits', + 'markerWidth', + 'maskContentUnits', + 'maskUnits', + 'numOctaves', + 'pathLength', + 'patternContentUnits', + 'patternTransform', + 'patternUnits', + 'pointsAtX', + 'pointsAtY', + 'pointsAtZ', + 'preserveAlpha', + 'preserveAspectRatio', + 'primitiveUnits', + 'refX', + 'refY', + 'repeatCount', + 'repeatDur', + 'requiredExtensions', + 'requiredFeatures', + 'specularConstant', + 'specularExponent', + 'spreadMethod', + 'startOffset', + 'stdDeviation', + 'stitchTiles', + 'surfaceScale', + 'systemLanguage', + 'tableValues', + 'targetX', + 'targetY', + 'textLength', + 'viewBox', + 'viewTarget', + 'xChannelSelector', + 'yChannelSelector', + 'zoomAndPan', +]); diff --git a/packages/redact/src/dom/features/class/full.ts b/packages/redact/src/dom/features/class/full.ts index c60af04..d5a8f3d 100644 --- a/packages/redact/src/dom/features/class/full.ts +++ b/packages/redact/src/dom/features/class/full.ts @@ -7,6 +7,7 @@ import { childrenToArray, scheduleUpdate, scheduleLifecycle, + scheduleErrorLifecycle, isThenable, handleSuspended, handleErrorInRender, @@ -16,6 +17,7 @@ import { isActivityHidden, layoutIsDisconnected, RenderErrorCapture, + renderErrorFallback, handleCommitError, } from '../../reconcile' @@ -28,7 +30,21 @@ function renderClass(fiber: Fiber, domParent: Node, anchor: Node | null): void { const previous = fiber.ms?.c let didRender = true let captured = fiber.ms?.error instanceof RenderErrorCapture ? fiber.ms.error as RenderErrorCapture : undefined - if (captured) delete fiber.ms.error + if (captured) { + delete fiber.ms.error + // A reset or new route props abandons a pending recovery render. + // Re-evaluate the current children instead of restoring the old error. + if (captured.props !== props || captured.state !== instance?.state) { + // A memo between this boundary and the throwing child must not hide the + // retried read. Only mark a source that still belongs to this boundary. + let ancestor: Fiber | null = captured.source + while (ancestor && ancestor !== fiber) ancestor = ancestor.parent + if (ancestor === fiber && !captured.source.um && !captured.source.pd) { + for (let retry = captured.source; retry !== fiber; retry = retry.parent!) retry.dy = true + } + captured = undefined + } + } // Class contextType: read the current value of the subscribed context so // `this.context` reflects the nearest Provider. Evaluated every render. @@ -99,7 +115,8 @@ function renderClass(fiber: Fiber, domParent: Node, anchor: Node | null): void { if (canCatch && fiber.root) fiber.root.eb = true const checkpoint = canCatch ? checkpointCommit(fiber) : -1 try { - reconcileChildren(fiber, childrenToArray(rendered), domParent, anchor) + if (captured) renderErrorFallback(fiber, () => reconcileChildren(fiber, childrenToArray(rendered), domParent, anchor)) + else reconcileChildren(fiber, childrenToArray(rendered), domParent, anchor) } catch (error) { if (!(error instanceof RenderErrorCapture) || error.boundary !== fiber) throw error rewindCommit(checkpoint) @@ -114,11 +131,13 @@ function renderClass(fiber: Fiber, domParent: Node, anchor: Node | null): void { } } else rendered = null fiber.ms = { ...(fiber.ms ?? {}), r: rendered } - reconcileChildren(fiber, childrenToArray(rendered), domParent, anchor) + renderErrorFallback(fiber, () => reconcileChildren(fiber, childrenToArray(rendered), domParent, anchor)) } fiber.mp = props const state = instance.state - queueMutation(() => { fiber.ms.c = { props, state } }) + queueMutation(() => { + fiber.ms.c = { props, state } + }) if (isNew) attachRef(fiber, instance) else syncRefIfChanged(fiber, instance) @@ -146,7 +165,7 @@ function renderClass(fiber: Fiber, domParent: Node, anchor: Node | null): void { if (process.env.NODE_ENV !== 'production') failRefreshBoundary(fiber) const error = captured.error const info = { componentStack: captured.stack } - scheduleLifecycle(fiber, () => { + scheduleErrorLifecycle(fiber, () => { try { fiber.root?.ce?.(error, { ...info, errorBoundary: instance }) } catch (callbackError) { setTimeout(() => { throw callbackError }) } instance.componentDidCatch?.(error, info) diff --git a/packages/redact/src/dom/features/hydration/full.ts b/packages/redact/src/dom/features/hydration/full.ts index d55f9ea..b177330 100644 --- a/packages/redact/src/dom/features/hydration/full.ts +++ b/packages/redact/src/dom/features/hydration/full.ts @@ -7,7 +7,7 @@ import { type ReactNode, } from '../../../core' import { attributeName, STRING_BOOLEAN_ATTRS } from '../../../core/attributes' -import { createHostNode, setProp, syncTextareaValue } from '../../dom' +import { setProp, syncTextareaValue } from '../../dom' import { queueMutation, queueProp, queueText } from '../../commit' import { drainReplayQueue } from '../../event-replay' import { discardPendingWork, discardPendingEffects, findRoot, flushSyncWork, renderRoot, scheduleRootRender, recoverRootError, unmountFiber } from '../../reconcile' @@ -18,53 +18,6 @@ import { componentStack } from '../../error-info' // feature boundary — the plugin's stub swap strips drainReplayQueue too. export { drainReplayQueue } -/** - * Preserve the user's scroll position across hydration. If the user scrolled - * between SSR paint and hydrate (common in dev where JS takes seconds to - * load), libraries that wire scroll-restoration into a `useLayoutEffect` - * near the root (e.g. TanStack Router) will run during our synchronous - * hydrate and call `window.scrollTo(savedFromLastVisit)` — overwriting the - * user's fresh scroll. We install a short-lived wrapper around scrollTo that - * suppresses programmatic calls when a user-initiated scroll happened - * recently. Only runs in the hydration feature — the stub skips it. - */ -export function installHydrationScrollGuard(): void { - if (typeof window === 'undefined') return - const w = window as any - if (w._r) return - const time = Date.now - const guardStartedAt = w._r = time() - let lastUserScrollAt = 0 - let programmatic = false - w.addEventListener( - 'scroll', - () => { - if (!programmatic) { - lastUserScrollAt = time() - } - }, - { capture: true, passive: true }, - ) - const origScrollTo = w.scrollTo - w.scrollTo = function (...args: any[]) { - const now = time() - if ( - now - guardStartedAt < 3000 && - now - lastUserScrollAt < 1500 - ) { - return - } - programmatic = true - try { - return (origScrollTo as any).apply(w, args) - } finally { - queueMicrotask(() => { - programmatic = false - }) - } - } -} - /** * Hydration cursor: walks existing DOM children in document order so we can * adopt them during fiber tree construction. One cursor per host parent. @@ -168,8 +121,6 @@ export function hydrateRootImpl( const isDocument = (container as Node).nodeType === 9 const root = createFiberRoot(target, options) - installHydrationScrollGuard() - const normalizedInitialChildren = isDocument ? normalizeDocumentChildren(initialChildren) : initialChildren let hydrationError: unknown = null @@ -367,7 +318,7 @@ export function adoptHostDom(fiber: Fiber, parent: Fiber): boolean { const syncHydrationProps = !props.suppressHydrationWarning && canRecoverHydrationPropMismatch(candidate, tag) - validateHydrationProps(fiber, candidate as Element, props, tag, isSvg) + const recoverInnerHTML = validateHydrationProps(fiber, candidate as Element, props, tag, isSvg) // Form state can change between server paint and hydration. Initialize reset // defaults without treating the live value or selection as a mismatch. if (tag === 'input' || tag === 'textarea') queueHydrationFormState(candidate as Element, props, tag) @@ -375,6 +326,7 @@ export function adoptHostDom(fiber: Fiber, parent: Fiber): boolean { if ( (syncHydrationProps && k !== 'children' && k !== 'key' && k !== 'ref' && + (k !== 'dangerouslySetInnerHTML' || recoverInnerHTML) && (k !== 'nonce' || hydrationAttribute(candidate as Element, k) !== props[k]) && k !== 'suppressHydrationWarning' && k !== 'suppressContentEditableWarning' && !isHydrationFormProp(tag, k)) || @@ -707,15 +659,32 @@ function isHydrationFormProp(tag: string, key: string): boolean { ) } +// Validation must normalize attributes and markup without starting resource loads +// or upgrading custom elements in the live document. +const hydrationProbeDocuments = new WeakMap() +function createHydrationProbe(tag: string, isSvg: boolean, owner: Document): Element { + let inert = hydrationProbeDocuments.get(owner) + if (!inert) { + inert = owner.implementation.createHTMLDocument('') + hydrationProbeDocuments.set(owner, inert) + } + return isSvg + ? inert.createElementNS('http://www.w3.org/2000/svg', tag) + : inert.createElement(tag) +} + function validateHydrationProps( fiber: Fiber, el: Element, props: Record, tag: string, isSvg: boolean, -): void { - if (props.suppressHydrationWarning) return +): boolean { + if (props.suppressHydrationWarning) return false + // Head hosts recover prop mismatches in place. A matching opaque subtree + // must not be rewritten: resetting a stylesheet restarts font loading. + let recoverInnerHTML = false const recoverInPlace = canRecoverHydrationPropMismatch(el, tag) let expected: Element | undefined @@ -734,11 +703,12 @@ function validateHydrationProps( let html = value?.__html if (html == null) continue if (tag !== 'script' && tag !== 'style') { - const probe = createHostNode(tag, isSvg) + const probe = createHydrationProbe(tag, isSvg, el.ownerDocument) probe.innerHTML = html html = probe.innerHTML } if ((el as HTMLElement).innerHTML !== '' + html) { + recoverInnerHTML = true failHydration( fiber, process.env.NODE_ENV !== 'production' @@ -753,7 +723,7 @@ function validateHydrationProps( if (isHydrationFormProp(tag, k)) continue if (k === 'style') { - expected ??= createHostNode(tag, isSvg) + expected ??= createHydrationProbe(tag, isSvg, el.ownerDocument) setProp(expected, k, value, undefined, isSvg) if ( !hydrationStylesMatch( @@ -786,7 +756,7 @@ function validateHydrationProps( // These attributes use setAttribute string coercion, not DOM property normalization. expectedValue = '' + value } else { - expected ??= createHostNode(tag, isSvg) + expected ??= createHydrationProbe(tag, isSvg, el.ownerDocument) setProp(expected, k, value, undefined, isSvg) expectedValue = expected.getAttribute(attr) } @@ -804,6 +774,7 @@ function validateHydrationProps( ) } } + return recoverInnerHTML } function hydrationStylesMatch( diff --git a/packages/redact/src/dom/features/hydration/stub.ts b/packages/redact/src/dom/features/hydration/stub.ts index 17be62c..d28bcd6 100644 --- a/packages/redact/src/dom/features/hydration/stub.ts +++ b/packages/redact/src/dom/features/hydration/stub.ts @@ -62,7 +62,6 @@ export function findHostParent(fiber: Fiber): Fiber { return fiber } -export function installHydrationScrollGuard(): void {} export function drainReplayQueue(): void {} diff --git a/packages/redact/src/dom/features/suspense/full.ts b/packages/redact/src/dom/features/suspense/full.ts index 45fa668..4bdd962 100644 --- a/packages/redact/src/dom/features/suspense/full.ts +++ b/packages/redact/src/dom/features/suspense/full.ts @@ -24,6 +24,7 @@ import { enqueueEffect, flushFiberCommits, scheduleLifecycle, + reconnectErrorLifecycles, handleCommitError, } from '../../reconcile' import { cleanupEffect } from '../../dispatcher' @@ -253,6 +254,7 @@ function reconnectLayout(fiber: Fiber): void { if (effect.t === 1 && !effect.s) enqueueEffect(fiber, effect) } if (instance?.componentDidMount) scheduleLifecycle(fiber, () => instance.componentDidMount()) + reconnectErrorLifecycles(fiber) flushFiberCommits(fiber) } diff --git a/packages/redact/src/dom/reconcile.ts b/packages/redact/src/dom/reconcile.ts index 5eedc9b..1023b85 100644 --- a/packages/redact/src/dom/reconcile.ts +++ b/packages/redact/src/dom/reconcile.ts @@ -158,6 +158,13 @@ function flushRoot(root: FiberRoot): void { pending.sort((a, b) => fiberDepth(a) - fiberDepth(b)) for (const fiber of pending) { if (!fiber.dy || fiber.um || fiber.pd) continue + // An earlier ancestor in this batch may have retained this subtree after + // suspending. Work queued before that hide must also retry through the + // boundary, never render a retained child against its previous props. + if (fiber.su) { + scheduleUpdate(fiber) + continue + } const boundary = root.eb && findErrorBoundary(fiber) const checkpoint = boundary || (root.sp && findSuspenseBoundary(fiber)) ? checkpointCommit(fiber) : -1 try { @@ -165,6 +172,9 @@ function flushRoot(root: FiberRoot): void { } catch (error) { if (error instanceof RenderSuspenseCapture) { rewindCommit(checkpoint) + // Retry the suspended descendant through unchanged memo/context ancestors. + // Scheduling only Suspense can otherwise skip a captured error fallback. + for (let retry: Fiber | null = fiber; retry && retry !== error.boundary; retry = retry.parent) retry.dy = true error.boundary.ms.u = true scheduleUpdate(error.boundary) continue @@ -587,19 +597,8 @@ export function reconcileChildren( prevNewFiber = fiber } - // Pass 2: walk the sibling chain we just built and render each fiber - // forward with the correct per-child anchor. During hydration the cursor - // walks DOM forward and each renderFiber adopts the next existing node, - // so per-child anchors are moot — fall back to the parent's anchor. - const hydrating = !!currentRoot?.h - // New siblings have no DOM yet, so searching them for an anchor cannot help. - // An empty replacement has no new chain. The old children remain linked - // until cleanup below, and must not render again on their way out. - if (prevNewFiber) renderChildChain(parent, domParent, anchor, hydrating || existing.length === 0) - - if (!prevNewFiber) parent.child = null - else prevNewFiber.sibling = null - + // Queue deletions before placements, while keeping both deferred until commit. + // A document cannot accept its replacement HTML node before the old one leaves. // Head content is additive — server may inject metadata/stylesheets (Vite // dev styles, Sentry, analytics) that aren't in the React tree. Unmounting // them on every reconcile thrashes styles and causes flash of unstyled @@ -619,6 +618,19 @@ export function reconcileChildren( } } + // Pass 2: walk the sibling chain we just built and render each fiber + // forward with the correct per-child anchor. During hydration the cursor + // walks DOM forward and each renderFiber adopts the next existing node, + // so per-child anchors are moot — fall back to the parent's anchor. + const hydrating = !!currentRoot?.h + // New siblings have no DOM yet, so searching them for an anchor cannot help. + // An empty replacement has no new chain. Unclaimed old children are already + // scheduled for deletion and must not render again on their way out. + if (prevNewFiber) renderChildChain(parent, domParent, anchor, hydrating || existing.length === 0) + + if (!prevNewFiber) parent.child = null + else prevNewFiber.sibling = null + // During hydration, DOM is already in document order from the cursor-driven // adoption walk. Running placeChildrenInOrder here would reappend nodes to // the end of domParent when the true anchor (often an end marker comment) @@ -1234,8 +1246,12 @@ export function findSuspenseBoundary(fiber: Fiber): Fiber | null { export class RenderErrorCapture { stack: string - constructor(public boundary: Fiber, public error: unknown, source: Fiber = boundary) { + props: Fiber['pp'] + state: unknown + constructor(public boundary: Fiber, public error: unknown, public source: Fiber = boundary) { this.stack = componentStack(source) + this.props = boundary.pp + this.state = boundary.sn?.state } } @@ -1246,9 +1262,16 @@ function captureError(capture: RenderErrorCapture): void { scheduleUpdate(fiber) } +// Errors from an error fallback belong to an ancestor, never its owner. +const renderingErrorFallbacks = new Set() +export function renderErrorFallback(fiber: Fiber, render: () => void): void { + renderingErrorFallbacks.add(fiber) + try { render() } finally { renderingErrorFallbacks.delete(fiber) } +} + function findErrorBoundary(fiber: Fiber): Fiber | null { for (let parent = fiber.parent; parent; parent = parent.parent) { - if (parent.um || parent.pd) continue + if (parent.um || parent.pd || renderingErrorFallbacks.has(parent)) continue const instance = parent.sn if (instance?._fiber === parent && (parent.ms?.b?.getDerivedStateFromError || instance.componentDidCatch)) return parent } @@ -1672,6 +1695,29 @@ export function scheduleLifecycle(fiber: Fiber, fn: () => void): void { stageCommit(fiber, fn) } +// Suspense reconnects retained layouts after rendering the successful primary. +// Keep error notifications with their boundary until that child-first walk. +const pendingErrorLifecycles = new WeakMap void>>() + +export function scheduleErrorLifecycle(fiber: Fiber, fn: () => void): void { + if (isActivityHidden(fiber)) return + if (!layoutIsDisconnected(fiber)) { stageCommit(fiber, fn); return } + const previous = pendingErrorLifecycles.get(fiber) + pendingErrorLifecycles.set(fiber, previous ? [...previous, fn] : [fn]) + onCommitRollback(() => { + if (previous) pendingErrorLifecycles.set(fiber, previous) + else pendingErrorLifecycles.delete(fiber) + }) +} + +export function reconnectErrorLifecycles(fiber: Fiber): void { + const pending = pendingErrorLifecycles.get(fiber) + if (!pending) return + pendingErrorLifecycles.delete(fiber) + onCommitRollback(() => { pendingErrorLifecycles.set(fiber, pending) }) + for (const fn of pending) stageCommit(fiber, fn) +} + export function runEffects(root: FiberRoot): void { if (root.fr) refreshFragmentInstances(root) if (!pendingInsertionEffects.length && !pendingCommits.length && !pendingEffects.length) return diff --git a/scripts/hydration-recovery-browser.config.mjs b/scripts/hydration-recovery-browser.config.mjs new file mode 100644 index 0000000..94ba813 --- /dev/null +++ b/scripts/hydration-recovery-browser.config.mjs @@ -0,0 +1,7 @@ +import { resolve } from 'node:path' +import config from './browser-api-browser.config.mjs' +export default { + ...config, + cacheDir: resolve(`node_modules/.vite/hydration-recovery-${process.env.REACT_REFERENCE === '1' ? 'react' : 'redact'}`), + test: { ...config.test, browser: { ...config.test.browser, providerOptions: { launch: process.env.RECOVERY_CHROME === '1' ? { channel: 'chrome' } : {} } }, include: ['hydration-scroll-ownership.test.tsx', 'hydration-runtime-regressions.test.tsx', 'retained-store-departure.test.tsx', 'error-fallback-recovery.test.tsx'], testNamePattern: process.env.REACT_REFERENCE === '1' ? /^(?!does not instantiate custom elements|still reports genuinely different opaque markup|preserves opaque head recovery policy).*$/ : undefined }, +} diff --git a/scripts/hydration-recovery.config.mjs b/scripts/hydration-recovery.config.mjs new file mode 100644 index 0000000..1fcdc03 --- /dev/null +++ b/scripts/hydration-recovery.config.mjs @@ -0,0 +1,5 @@ +import config from './browser-api.config.mjs' +export default { + ...config, + test: { ...config.test, testNamePattern: process.env.REACT_REFERENCE === '1' ? /^(?!does not instantiate custom elements|still reports genuinely different opaque markup|preserves opaque head recovery policy).*$/ : undefined, include: ['tests/hydration-scroll-ownership.test.tsx', 'tests/hydration-runtime-regressions.test.tsx', 'tests/retained-store-departure.test.tsx', 'tests/error-fallback-recovery.test.tsx'] }, +} diff --git a/scripts/size-check.mjs b/scripts/size-check.mjs index 05b220a..d7e3ae3 100644 --- a/scripts/size-check.mjs +++ b/scripts/size-check.mjs @@ -7,26 +7,26 @@ import { measureSizes } from './measure-size.mjs' const BUDGETS = { 'redact': 2793, 'redact/jsx-runtime': 217, - // Recovery preserves provider ancestry. The reviewed built entry grows by - // 36 bytes, while the combined client shrinks by 64. Keep the approved budget. - // See benchmarks/HYDRATION_RECOVERY.md for the bound source/dist measurements. - 'redact/dom': 25282, - 'redact/dom-client': 25006, - 'redact/server': 8989, - 'client total': 28183, - 'client total (default Vite)': 23335, - 'redact/dom-client (activity=stub)': 24438, - 'redact/dom-client (fragmentRefs=stub)': 22814, - 'redact/dom-client (viewTransitions=stub)': 20159, - 'redact/dom-client (portal=stub)': 24972, - 'redact/dom-client (context=stub)': 24719, - 'redact/dom-client (suspense=stub)': 23479, - 'redact/dom-client (memo=stub)': 24912, - 'redact/dom-client (forwardRef=stub)': 24973, - 'redact/dom-client (lazy=stub)': 24983, - 'redact/dom-client (classComponents=stub)': 24512, - 'redact/dom-client (hydration=stub)': 22400, - 'redact/dom-client (nano)': 12491, + // Hydration and Suspense correctness costs are measured against pristine main. + // Only exceeded limits changed, with no added margin. + // See benchmarks/HYDRATION_SUSPENSE_RECOVERY.md for the complete comparison. + 'redact/dom': 25949, + 'redact/dom-client': 25461, + 'redact/server': 9318, + 'client total': 28634, + 'client total (default Vite)': 23814, + 'redact/dom-client (activity=stub)': 24911, + 'redact/dom-client (fragmentRefs=stub)': 23273, + 'redact/dom-client (viewTransitions=stub)': 20674, + 'redact/dom-client (portal=stub)': 25428, + 'redact/dom-client (context=stub)': 25179, + 'redact/dom-client (suspense=stub)': 23907, + 'redact/dom-client (memo=stub)': 25371, + 'redact/dom-client (forwardRef=stub)': 25415, + 'redact/dom-client (lazy=stub)': 25449, + 'redact/dom-client (classComponents=stub)': 24857, + 'redact/dom-client (hydration=stub)': 22986, + 'redact/dom-client (nano)': 12905, } let failed = false diff --git a/tests/error-fallback-recovery.test.tsx b/tests/error-fallback-recovery.test.tsx new file mode 100644 index 0000000..c3a1ce6 --- /dev/null +++ b/tests/error-fallback-recovery.test.tsx @@ -0,0 +1,111 @@ +import { afterEach, expect, it, vi } from 'vitest' +import * as React from 'react' +import { createRoot, hydrateRoot } from 'react-dom/client' +import { flushSync } from 'react-dom' + +const cleanups: Array<() => void> = [] +afterEach(() => { for (const cleanup of cleanups.splice(0)) cleanup() }) +const tick = () => new Promise(resolve => setTimeout(resolve, 30)) +type Props = { children?: React.ReactNode; fallback: (props: { error: unknown }) => React.ReactNode; onCatch?: (error: unknown) => void; reset?: number } +class Boundary extends React.Component { + state: { error: unknown; reset?: number } = { error: null } + static getDerivedStateFromProps(props: Props, state: { reset?: number }) { return state.reset !== props.reset ? { error: null, reset: props.reset } : null } + static getDerivedStateFromError(error: unknown) { return { error } } + componentDidCatch(error: unknown) { this.props.onCatch?.(error) } + render() { const Fallback = this.props.fallback; return this.state.error ? : this.props.children } +} +function setup() { + const container = document.createElement('div'); document.body.append(container) + const errors: unknown[] = [] + const root = createRoot(container, { onCaughtError() {}, onUncaughtError: error => errors.push(error) }) + cleanups.push(() => { flushSync(() => root.unmount()); container.remove() }) + return { container, root, errors, render: (element: React.ReactNode) => flushSync(() => root.render(element)) } +} + +it('propagates errors thrown by a fallback to its ancestor instead of recatching them', async () => { + const { container, render, errors } = setup() + const caught: unknown[] = [] + const failure = new Error('original failure') + function Throw(): React.ReactNode { throw failure } + function Rethrow({ error }: { error: unknown }): React.ReactNode { throw error } + function Recovered() { return Recovered } + render( caught.push(error)}> caught.push('wrong owner')}>) + await tick() + expect(container.textContent).toBe('Recovered') + expect(caught).toEqual([failure]) + expect(errors).toEqual([]) +}) + +for (const source of ['state', 'store'] as const) { + it(`commits a suspended error fallback and its catch lifecycle once after a ${source} update through memo`, async () => { + const { container, render, errors } = setup() + const events: string[] = [] + const failure = new Error('denied') + let denied = false, ready = false + let setDenied!: (value: boolean) => void + let release!: () => void + const waiting = new Promise(resolve => { release = () => { ready = true; resolve() } }) + const listeners = new Set<() => void>() + const subscribe = (callback: () => void) => { listeners.add(callback); return () => { listeners.delete(callback) } } + const snapshot = () => denied + function Content() { + const [state, set] = React.useState(false); setDenied = set + const external = React.useSyncExternalStore(subscribe, snapshot) + if (source === 'state' ? state : external) throw failure + return
Private
+ } + function Fallback() { + if (!ready) throw waiting + React.useLayoutEffect(() => { events.push('fallback-layout') }, []) + return Denied + } + const Stable = React.memo(() => ) + const Wrapped = React.memo(() => events.push('caught')}>) + render(Waiting}>) + denied = true + if (source === 'state') setDenied(true) + else for (const notify of listeners) notify() + await tick() + expect(container.textContent).toContain('Waiting') + release() + await tick() + await vi.waitFor(() => expect(container.textContent).toBe('Denied')) + expect(events).toEqual(['fallback-layout', 'caught']) + expect(errors).toEqual([]) + }) +} + +it('abandons a suspended captured error when the boundary receives reset props', async () => { + const { container, render, errors } = setup() + let fail = false + let update!: () => void + const caught: unknown[] = [] + const pending = new Promise(() => {}) + function Content() { const [, set] = React.useState(0); update = () => set(value => value + 1); if (fail) throw new Error('old error'); return Healthy } + function Pending(): React.ReactNode { throw pending } + const Stable = React.memo(Content) + const tree = (reset: number) => Waiting}> caught.push(error)}> + render(tree(0)); fail = true; update(); await tick() + expect(container.textContent).toContain('Waiting') + fail = false; render(tree(1)); await tick() + expect(container.textContent).toBe('Healthy') + expect(caught).toEqual([]) + expect(errors).toEqual([]) +}) + +it('replaces document HTML only after deleting the previous root, while preserving the doctype', async () => { + const doc = document.implementation.createHTMLDocument('') + const errors: unknown[] = [] + let fail!: () => void + function Content() { const [failed, set] = React.useState(false); fail = () => set(true); if (failed) throw new Error('document failed'); return
Healthy
} + function Fallback() { return
Recovered
} + doc.documentElement.innerHTML = '
Healthy
' + const root = hydrateRoot(doc, , { onCaughtError() {}, onUncaughtError: error => errors.push(error) }) + cleanups.push(() => flushSync(() => root.unmount())) + await tick() + fail(); await tick() + await vi.waitFor(() => expect(doc.body.textContent).toBe('Recovered')) + expect(doc.querySelectorAll('html')).toHaveLength(1) + expect(doc.doctype).not.toBeNull() + expect(errors).toEqual([]) +}) diff --git a/tests/hydration-runtime-regressions.test.tsx b/tests/hydration-runtime-regressions.test.tsx new file mode 100644 index 0000000..003dc59 --- /dev/null +++ b/tests/hydration-runtime-regressions.test.tsx @@ -0,0 +1,118 @@ +import { afterEach, expect, it, vi } from 'vitest' +import * as React from 'react' +import { hydrateRoot, type Root } from 'react-dom/client' +import { flushSync } from 'react-dom' +import { renderToString } from 'react-dom/server' + +const roots: Root[] = [] +const containers: Element[] = [] +afterEach(() => { + for (const root of roots.splice(0)) flushSync(() => root.unmount()) + for (const container of containers.splice(0)) container.remove() + vi.restoreAllMocks() +}) +const tick = () => new Promise(resolve => setTimeout(resolve, 30)) +function container(markup: string) { + const element = document.createElement('div') + element.innerHTML = markup + document.body.append(element) + containers.push(element) + return element +} + +it('preserves HTML-parser SVG casing during hydration and later updates', async () => { + const tree = (units: string) => + const element = container(renderToString(tree('userSpaceOnUse'))) + const original = element.querySelector('clipPath')! + const errors: unknown[] = [] + const root = hydrateRoot(element, tree('userSpaceOnUse'), { onRecoverableError: error => errors.push(error) }) + roots.push(root) + await tick() + expect(element.querySelector('clipPath')).toBe(original) + expect(errors).toEqual([]) + expect(element.querySelector('svg')!.getAttribute('viewBox')).toBe('0 0 10 10') + expect(element.querySelector('linearGradient')!.getAttribute('gradientUnits')).toBe('userSpaceOnUse') + expect(element.querySelector('linearGradient')!.getAttribute('gradientTransform')).toBe('rotate(45)') + expect(original.getAttribute('clipPathUnits')).toBe('userSpaceOnUse') + expect(original.hasAttribute('clip-path-units')).toBe(false) + flushSync(() => root.render(tree('objectBoundingBox'))) + expect(original.getAttribute('clipPathUnits')).toBe('objectBoundingBox') + expect(element.querySelector('linearGradient')!.getAttribute('gradientUnits')).toBe('objectBoundingBox') + expect(element.querySelector('filter')!.getAttribute('filterUnits')).toBe('objectBoundingBox') + expect(element.querySelector('feGaussianBlur')!.getAttribute('stdDeviation')).toBe('2') + expect(element.querySelector('svg > path')!.getAttribute('stroke-width')).toBe('2') +}) + +it('does not instantiate custom elements while normalizing opaque hydration markup', async () => { + let constructions = 0 + const name = 'hydration-probe-' + Math.random().toString(36).slice(2) + customElements.define(name, class extends HTMLElement { constructor() { super(); constructions++ } }) + const markup = `<${name}>` + const tree =
+ const element = container(renderToString(tree)) + const original = element.firstChild + expect(constructions).toBe(1) + const errors: unknown[] = [] + roots.push(hydrateRoot(element, tree, { onRecoverableError: error => errors.push(error) })) + await tick() + expect(constructions).toBe(1) + expect(element.firstChild).toBe(original) + expect(errors).toEqual([]) +}) + +it('keeps matching opaque stylesheet nodes intact during head hydration', async () => { + const tree =

Ready

' + const style = doc.querySelector('style')! + const original = style.firstChild + const records: MutationRecord[] = [] + const observer = new MutationObserver(next => records.push(...next)) + observer.observe(style, { childList: true, characterData: true, subtree: true }) + const errors: unknown[] = [] + const root = hydrateRoot(doc, tree, { onRecoverableError: error => errors.push(error) }) + roots.push(root) + await tick() + records.push(...observer.takeRecords()) + observer.disconnect() + expect(style.firstChild).toBe(original) + expect(records).toEqual([]) + expect(errors).toEqual([]) +}) + + +const nativeBrowser = typeof navigator !== 'undefined' && !navigator.userAgent.includes('jsdom') +it.skipIf(!nativeBrowser)('does not fetch offscreen lazy images while probing hydration attributes', async () => { + const url = new URL('/__hydration_probe_image_' + Math.random().toString(36).slice(2) + '.png', location.href).href + const tree = Offscreen + const element = container(renderToString(tree)) + await tick() + const errors: unknown[] = [] + roots.push(hydrateRoot(element, tree, { onRecoverableError: error => errors.push(error) })) + await new Promise(resolve => setTimeout(resolve, 200)) + expect(performance.getEntriesByName(url)).toHaveLength(0) + expect(errors).toEqual([]) +}) + +it('still reports genuinely different opaque markup after inert normalization', async () => { + const element = container('
Server
') + const errors: unknown[] = [] + roots.push(hydrateRoot(element,
Client' }} />, { onRecoverableError: error => errors.push(error) })) + await tick() + expect(errors.length).toBeGreaterThan(0) + expect(element.innerHTML).toContain('Client') +}) + +for (const suppressed of [false, true]) { + it(`preserves opaque head recovery policy with suppressed=${suppressed}`, async () => { + const doc = document.implementation.createHTMLDocument('') + doc.documentElement.innerHTML = '

Ready

' + const original = doc.querySelector('style')! + const errors: unknown[] = [] + roots.push(hydrateRoot(doc,