feat: deferred hydration#7362
Open
schiller-manuel wants to merge 5 commits intomainfrom
Open
Conversation
github-actions
Bot
added
documentation
Contributor
📝 WalkthroughI can’t generate the full hidden review stack artifact with every rangeId here — the PR contains too many rangeIds to reliably list in this response. If you want, I can:
Which would you prefer? ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
|
View your CI Pipeline Execution ↗ for commit 8567b14
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview8 package(s) bumped directly, 16 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
Contributor
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Sheraff
reviewed
May 7, 2026
| return () => globalThis.clearTimeout(timeoutId) | ||
| } | ||
|
|
||
| /* @__NO_SIDE_EFFECTS__ */ |
Contributor
There was a problem hiding this comment.
Are these annotations necessary if we already have "sideEffects": false, in the package.json? (honest question, i'm not sure one way or the other)
Contributor
Author
There was a problem hiding this comment.
they seemed to have helped with bundlesize at some time. i will revisit
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenRenamed.tsx (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame absolute local-machine path issue as
hydrateWhenMultiple.tsx.Line 1 contains
/Users/caligano/source/tanstack/router/.... See the comment onhydrateWhenMultiple.tsx— the fix needs to land in the snapshot generator's path-normalization step; both snapshots must then be regenerated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenRenamed.tsx` around lines 1 - 2, The snapshot contains an absolute local path in the dynamic import call (see _lazyHydratedComponent and the generated import string containing hydrateWhenRenamed_ad6838514c), so update the snapshot generator's path-normalization step to replace machine-specific absolute paths with a normalized relative or repo-root-based path (preserving the filename and query params like ?tss-hydrate=...&tss-hydrate-index=...), then regenerate the snapshots (including hydrateWhenRenamed.tsx and hydrateWhenMultiple.tsx) so the _Hydrate_/_Hydrate_0_preload lines use the normalized path instead of /Users/caligano/...
🟡 Minor comments (6)
packages/start-server-core/tests/transformAssets.test.ts-4-4 (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused import
buildManifestWithClientEntry.This import is not used anywhere in the test file and should be removed to maintain code cleanliness and comply with strict TypeScript configuration requirements.
Remove unused import
import { adaptTransformAssetUrlsToTransformAssets, - buildManifestWithClientEntry, resolveTransformAssetsConfig, transformManifestAssets, } from '../src/transformAssetUrls'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-server-core/tests/transformAssets.test.ts` at line 4, Remove the unused import buildManifestWithClientEntry from the imports at the top of tests/transformAssets.test.ts; locate the import list that currently includes buildManifestWithClientEntry and delete that symbol so the file only imports what it actually uses, then run the TypeScript/linters to confirm no unused-import errors remain.e2e/react-start/rsc-deferred-hydration/src/routes/index.tsx-24-30 (1)
24-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing whitespace after inline
<strong>labels.Line 24 and Line 29 currently render concatenated text (e.g.
shellA,islandA) because there’s no whitespace node between</strong>and the following text.Suggested fix
- <strong>Composite server shell</strong>A server component owns the + <strong>Composite server shell</strong>{' '}A server component owns the visual frame while a client child hydrates only after it becomes visible. </Link> <Link to="/css" className="card"> - <strong>CSS module client island</strong>A server component renders a + <strong>CSS module client island</strong>{' '}A server component renders a client Hydrate boundary whose child uses CSS modules. </Link>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/react-start/rsc-deferred-hydration/src/routes/index.tsx` around lines 24 - 30, The JSX for the two Link cards concatenates the closing </strong> tag with the following text, producing strings like "shellA" and "islandA"; update the JSX in the Link elements (the one rendering "<strong>Composite server shell</strong>..." and the one rendering "<strong>CSS module client island</strong>...") to include an explicit whitespace node between </strong> and the following text (for example add a space or {' '} after each </strong>) so the label and description are separated correctly.docs/start/framework/solid/guide/deferred-hydration.md-2-2 (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a relative internal docs reference on Line 2.
refcurrently uses an absolute docs path; switch to a relative link format for consistency with docs conventions.Proposed fix
-ref: docs/start/framework/react/guide/deferred-hydration.md +ref: ../../react/guide/deferred-hydration.mdAs per coding guidelines: "
**/*.md: Use internal docs links relative to docs/ folder (e.g., ./guide/data-loading)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/start/framework/solid/guide/deferred-hydration.md` at line 2, The docs reference on Line 2 uses an absolute path; update the `ref` entry in docs/start/framework/solid/guide/deferred-hydration.md to use a relative docs link instead (replace "docs/start/framework/react/guide/deferred-hydration.md" with the relative path "../../react/guide/deferred-hydration.md") so it follows the project convention of internal docs links relative to the docs/ folder.docs/start/framework/react/guide/deferred-hydration.md-319-319 (1)
319-319:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the repo's internal docs link format here.
This should use the relative docs path without the
.mdsuffix, e.g../selective-ssr.As per coding guidelines "Use internal docs links relative to docs/ folder (e.g., ./guide/data-loading)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/start/framework/react/guide/deferred-hydration.md` at line 319, Update the internal docs link to use the repo's relative docs format by removing the .md suffix: change the link string `./selective-ssr.md` to `./selective-ssr` in the sentence referencing Selective SSR (keep the mention of `ClientOnly` intact); ensure the link uses the same relative path pattern used elsewhere in the docs (e.g., `./guide/...`) so the reference resolves correctly.e2e/solid-start/deferred-hydration/tests/hydration.spec.ts-99-101 (1)
99-101:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEscape the expected text before turning it into a regex.
htmlContainsTextcurrently treats the literal expectation as regex syntax, so an assertion containing(,.,?, etc. will match the wrong thing. Escaping first keeps the whitespace-flexibility without making the helper content-dependent.💡 Proposed fix
function htmlContainsText(html: string, text: string) { - const pattern = text.split(' ').join('(?:\\s|<!-- -->)+') + const escapedText = text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + const pattern = escapedText.split(' ').join('(?:\\s|<!-- -->)+') expect(html).toMatch(new RegExp(pattern)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/solid-start/deferred-hydration/tests/hydration.spec.ts` around lines 99 - 101, The helper htmlContainsText treats the provided text as raw regex; update it to escape regex metacharacters in the text before converting spaces to the whitespace-flexible pattern so literals like (.?+*) are matched literally. In the htmlContainsText function, create an escaped version of the text (e.g., escape all characters that match /[.*+?^${}()|[\]\\]/g) then replace spaces in that escaped string with '(?:\\s|<!-- -->)+' and use the resulting string to build the RegExp for expect(html).toMatch; keep the original html and text parameter names and the same behavior otherwise.packages/react-start-client/src/hydration/visible.tsx-147-166 (1)
147-166:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
gateRef.currentnot narrowed toVisibleGateat the JSX usage siteTypeScript's control flow analysis doesn't persist narrowing across callback boundaries. After the two
React.useEffectcalls (lines 118 and 135),gateRef.currentreverts toVisibleGate | undefinedat the type level. Line 155 passes it directly to<HydrationGate gate={...}>which requiresgate: VisibleGate, violating strict mode — the same pattern the non-null assertion on line 136 (const gate = gateRef.current!) already guards against.Suggested fix
+ const gate = gateRef.current! + return ( <div ref={markerRef} {...{ [hydrateIdAttribute]: id, }} > <React.Suspense fallback={props.fallback ?? null}> - <HydrationGate gate={gateRef.current}> + <HydrationGate gate={gate}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-start-client/src/hydration/visible.tsx` around lines 147 - 166, gateRef.current is typed as VisibleGate | undefined and TypeScript loses the narrowed type across effects, so pass a non-optional VisibleGate into <HydrationGate>: read and assert the ref once (e.g. const gate = gateRef.current!) after the effects where you already use a non-null assertion, then use gate in the JSX (<HydrationGate gate={gate}>), or alternatively conditionally render the JSX only when gateRef.current is truthy; reference gateRef, VisibleGate, and HydrationGate to locate and update the usage.
🧹 Nitpick comments (8)
packages/start-plugin-core/tests/vite-start-compiler-plugin.test.ts (1)
140-186: ⚡ Quick winConsider asserting
changedModuleinvalidation for completeness.The sibling test at line 135 asserts
expect(invalidatedModules).toContain(changedModule), but the new test only checkshydrateModule. If the intent is thatchangedModuleis always invalidated when the plugin processes it (matching the server-fn path), this assertion should be mirrored here; if the hydration path intentionally leaves source-file invalidation to Vite's native HMR, a brief comment to that effect would clarify the asymmetry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/tests/vite-start-compiler-plugin.test.ts` around lines 140 - 186, Add an assertion (or a clarifying comment) to mirror the sibling test: ensure that when invoking plugin.hotUpdate with modules containing changedModule the test also asserts invalidatedModules contains changedModule (in addition to hydrateModule) unless there is an intentional reason to leave changedModule to Vite HMR; reference the changedModule, hydrateModule, invalidatedModules and the plugin.hotUpdate call so you can locate and either add expect(invalidatedModules).toContain(changedModule) or a short comment explaining why that assertion is omitted.packages/start-client-core/src/hydration/media.ts (1)
5-16: ⚡ Quick winSilent early return for empty
querycan make aHydrateboundary permanently stuck — consider a dev-mode warning.
if (!query) returnexits without resolving the gate, so amedia('')boundary will never hydrate and the caller gets no indication of the mistake. Sincequery: stringaccepts an empty string at the type level, this is a reachable footgun.🛡️ Suggested guard
function listenForMedia(query: string, callback: () => void) { - if (!query) return + if (!query) { + if (process.env.NODE_ENV !== 'production') { + console.warn('[TanStack Start] media() called with an empty query string — the Hydrate boundary will never activate.') + } + return + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-client-core/src/hydration/media.ts` around lines 5 - 16, The function listenForMedia currently returns early on an empty query which can leave a Hydrate boundary stuck; change listenForMedia to detect an empty query, emit a dev-mode warning (console.warn) mentioning the empty media query/usage like media(''), immediately invoke the callback so the gate resolves, and return a no-op cleanup function; keep the existing behavior for non-empty queries that uses window.matchMedia, onChange handler, addEventListener/removeEventListener.packages/start-client-core/src/hydration/condition.ts (1)
12-23: 💤 Low valueParameter
conditionshadows the exported functioncondition— prefer a distinct name.The parameter on line 12 shares the identifier with the exported function, requiring mental disambiguation throughout the body. Renaming to
whenorinputeliminates this.♻️ Suggested rename
-export function condition(condition: HydrationCondition): HydrationStrategy { - const conditionValue = readCondition(condition) +export function condition(when: HydrationCondition): HydrationStrategy { + const conditionValue = readCondition(when) return { type: conditionType, key: `${conditionType}:${conditionValue ? '1' : '0'}`, - shouldDefer: () => !readCondition(condition), + shouldDefer: () => !readCondition(when), setup: ({ gate }) => { - if (readCondition(condition)) gate.resolve() + if (readCondition(when)) gate.resolve() }, } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-client-core/src/hydration/condition.ts` around lines 12 - 23, The parameter name shadows the exported function condition; rename the parameter (e.g., to when or input) wherever it appears in the condition function signature and body (the readCondition calls, key computation, shouldDefer, and setup closure) so the exported symbol condition is not shadowed; update the parameter type reference (HydrationCondition) and keep all uses of readCondition(condition) changed to readCondition(when) (or your chosen name) while preserving conditionType, HydrationStrategy, shouldDefer, and setup/gate.resolve behavior.packages/react-start-client/src/hydration/load.tsx (1)
34-34: 💤 Low valueConsider a safer return type for
HydratedBoundary.The cast
children as React.JSX.Elementmay be inaccurate whenchildrenisnull,undefined, an array, or a fragment. This could cause type mismatches downstream. A cleaner approach is to return<>{children}</>which always produces a valid JSX element.♻️ Suggested fix
- return children as React.JSX.Element + return <>{children}</>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-start-client/src/hydration/load.tsx` at line 34, The current HydratedBoundary returns children using an unsafe cast (children as React.JSX.Element) which can be incorrect for null/undefined/arrays/fragments; update the return in the HydratedBoundary component to wrap children in a fragment (i.e., return <>{children}</>) so the component always returns a valid JSX element and avoids incorrect type assertions.e2e/react-start/rsc-deferred-hydration/tests/hydration.spec.ts (1)
21-27: 💤 Low valueReliance on React internal property naming may be fragile.
The check
key.startsWith('__react')depends on React's internal property naming convention which could change between React versions. Consider adding a comment documenting this assumption, or using a more stable detection mechanism if available.📝 Suggested documentation
async function waitForHydrateMarkerToMount(page: Page, id: string) { + // React attaches internal properties starting with '__react' to hydrated DOM nodes. + // This heuristic may need updating if React changes its internal naming convention. await page.waitForFunction((testId) => { const button = document.querySelector(`[data-testid="${testId}-button"]`) const marker = button?.closest('[data-ts-hydrate-id]') return Object.keys(marker ?? {}).some((key) => key.startsWith('__react')) }, id) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/react-start/rsc-deferred-hydration/tests/hydration.spec.ts` around lines 21 - 27, The current implementation in waitForHydrateMarkerToMount relies on detecting React internals by checking Object.keys(marker ?? {}).some((key) => key.startsWith('__react')), which is fragile; update this by either replacing the check with a more stable detection (e.g., looking for a stable attribute or marker set by your app framework) or, if no stable alternative exists, add a clear comment above the check documenting the assumption that React internals start with "__react" and why it's acceptable here, referencing the function waitForHydrateMarkerToMount and the marker variable so reviewers know exactly where the fragile check occurs.e2e/react-start/deferred-hydration/tests/hydration.spec.ts (1)
99-102: 💤 Low valueLow risk of ReDoS in test context, but consider escaping special characters.
The static analysis flags potential ReDoS from constructing a regex from variable input. While the
textparameter comes from test constants (low risk), the pattern doesn't escape regex metacharacters. Iftextcontains characters like.,*,(, etc., the regex behavior may be unexpected.♻️ Optional: escape regex special characters
function htmlContainsText(html: string, text: string) { - const pattern = text.split(' ').join('(?:\\s|<!-- -->)+') + const escaped = text.split(' ').map(word => + word.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + ) + const pattern = escaped.join('(?:\\s|<!-- -->)+') expect(html).toMatch(new RegExp(pattern)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/react-start/deferred-hydration/tests/hydration.spec.ts` around lines 99 - 102, The test helper htmlContainsText builds a RegExp from unescaped input (variable text) which can misinterpret regex metacharacters; update htmlContainsText to escape regex-special characters in each token of text before joining with the (?:\\s|<!-- -->)+ separator so the produced pattern matches literal characters, then construct the RegExp with the escaped pattern (i.e., escape characters like . * + ? ^ $ { } ( ) | [ ] \\ before joining and keep using new RegExp(pattern) as currently done).packages/solid-start-client/src/hydration/visible.tsx (2)
114-118: 💤 Low valueConsider adding proper type definitions to avoid
anycasts.The
gate as anycast indicates a type mismatch between the localVisibleGateand the expected strategy gate interface. This reduces type safety and could mask runtime issues.♻️ Suggested type alignment
Consider defining a shared gate interface that both the local gate and strategy setup expect:
// Define a minimal gate interface that satisfies the strategy contract interface StrategyGate { resolved: boolean resolve: () => void } // Then the setup call becomes type-safe: const cleanup = strategy.setup?.({ element: markerElement ?? null, gate: gate satisfies StrategyGate, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/solid-start-client/src/hydration/visible.tsx` around lines 114 - 118, The gate is being cast to any when calling strategy.setup, masking a type mismatch; declare a minimal shared gate interface (e.g., StrategyGate with resolved: boolean and resolve: () => void) and update the local gate variable/type (the VisibleGate or the variable named gate) and/or the strategy.setup signature to use that interface so you can pass gate directly instead of using `as any`; ensure the gate satisfies the interface (or make strategy.setup generic over the gate type) and remove the `as any` cast in the call to strategy.setup({ element: markerElement ?? null, gate }).
171-180: 💤 Low valueType the strategy callback parameters for better type safety.
The
anycasts on lines 174 and 176 lose all type information for the callback parameters. Since these match defined interfaces (HydrationRuntimeContext), explicit typing would catch mismatches.♻️ Suggested typing
return { type: visibleType, key: visibleType, - setup: ({ element, gate }: any) => + setup: ({ element, gate }) => observeVisible(element, gate.resolve, rootMargin, threshold), - setupPrefetch: ({ element, prefetch }: any) => + setupPrefetch: ({ element, prefetch }) => observeVisible(element, prefetch, rootMargin, threshold), $$renderHydrate: StrategyHydrate, }The return type annotation
SolidHydrationStrategy & HydrationPrefetchStrategyshould provide inference for these callbacks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/solid-start-client/src/hydration/visible.tsx` around lines 171 - 180, The callbacks for setup and setupPrefetch are currently typed with any; replace those any casts by using the proper HydrationRuntimeContext parameter type (or the inferred parameter type from the SolidHydrationStrategy & HydrationPrefetchStrategy return) so the signatures read setup: ({ element, gate }: HydrationRuntimeContext) and setupPrefetch: ({ element, prefetch }: HydrationRuntimeContext); keep the same bodies calling observeVisible(element, gate.resolve, rootMargin, threshold) and observeVisible(element, prefetch, rootMargin, threshold) and preserve visibleType and $$renderHydrate: StrategyHydrate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/react-start/deferred-hydration/server.js`:
- Around line 61-79: The current flow calls stat(filePath) then calls
createReadStream(filePath) which opens a TOCTOU window and can emit an unhandled
'error' if the file disappears; to fix, attach error handlers to the readable
stream returned by createReadStream (and to the piped destination) so any FS
errors are caught and handled: for example, on stream 'error' set an appropriate
status (404/500) if headers not sent and end the response, or destroy the
response if already sent, and also listen for 'error' on the stream returned by
pipe/res to avoid uncaught exceptions; update the code around stat(),
createReadStream(filePath), and the pipe to ensure all stream errors are handled
gracefully.
In `@e2e/react-start/rsc-deferred-hydration/server.js`:
- Around line 22-39: The start function that spawns the child process only
attaches an 'exit' listener to child; add a child.once('exit', ...) and also
attach child.once('error', err => ...) so spawn failures are handled explicitly.
In the 'error' handler log the error (e.g., console.error or existing logger)
and call process.exit(1); keep the existing signal handling and
process.exit(code ?? 0) behavior in the exit handler but switch to once to avoid
double-processing when an error occurs.
In `@e2e/solid-start/deferred-hydration/server.js`:
- Around line 61-79: The code currently races between stat() and
createReadStream(), so attach an error handler to the stream created by
createReadStream(filePath) to avoid unhandled exceptions; specifically, after
calling createReadStream(filePath) store the stream in a variable (e.g., const
stream = createReadStream(filePath)), add stream.on('error', err => { if
(!res.headersSent) { res.statusCode = 500; res.setHeader('content-type',
'text/plain'); res.end('Internal Server Error'); } else { try {
res.destroy(err); } catch {} } }), and then pipe with stream.pipe(res); also
ensure the HEAD branch still ends early (res.end()) before creating the stream
to avoid creating it for HEAD requests.
In `@packages/react-start-client/src/lazyHydratedComponent.tsx`:
- Around line 4-8: The public helper lazyHydratedComponent uses a too-permissive
importer type; change its importer parameter from () => Promise<any> to a
stricter shape such as () => Promise<Record<string, any>> (or a generic TModule
extends Record<string, any>) so it matches lazyRouteComponent's expectation and
preserves module typing; update the lazyHydratedComponent signature (and its
generic bounds if you add TModule) to reference Record<string, any> and keep the
return type AsyncRouteComponent<TProps>.
In `@packages/router-utils/src/compiler-helpers.ts`:
- Around line 104-149: The helpers only unwrap ExportNamedDeclaration, so named
default exports (export default function foo() {} / export default class Foo {})
are ignored; update both collectLocalBindingsFromStatement and
buildDeclarationMap to also unwrap ExportDefaultDeclaration when .declaration
exists (i.e., treat node/statement = t.isExportNamedDeclaration(...) &&
.declaration ? .declaration : t.isExportDefaultDeclaration(...) && .declaration
? .declaration : node/statement) and then keep the existing logic to add
function/class ids and variable declarator pattern identifiers to bindings/map
for the unwrapped declaration.
- Around line 7-62: collectIdentifiersFromNode currently descends into nested
scopes without tracking shadowed bindings, causing identifiers declared in inner
functions/classes to be reported as module-level dependencies; modify the inner
walk in collectIdentifiersFromNode to be scope-aware by threading a scope set
(or stack) of locally-declared names through each recursive call and updating it
when entering binding-creating nodes (e.g., FunctionDeclaration,
FunctionExpression, ArrowFunctionExpression params, ClassDeclaration,
VariableDeclarator ids, CatchClause params, ImportDeclaration specifiers, and
possibly TypeAlias/Interface names), then when you encounter an identifier
(t.isIdentifier or t.isJSXIdentifier) only add it to ids if it is not present in
any active scope in that scope stack; update the walk signature (e.g.,
walk(current, parent, grandparent, parentKey, scope)) and ensure you
clone/extend the scope for nested scopes so shadowing is respected while still
descending into inner nodes.
In `@packages/solid-start-client/src/GenericHydrate.tsx`:
- Line 52: The file uses type-assertion escape hatches (props as
InternalHydrateProps, Dynamic as any, Solid.Show as any) in the GenericHydrate
runtime; update GenericHydrate's function signature to accept a properly typed
union/augmented props type (include InternalHydrateProps in the
generic/parameter) and remove all "as any" assertions; render Dynamic and
Solid.Show with typed JSX using spread props (e.g., <Dynamic {...typedProps}>
and <Solid.Show when={typedCondition}>), ensuring InternalHydrateProps fields
are declared on the typed props so the compiler enforces shapes; apply the same
typed-props changes to the other occurrences referenced (around the 213-265
region) so no type assertions remain.
In `@packages/solid-start-client/src/hydration/load.tsx`:
- Around line 42-72: The code disables TypeScript safety by using
createComponent(Dynamic as any, ...) — remove the `as any` cast and switch to
typed JSX so Dynamic is used with its proper types; replace the
createComponent(Dynamic as any, { component: 'div', get [hydrateIdAttribute]() {
return id }, [hydrateWhenAttribute]: loadType, ... }) invocation with an
equivalent JSX element (e.g., <Dynamic component="div" {...}>) that passes
hydrateIdAttribute, hydrateWhenAttribute, children (Suspense wrapping
HydratedBoundary) and the same getters/props, ensuring the Dynamic component
reference and HydratedBoundary, hydrateIdAttribute, hydrateWhenAttribute,
props.fallback, props.onHydrated and internalProps.when.onHydrated are all
passed with correct typings instead of casting to any.
In `@packages/solid-start-client/src/lazyHydratedComponent.tsx`:
- Around line 4-6: The current lazyHydratedComponent signature uses broad any
types causing type leakage; tighten it by making importer generic over a module
type and constraining exportName to keys of that module and the exported
component to be a component type that accepts the generic props. Specifically,
change the signature of lazyHydratedComponent<TProps, TModule, TExport> so
importer is () => Promise<TModule>, exportName is TExport extends keyof TModule,
and require that TModule[TExport] is a Component type (or a function accepting
TProps) so the function returns a typed component consuming TProps; update
references inside lazyHydratedComponent to use these generics instead of any.
In `@packages/start-client-core/src/hydration/interaction.ts`:
- Around line 121-151: createReplayEvent currently only handles MouseEvent and
FocusEvent so KeyboardEvent falls back to a generic Event and loses
key/code/modifier info; add a branch in createReplayEvent that detects event
instanceof KeyboardEvent and returns new KeyboardEvent(event.type, { ...init,
key: event.key, code: event.code, altKey: event.altKey, ctrlKey: event.ctrlKey,
shiftKey: event.shiftKey, metaKey: event.metaKey, repeat: event.repeat,
isComposing: (event as KeyboardEvent).isComposing }) so replayed keydown/keyup
retain key, code and modifier properties for HydrationInteractionEvent
consumers. Ensure you reference createReplayEvent and the KeyboardEvent
constructor when implementing.
In `@packages/start-plugin-core/src/start-compiler/compiler.ts`:
- Around line 1040-1059: The early exit in the builtInTransforms block returns
before calling this.ingestModule when fileKinds.size === 0, so modules without
local candidates never get cached (breaking getTransitiveImporters()/moduleCache
traversal); move or invoke this.ingestModule({ code, id, parserFilename })
before the empty-checked early-break (or replace the break with a conditional
flow that still calls ingestModule) so every visited module is cached even when
fileKinds is empty, ensuring getTransitiveImporters() can see plain importer
modules; update references around builtInTransforms, fileKinds, ingestModule,
moduleCache and getTransitiveImporters accordingly.
In `@packages/start-plugin-core/src/start-compiler/host.ts`:
- Around line 98-111: The current createCompilerVirtualModuleIdPattern function
composes multiple plugin virtualModuleIdPattern Regexes into one via .source,
which loses flags and can create invalid regexes; change it to preserve original
patterns by returning the single plugin RegExp unchanged when exactly one
pattern exists, and return undefined when there are zero or multiple patterns so
callers can instead iterate plugins and test each plugin.virtualModuleIdPattern
against the id (or implement sequential checks elsewhere); reference
createCompilerVirtualModuleIdPattern and
StartCompilerPlugin.virtualModuleIdPattern to locate where to make this change.
In `@packages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.ts`:
- Around line 482-487: The current call to mergeReachableHydrationChunkData uses
options.entryChunk (the global client entry) which walks the entire client graph
and pulls unrelated hydration CSS into the root route; instead, build a
route-scoped starting set and only traverse chunks owned by the root route.
Replace the call that passes options.entryChunk with a route-scoped chunk set
(e.g., derive the root route's entry chunk(s) or filter options.chunksByFileName
for chunks whose owner/entry matches rootRoute or whose importers include
rootRoute's module id), then call mergeReachableHydrationChunkData for that
scoped chunk set (or pass a filtered chunksByFileName) so only hydration assets
reachable from the rootRoute itself are merged; keep
dedupeNestedRouteManifestEntries unchanged.
In
`@packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenMultiple.tsx`:
- Around line 1-6: The snapshot contains hardcoded absolute machine paths in the
generated dynamic import strings for the lazy-hydrated components
(_Hydrate_3/_Hydrate_2/_Hydrate_) which will break CI and other dev machines;
update the snapshot-generation step that emits those import strings to normalize
paths (e.g., replace the project root absolute prefix with a stable placeholder
like <rootDir> or emit repo-relative paths) before writing snapshots, ensure the
normalization is applied to the code that builds the dynamic import argument,
and then regenerate the snapshots so the new normalized strings replace the
original absolute paths.
In
`@packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNested.tsx`:
- Around line 1-3: The snapshot contains host-specific absolute import
specifiers (e.g., import(...) paths inside generated lines for _Hydrate_2 and
_Hydrate_ that include /Users/caligano/...), so update the snapshot emission
logic to normalize those IDs into machine-agnostic forms (e.g., relative paths
or stable hashed identifiers). Locate the code that generates these dynamic
import specifiers (the logic producing strings like
"hydrateWhenNested_383a2d73fd" passed into _lazyHydratedComponent) and replace
absolute filesystem prefixes with a normalized token (strip workspace root or
replace with a deterministic prefix or use only the hydrateWhenNested_<hash>
part). Ensure the updated emitter still preserves uniqueness (keep the existing
hash suffix) and update tests/snapshot serializer to produce the normalized
import specifier for _lazyHydratedComponent and related symbols.
In
`@packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNever.tsx`:
- Line 1: The snapshot import string for the lazy hydrated component (_Hydrate_
created via _lazyHydratedComponent) embeds a machine-specific absolute path and
a splitId hash (e.g., hydrateWhenNever_9a00c8d701), which breaks CI; update the
transform that builds the import URL so it normalizes absolute file paths to a
repo-relative form (or replaces the project-root prefix with a stable
placeholder like "<rootDir>") before constructing the import string and before
computing the splitId/hash, ensuring _lazyHydratedComponent(...) import
arguments and generated splitId are deterministic across machines.
In
`@packages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.Hydrate_0.client.tsx`:
- Around line 1-2: The snapshot contains a machine-specific absolute import path
in the generated lazy import inside the virtual snapshot (see the
_lazyHydratedComponent call that imports
"…/hydrateWhenNested.tsx?tss-hydrate=hydrateWhenNested_895e8f985c"), so update
the build/test pipeline to normalize emitted import paths before snapshots are
written/compared: either change the compiler transform that emits the import to
produce a repo-root-relative path (e.g. "/packages/...") or add a snapshot
serializer / expect.addSnapshotSerializer that replaces the repository absolute
root prefix with a stable placeholder (or strips it) for all snapshots under
snapshots/virtual so the generated _Hydrate_ import path is deterministic across
machines.
---
Minor comments:
In `@docs/start/framework/react/guide/deferred-hydration.md`:
- Line 319: Update the internal docs link to use the repo's relative docs format
by removing the .md suffix: change the link string `./selective-ssr.md` to
`./selective-ssr` in the sentence referencing Selective SSR (keep the mention of
`ClientOnly` intact); ensure the link uses the same relative path pattern used
elsewhere in the docs (e.g., `./guide/...`) so the reference resolves correctly.
In `@docs/start/framework/solid/guide/deferred-hydration.md`:
- Line 2: The docs reference on Line 2 uses an absolute path; update the `ref`
entry in docs/start/framework/solid/guide/deferred-hydration.md to use a
relative docs link instead (replace
"docs/start/framework/react/guide/deferred-hydration.md" with the relative path
"../../react/guide/deferred-hydration.md") so it follows the project convention
of internal docs links relative to the docs/ folder.
In `@e2e/react-start/rsc-deferred-hydration/src/routes/index.tsx`:
- Around line 24-30: The JSX for the two Link cards concatenates the closing
</strong> tag with the following text, producing strings like "shellA" and
"islandA"; update the JSX in the Link elements (the one rendering
"<strong>Composite server shell</strong>..." and the one rendering "<strong>CSS
module client island</strong>...") to include an explicit whitespace node
between </strong> and the following text (for example add a space or {' '} after
each </strong>) so the label and description are separated correctly.
In `@e2e/solid-start/deferred-hydration/tests/hydration.spec.ts`:
- Around line 99-101: The helper htmlContainsText treats the provided text as
raw regex; update it to escape regex metacharacters in the text before
converting spaces to the whitespace-flexible pattern so literals like (.?+*) are
matched literally. In the htmlContainsText function, create an escaped version
of the text (e.g., escape all characters that match /[.*+?^${}()|[\]\\]/g) then
replace spaces in that escaped string with '(?:\\s|<!-- -->)+' and use the
resulting string to build the RegExp for expect(html).toMatch; keep the original
html and text parameter names and the same behavior otherwise.
In `@packages/react-start-client/src/hydration/visible.tsx`:
- Around line 147-166: gateRef.current is typed as VisibleGate | undefined and
TypeScript loses the narrowed type across effects, so pass a non-optional
VisibleGate into <HydrationGate>: read and assert the ref once (e.g. const gate
= gateRef.current!) after the effects where you already use a non-null
assertion, then use gate in the JSX (<HydrationGate gate={gate}>), or
alternatively conditionally render the JSX only when gateRef.current is truthy;
reference gateRef, VisibleGate, and HydrationGate to locate and update the
usage.
In `@packages/start-server-core/tests/transformAssets.test.ts`:
- Line 4: Remove the unused import buildManifestWithClientEntry from the imports
at the top of tests/transformAssets.test.ts; locate the import list that
currently includes buildManifestWithClientEntry and delete that symbol so the
file only imports what it actually uses, then run the TypeScript/linters to
confirm no unused-import errors remain.
---
Duplicate comments:
In
`@packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenRenamed.tsx`:
- Around line 1-2: The snapshot contains an absolute local path in the dynamic
import call (see _lazyHydratedComponent and the generated import string
containing hydrateWhenRenamed_ad6838514c), so update the snapshot generator's
path-normalization step to replace machine-specific absolute paths with a
normalized relative or repo-root-based path (preserving the filename and query
params like ?tss-hydrate=...&tss-hydrate-index=...), then regenerate the
snapshots (including hydrateWhenRenamed.tsx and hydrateWhenMultiple.tsx) so the
_Hydrate_/_Hydrate_0_preload lines use the normalized path instead of
/Users/caligano/...
---
Nitpick comments:
In `@e2e/react-start/deferred-hydration/tests/hydration.spec.ts`:
- Around line 99-102: The test helper htmlContainsText builds a RegExp from
unescaped input (variable text) which can misinterpret regex metacharacters;
update htmlContainsText to escape regex-special characters in each token of text
before joining with the (?:\\s|<!-- -->)+ separator so the produced pattern
matches literal characters, then construct the RegExp with the escaped pattern
(i.e., escape characters like . * + ? ^ $ { } ( ) | [ ] \\ before joining and
keep using new RegExp(pattern) as currently done).
In `@e2e/react-start/rsc-deferred-hydration/tests/hydration.spec.ts`:
- Around line 21-27: The current implementation in waitForHydrateMarkerToMount
relies on detecting React internals by checking Object.keys(marker ??
{}).some((key) => key.startsWith('__react')), which is fragile; update this by
either replacing the check with a more stable detection (e.g., looking for a
stable attribute or marker set by your app framework) or, if no stable
alternative exists, add a clear comment above the check documenting the
assumption that React internals start with "__react" and why it's acceptable
here, referencing the function waitForHydrateMarkerToMount and the marker
variable so reviewers know exactly where the fragile check occurs.
In `@packages/react-start-client/src/hydration/load.tsx`:
- Line 34: The current HydratedBoundary returns children using an unsafe cast
(children as React.JSX.Element) which can be incorrect for
null/undefined/arrays/fragments; update the return in the HydratedBoundary
component to wrap children in a fragment (i.e., return <>{children}</>) so the
component always returns a valid JSX element and avoids incorrect type
assertions.
In `@packages/solid-start-client/src/hydration/visible.tsx`:
- Around line 114-118: The gate is being cast to any when calling
strategy.setup, masking a type mismatch; declare a minimal shared gate interface
(e.g., StrategyGate with resolved: boolean and resolve: () => void) and update
the local gate variable/type (the VisibleGate or the variable named gate) and/or
the strategy.setup signature to use that interface so you can pass gate directly
instead of using `as any`; ensure the gate satisfies the interface (or make
strategy.setup generic over the gate type) and remove the `as any` cast in the
call to strategy.setup({ element: markerElement ?? null, gate }).
- Around line 171-180: The callbacks for setup and setupPrefetch are currently
typed with any; replace those any casts by using the proper
HydrationRuntimeContext parameter type (or the inferred parameter type from the
SolidHydrationStrategy & HydrationPrefetchStrategy return) so the signatures
read setup: ({ element, gate }: HydrationRuntimeContext) and setupPrefetch: ({
element, prefetch }: HydrationRuntimeContext); keep the same bodies calling
observeVisible(element, gate.resolve, rootMargin, threshold) and
observeVisible(element, prefetch, rootMargin, threshold) and preserve
visibleType and $$renderHydrate: StrategyHydrate.
In `@packages/start-client-core/src/hydration/condition.ts`:
- Around line 12-23: The parameter name shadows the exported function condition;
rename the parameter (e.g., to when or input) wherever it appears in the
condition function signature and body (the readCondition calls, key computation,
shouldDefer, and setup closure) so the exported symbol condition is not
shadowed; update the parameter type reference (HydrationCondition) and keep all
uses of readCondition(condition) changed to readCondition(when) (or your chosen
name) while preserving conditionType, HydrationStrategy, shouldDefer, and
setup/gate.resolve behavior.
In `@packages/start-client-core/src/hydration/media.ts`:
- Around line 5-16: The function listenForMedia currently returns early on an
empty query which can leave a Hydrate boundary stuck; change listenForMedia to
detect an empty query, emit a dev-mode warning (console.warn) mentioning the
empty media query/usage like media(''), immediately invoke the callback so the
gate resolves, and return a no-op cleanup function; keep the existing behavior
for non-empty queries that uses window.matchMedia, onChange handler,
addEventListener/removeEventListener.
In `@packages/start-plugin-core/tests/vite-start-compiler-plugin.test.ts`:
- Around line 140-186: Add an assertion (or a clarifying comment) to mirror the
sibling test: ensure that when invoking plugin.hotUpdate with modules containing
changedModule the test also asserts invalidatedModules contains changedModule
(in addition to hydrateModule) unless there is an intentional reason to leave
changedModule to Vite HMR; reference the changedModule, hydrateModule,
invalidatedModules and the plugin.hotUpdate call so you can locate and either
add expect(invalidatedModules).toContain(changedModule) or a short comment
explaining why that assertion is omitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 488e115a-0178-4852-9471-8019a76d80f6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (195)
.changeset/deferred-hydration-start.mdbenchmarks/bundle-size/README.mdbenchmarks/bundle-size/scenarios/react-start-deferred-hydration/src/router.tsxbenchmarks/bundle-size/scenarios/react-start-deferred-hydration/src/routes/__root.tsxbenchmarks/bundle-size/scenarios/react-start-deferred-hydration/src/routes/about.tsxbenchmarks/bundle-size/scenarios/react-start-deferred-hydration/src/routes/index.tsxbenchmarks/bundle-size/scenarios/react-start-deferred-hydration/vite.config.tsbenchmarks/bundle-size/scenarios/solid-start-deferred-hydration/src/router.tsxbenchmarks/bundle-size/scenarios/solid-start-deferred-hydration/src/routes/__root.tsxbenchmarks/bundle-size/scenarios/solid-start-deferred-hydration/src/routes/about.tsxbenchmarks/bundle-size/scenarios/solid-start-deferred-hydration/src/routes/index.tsxbenchmarks/bundle-size/scenarios/solid-start-deferred-hydration/vite.config.tsdocs/start/config.jsondocs/start/framework/react/guide/deferred-hydration.mddocs/start/framework/solid/guide/deferred-hydration.mde2e/react-start/deferred-hydration/.gitignoree2e/react-start/deferred-hydration/package.jsone2e/react-start/deferred-hydration/playwright.config.tse2e/react-start/deferred-hydration/rsbuild.config.tse2e/react-start/deferred-hydration/server.jse2e/react-start/deferred-hydration/src/routeTree.gen.tse2e/react-start/deferred-hydration/src/router.tsxe2e/react-start/deferred-hydration/src/routes/__root.tsxe2e/react-start/deferred-hydration/src/routes/components.tsxe2e/react-start/deferred-hydration/src/routes/css.tsxe2e/react-start/deferred-hydration/src/routes/css/deferred-only.module.csse2e/react-start/deferred-hydration/src/routes/css/outer.module.csse2e/react-start/deferred-hydration/src/routes/css/shared.module.csse2e/react-start/deferred-hydration/src/routes/imported.tsxe2e/react-start/deferred-hydration/src/routes/index.tsxe2e/react-start/deferred-hydration/src/shared/ImportedHydrateWidget.tsxe2e/react-start/deferred-hydration/tests/hydration.spec.tse2e/react-start/deferred-hydration/tsconfig.jsone2e/react-start/deferred-hydration/vite.config.tse2e/react-start/rsc-deferred-hydration/.gitignoree2e/react-start/rsc-deferred-hydration/package.jsone2e/react-start/rsc-deferred-hydration/playwright.config.tse2e/react-start/rsc-deferred-hydration/server.jse2e/react-start/rsc-deferred-hydration/src/components/CssHydrateIsland.module.csse2e/react-start/rsc-deferred-hydration/src/components/CssHydrateIsland.tsxe2e/react-start/rsc-deferred-hydration/src/components/DeferredHydrateIsland.tsxe2e/react-start/rsc-deferred-hydration/src/routeTree.gen.tse2e/react-start/rsc-deferred-hydration/src/router.tsxe2e/react-start/rsc-deferred-hydration/src/routes/__root.tsxe2e/react-start/rsc-deferred-hydration/src/routes/composite.tsxe2e/react-start/rsc-deferred-hydration/src/routes/css.tsxe2e/react-start/rsc-deferred-hydration/src/routes/index.tsxe2e/react-start/rsc-deferred-hydration/src/routes/server-client.tsxe2e/react-start/rsc-deferred-hydration/src/server/serverHydrateComponents.tsxe2e/react-start/rsc-deferred-hydration/src/server/serverHydrateContent.tsxe2e/react-start/rsc-deferred-hydration/tests/hydration.spec.tse2e/react-start/rsc-deferred-hydration/tests/setup/global.setup.tse2e/react-start/rsc-deferred-hydration/tests/setup/global.teardown.tse2e/react-start/rsc-deferred-hydration/tsconfig.jsone2e/react-start/rsc-deferred-hydration/vite.config.tse2e/react-start/server-routes/src/routeTree.gen.tse2e/solid-start/deferred-hydration/.gitignoree2e/solid-start/deferred-hydration/package.jsone2e/solid-start/deferred-hydration/playwright.config.tse2e/solid-start/deferred-hydration/rsbuild.config.tse2e/solid-start/deferred-hydration/server.jse2e/solid-start/deferred-hydration/src/routeTree.gen.tse2e/solid-start/deferred-hydration/src/router.tsxe2e/solid-start/deferred-hydration/src/routes/__root.tsxe2e/solid-start/deferred-hydration/src/routes/components.tsxe2e/solid-start/deferred-hydration/src/routes/css.tsxe2e/solid-start/deferred-hydration/src/routes/css/deferred-only.module.csse2e/solid-start/deferred-hydration/src/routes/css/outer.module.csse2e/solid-start/deferred-hydration/src/routes/css/shared.module.csse2e/solid-start/deferred-hydration/src/routes/imported.tsxe2e/solid-start/deferred-hydration/src/routes/index.tsxe2e/solid-start/deferred-hydration/src/shared/ImportedHydrateWidget.tsxe2e/solid-start/deferred-hydration/tests/hydration.spec.tse2e/solid-start/deferred-hydration/tsconfig.jsone2e/solid-start/deferred-hydration/vite.config.tspackages/react-router/src/index.tsxpackages/react-start-client/package.jsonpackages/react-start-client/src/GenericHydrate.tsxpackages/react-start-client/src/Hydrate.tsxpackages/react-start-client/src/hydration.tspackages/react-start-client/src/hydration/generic.tspackages/react-start-client/src/hydration/idle.tspackages/react-start-client/src/hydration/load.tsxpackages/react-start-client/src/hydration/never.tsxpackages/react-start-client/src/hydration/visible.tsxpackages/react-start-client/src/index.tsxpackages/react-start-client/src/lazyHydratedComponent.tsxpackages/react-start-client/src/tests/Hydrate.test-d.tsxpackages/react-start-client/src/tests/Hydrate.test.tsxpackages/react-start-client/vite.config.tspackages/react-start/package.jsonpackages/react-start/src/hydration.tspackages/react-start/src/index.tspackages/react-start/vite.config.tspackages/router-core/src/index.tspackages/router-core/src/ssr/ssr-server.tspackages/router-plugin/src/core/code-splitter/compilers.tspackages/router-plugin/src/core/code-splitter/plugins.tspackages/router-plugin/src/core/config.tspackages/router-plugin/src/core/router-code-splitter-plugin.tspackages/router-plugin/src/index.tspackages/router-plugin/tests/code-splitter.test.tspackages/router-utils/src/compiler-helpers.tspackages/router-utils/src/index.tspackages/router-utils/src/path-ids.tspackages/solid-router/src/ClientOnly.tsxpackages/solid-start-client/package.jsonpackages/solid-start-client/src/GenericHydrate.tsxpackages/solid-start-client/src/Hydrate.tsxpackages/solid-start-client/src/hydration.tspackages/solid-start-client/src/hydration/generic.tspackages/solid-start-client/src/hydration/idle.tspackages/solid-start-client/src/hydration/load.tsxpackages/solid-start-client/src/hydration/never.tspackages/solid-start-client/src/hydration/visible.tsxpackages/solid-start-client/src/index.tsxpackages/solid-start-client/src/lazyHydratedComponent.tsxpackages/solid-start-client/src/tests/Hydrate.test-d.tsxpackages/solid-start-client/vite.config.tspackages/solid-start/package.jsonpackages/solid-start/src/hydration.tspackages/solid-start/src/index.tspackages/solid-start/vite.config.tspackages/start-client-core/package.jsonpackages/start-client-core/src/hydration.tspackages/start-client-core/src/hydration/condition.tspackages/start-client-core/src/hydration/constants.tspackages/start-client-core/src/hydration/idle.tspackages/start-client-core/src/hydration/interaction.tspackages/start-client-core/src/hydration/load.tspackages/start-client-core/src/hydration/media.tspackages/start-client-core/src/hydration/never.tspackages/start-client-core/src/hydration/runtime.tspackages/start-client-core/src/hydration/types.tspackages/start-client-core/src/hydration/visible.tspackages/start-client-core/vite.config.tspackages/start-plugin-core/src/hydrate-when-transform.tspackages/start-plugin-core/src/hydration-constants.tspackages/start-plugin-core/src/rsbuild/normalized-client-build.tspackages/start-plugin-core/src/rsbuild/start-compiler-host.tspackages/start-plugin-core/src/start-compiler/compiler.tspackages/start-plugin-core/src/start-compiler/config.tspackages/start-plugin-core/src/start-compiler/host.tspackages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.tspackages/start-plugin-core/src/types.tspackages/start-plugin-core/src/vite/start-compiler-plugin/plugin.tspackages/start-plugin-core/src/vite/start-manifest-plugin/normalized-client-build.tspackages/start-plugin-core/tests/hydrate-when-transform.test.tspackages/start-plugin-core/tests/hydrateWhen/error-files/hydrateWhenFunctionChild.tsxpackages/start-plugin-core/tests/hydrateWhen/error-files/hydrateWhenHookCall.tsxpackages/start-plugin-core/tests/hydrateWhen/error-files/hydrateWhenSuperCapture.tsxpackages/start-plugin-core/tests/hydrateWhen/error-files/hydrateWhenThisCapture.tsxpackages/start-plugin-core/tests/hydrateWhen/hydrateWhen.test.tspackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenBasic.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenMultiple.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNested.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNever.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNoImport.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNotFromTanstack.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenObjectFallback.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenRenamed.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenSplitFalse.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenSplitFalseFunctionChild.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenWrongImportName.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenBasic.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenMultiple.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenNested.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenNever.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenNoImport.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenNotFromTanstack.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenObjectFallback.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenRenamed.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenSplitFalse.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenSplitFalseFunctionChild.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenWrongImportName.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.Hydrate_0.client.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.Hydrate_0.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.Hydrate_2.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenBasic.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenMultiple.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenNested.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenNever.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenNoImport.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenNotFromTanstack.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenObjectFallback.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenRenamed.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenSplitFalse.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenSplitFalseFunctionChild.tsxpackages/start-plugin-core/tests/hydrateWhen/test-files/hydrateWhenWrongImportName.tsxpackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.tspackages/start-plugin-core/tests/vite-start-compiler-plugin.test.tspackages/start-plugin-core/tsconfig.jsonpackages/start-server-core/src/createStartHandler.tspackages/start-server-core/tests/transformAssets.test.tsscripts/benchmarks/bundle-size/measure.mjs
Comment on lines
+22
to
+39
| export function start() { | ||
| const child = spawn( | ||
| 'srvx', | ||
| ['--prod', '-s', resolveDistClientDir(), resolveDistServerEntryPath()], | ||
| { | ||
| stdio: 'inherit', | ||
| shell: process.platform === 'win32', | ||
| }, | ||
| ) | ||
|
|
||
| child.on('exit', (code, signal) => { | ||
| if (signal) { | ||
| process.kill(process.pid, signal) | ||
| return | ||
| } | ||
|
|
||
| process.exit(code ?? 0) | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "server.js" | grep -E "e2e.*rsc-deferred-hydration"Repository: TanStack/router
Length of output: 110
🏁 Script executed:
cat -n e2e/react-start/rsc-deferred-hydration/server.js 2>/dev/null || echo "File not found"Repository: TanStack/router
Length of output: 1441
🌐 Web query:
Node.js spawn child process error event vs exit event handling
💡 Result:
In Node.js when you spawn a child process (child_process.spawn), you generally use these three different signals: 1) 'error' event (spawn/kill/message failures) - The ChildProcess 'error' event is emitted when Node.js cannot spawn the child, cannot kill it, cannot deliver a message, or when the child is aborted via the signal option. [1] - Importantly: 'error' is not emitted just because the child program printed to stderr or exited with a non-zero exit code. Those are separate concerns from Node transport/spawn failures. [1] 2) 'exit' event (process finished) - 'exit' is emitted after the child process ends. The handler receives (code, signal): code is the final exit code if it exited normally, otherwise null; signal is the terminating signal name if it was killed by a signal, otherwise null. One of code/signal is always non-null. [1] 3) 'close' event (streams closed) - 'close' is emitted after the child has ended and the stdio streams of the child have been closed. Node’s docs emphasize it is distinct from 'exit' (multiple processes might share stdio), and that 'close' always happens after 'exit' was already emitted (or after 'error' if the child failed to spawn). [1] So which should you handle? - If your goal is “did the child program finish successfully/fail with an exit code?”, listen to 'exit' (and possibly also 'close' if you use pipes/streams and must know when I/O is fully done). [1] - If your goal is “did Node have a failure to start/kill the process (or deliver IPC message)?” listen to 'error'. [1] Typical gotcha (why people expect 'error' but only see 'exit') - A child that runs and exits with code 1 will generally trigger 'exit' with code=1, not 'error'. 'error' typically only appears for failures like “could not be spawned” (e.g., executable not found) or other Node-level delivery problems. [1] Example pattern - Handle both, but with clear intent: - 'error': spawn/kill/IPC delivery problem - 'exit': program ended; inspect code/signal - 'close': you need stdio fully closed (especially if you care about pipe completion) (If you share your specific snippet and how you set stdio/pipes, I can suggest the minimal event wiring for your use-case.)
Citations:
Add error handler to spawned child process.
The current code only listens for 'exit'. If srvx fails to spawn (e.g., executable not found), Node.js emits 'error' instead, leaving no handler and causing the E2E launcher to fail unclearly.
Add an 'error' listener and use 'once' for 'exit' to ensure both spawn failures and process completion are handled explicitly:
Suggested fix
export function start() {
const child = spawn(
'srvx',
['--prod', '-s', resolveDistClientDir(), resolveDistServerEntryPath()],
{
stdio: 'inherit',
shell: process.platform === 'win32',
},
)
+
+ child.once('error', (error) => {
+ console.error(error)
+ process.exit(1)
+ })
- child.on('exit', (code, signal) => {
+ child.once('exit', (code, signal) => {
if (signal) {
process.kill(process.pid, signal)
return
}
process.exit(code ?? 0)
})
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/react-start/rsc-deferred-hydration/server.js` around lines 22 - 39, The
start function that spawns the child process only attaches an 'exit' listener to
child; add a child.once('exit', ...) and also attach child.once('error', err =>
...) so spawn failures are handled explicitly. In the 'error' handler log the
error (e.g., console.error or existing logger) and call process.exit(1); keep
the existing signal handling and process.exit(code ?? 0) behavior in the exit
handler but switch to once to avoid double-processing when an error occurs.
Comment on lines
+4
to
+8
| export function lazyHydratedComponent<TProps>( | ||
| importer: () => Promise<any>, | ||
| exportName: string, | ||
| ): AsyncRouteComponent<TProps> { | ||
| return lazyRouteComponent(importer, exportName) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current usage and scope before applying stricter typing
rg -nP --type=ts --type=tsx 'lazyHydratedComponent|Promise<any>' packages/react-start-client/src/lazyHydratedComponent.tsx
rg -nP --type=ts --type=tsx '\blazyHydratedComponent\s*\(' packagesRepository: TanStack/router
Length of output: 115
🏁 Script executed:
# First, verify the file exists
git ls-files | grep -i lazyHydratedComponent
# Check the file content
cat -n packages/react-start-client/src/lazyHydratedComponent.tsxRepository: TanStack/router
Length of output: 551
🏁 Script executed:
# Search for lazyHydratedComponent usage without type restrictions
rg 'lazyHydratedComponent' packages --type ts
rg 'lazyHydratedComponent' packages --type jsRepository: TanStack/router
Length of output: 6651
🏁 Script executed:
# Find lazyRouteComponent definition
rg 'function lazyRouteComponent|const lazyRouteComponent' --type ts
# Check the import to understand the dependency
head -5 packages/react-start-client/src/lazyHydratedComponent.tsxRepository: TanStack/router
Length of output: 518
🏁 Script executed:
# Read the lazyRouteComponent implementation to understand its signature
cat -n packages/react-router/src/lazyRouteComponent.tsx | head -30Repository: TanStack/router
Length of output: 1295
Replace Promise<any> with stricter typing in the public helper signature.
The current Promise<any> parameter defeats type safety and violates the strict typing guideline. Since lazyRouteComponent expects () => Promise<T> where T extends Record<string, any>, use Record<string, any> to maintain type information for the exported module:
♻️ Proposed fix
export function lazyHydratedComponent<TProps>(
- importer: () => Promise<any>,
+ importer: () => Promise<Record<string, any>>,
exportName: string,
): AsyncRouteComponent<TProps> {
return lazyRouteComponent(importer, exportName)
}As per coding guidelines, **/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function lazyHydratedComponent<TProps>( | |
| importer: () => Promise<any>, | |
| exportName: string, | |
| ): AsyncRouteComponent<TProps> { | |
| return lazyRouteComponent(importer, exportName) | |
| export function lazyHydratedComponent<TProps>( | |
| importer: () => Promise<Record<string, any>>, | |
| exportName: string, | |
| ): AsyncRouteComponent<TProps> { | |
| return lazyRouteComponent(importer, exportName) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react-start-client/src/lazyHydratedComponent.tsx` around lines 4 -
8, The public helper lazyHydratedComponent uses a too-permissive importer type;
change its importer parameter from () => Promise<any> to a stricter shape such
as () => Promise<Record<string, any>> (or a generic TModule extends
Record<string, any>) so it matches lazyRouteComponent's expectation and
preserves module typing; update the lazyHydratedComponent signature (and its
generic bounds if you add TModule) to reference Record<string, any> and keep the
return type AsyncRouteComponent<TProps>.
Sheraff
reviewed
May 7, 2026
Comment on lines
+9
to
+17
| function getIdleScheduler() { | ||
| return globalThis as unknown as { | ||
| requestIdleCallback?: ( | ||
| callback: IdleRequestCallback, | ||
| options?: IdleRequestOptions, | ||
| ) => number | ||
| cancelIdleCallback?: (handle: number) => void | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
pretty minor, but what is the point of this function?
Contributor
Author
There was a problem hiding this comment.
typing. can be inlined
Sheraff
reviewed
May 7, 2026
Comment on lines
+281
to
+286
| onHydrated: (id) => { | ||
| if (typeof globalThis.requestAnimationFrame === 'function') { | ||
| globalThis.requestAnimationFrame(() => replayHydrationEvents(id)) | ||
| } else { | ||
| globalThis.setTimeout(() => replayHydrationEvents(id), 0) | ||
| } |
Contributor
There was a problem hiding this comment.
I understand the progressive enhancement for requestIdleCallback, but isn't requestAnimationFrame supported widely enough that it's not necessary here?
Sheraff
reviewed
May 7, 2026
Comment on lines
+95
to
+108
| function isElementInViewport(element: Element) { | ||
| const rect = element.getBoundingClientRect() | ||
| const viewportHeight = | ||
| window.innerHeight || document.documentElement.clientHeight | ||
| const viewportWidth = | ||
| window.innerWidth || document.documentElement.clientWidth | ||
|
|
||
| return ( | ||
| rect.bottom > 0 && | ||
| rect.right > 0 && | ||
| rect.top < viewportHeight && | ||
| rect.left < viewportWidth | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
why do we need this function if we're using IntersectionObserver? the observer entry should already tell us whether the element is visible, right?
Sheraff
reviewed
May 7, 2026
Comment on lines
+130
to
+136
| if (typeof window.requestAnimationFrame === 'function') { | ||
| visibleFallbackCheckType = 'animation-frame' | ||
| visibleFallbackCheckHandle = window.requestAnimationFrame( | ||
| checkVisibleFallbacks, | ||
| ) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
same here, i'm not sure there is a need for a "no requestAnimationFrame case"
Sheraff
reviewed
May 7, 2026
| function scheduleVisibleFallbackCheck() { | ||
| if ( | ||
| visibleFallbackCheckHandle !== undefined || | ||
| typeof window === 'undefined' |
Contributor
There was a problem hiding this comment.
isn't this already guaranteed to be running on the client? do we still need a window check?
Sheraff
reviewed
May 7, 2026
Comment on lines
+22
to
+30
| function hasIntersectionObserver() { | ||
| return ( | ||
| typeof ( | ||
| globalThis as unknown as { | ||
| IntersectionObserver?: typeof IntersectionObserver | ||
| } | ||
| ).IntersectionObserver === 'function' | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
The scope for IntersectionObserver is very generous
I'm not sure it is worth it to try and support even more browsers, and it seems to add a lot of code and complexity
Sheraff
reviewed
May 7, 2026
| if (disposed) return | ||
| disposed = true | ||
| removeResolveListener() | ||
| cleanups.splice(0).forEach((fn) => fn()) |
Contributor
There was a problem hiding this comment.
why do we need to clone the array here?
Suggested change
| cleanups.splice(0).forEach((fn) => fn()) | |
| cleanups.forEach((fn) => fn()) |
schiller-manuel
force-pushed
the
feat-deferred-hydration
branch
from
876b77b to
0281ed9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts (1)
645-645: ⚡ Quick winDrop the new
as anycasts from these route-tree fixtures.These tests are exercising manifest shape, so
as anyhides exactly the regressions we want TypeScript to catch here. A small alias liketype RouteTreeRoutesArg = Parameters<typeof buildStartManifest>[0]['routeTreeRoutes']plussatisfies RouteTreeRoutesArgwould keep the fixtures strict without adding much noise.As per coding guidelines, "
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety`."Also applies to: 694-694, 755-755, 805-805, 856-856
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts` at line 645, Tests are using liberal "as any" casts on route-tree fixtures (e.g. the object passed as routeTreeRoutes to buildStartManifest) which hides type regressions; remove the "as any" casts and instead create a local alias like type RouteTreeRoutesArg = Parameters<typeof buildStartManifest>[0]['routeTreeRoutes'] and annotate each fixture with "satisfies RouteTreeRoutesArg" so the fixture stays strictly typed (replace instances such as "__root__: { children: ['/about'] } as any" with the same object followed by "satisfies RouteTreeRoutesArg").packages/solid-start-client/src/hydration/idle.ts (1)
14-15: ⚡ Quick winReplace
anywith proper type forcontextparameter.The
context: anytyping bypasses TypeScript's strict mode checking. Use the appropriate type from the core hydration module.Suggested fix
+import type { HydrationRuntimeContext } from '@tanstack/start-client-core/hydration' + - const setup = (context: any) => + const setup = (context: HydrationRuntimeContext) => scheduleIdle(context.prefetch ?? context.gate.resolve, timeout)As per coding guidelines:
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/solid-start-client/src/hydration/idle.ts` around lines 14 - 15, The setup function currently types its parameter as context: any which disables strict checking; change it to the proper hydration context type from the core hydration module (e.g., import and use the HydrationContext or equivalent exported type) and update the signature of setup(context: HydrationContext) so usages of context.prefetch and context.gate.resolve keep their correct types; ensure the import is added from the core hydration module and adjust any downstream references or tests if the name differs (HydrationContext, HydrationState, or similar) so TypeScript strict mode passes.packages/react-start-client/src/hydration/idle.ts (1)
16-17: ⚡ Quick winType the runtime context instead of
any.This drops the compiler check exactly where the strategy branches between
prefetchandgate, so nullability/property mismatches here will be invisible until runtime. Please typesetupwith the actual runtime context instead of erasing the contract.As per coding guidelines,
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-start-client/src/hydration/idle.ts` around lines 16 - 17, The setup function currently uses an untyped parameter ("context: any"); replace that with a proper runtime context type (e.g., an interface used by this module) that models the two branches accessed: a possibly-present prefetch function and a gate object with a resolve method so the compiler checks nullability and property names; update the setup signature from setup(context: any) to setup(context: RuntimeContext) (or an inline type) where RuntimeContext defines prefetch?: () => void and gate: { resolve: () => void } (mark properties optional or required to match actual runtime usage), then keep calling scheduleIdle(context.prefetch ?? context.gate.resolve, timeout) so the TypeScript compiler can validate the selection between context.prefetch and context.gate.resolve.packages/router-utils/tests/stripTypeExports.test.ts (1)
135-138: ⚡ Quick winAlso assert that the stripped export disappears completely.
This still passes if the transform leaves behind
export {}/export {};after removing both type-only specifiers. Adding a negative assertion for the empty export would make this regression visible.Suggested assertion
expect(result).not.toContain('export { type Foo') + expect(result).not.toContain('export {}') + expect(result).not.toContain('export {};') expect(result).toContain('type Foo = string') expect(result).toContain('type Bar = number')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/router-utils/tests/stripTypeExports.test.ts` around lines 135 - 138, Add an assertion to ensure a leftover empty export is removed after stripping type-only specifiers: in stripTypeExports.test.ts, after the existing checks (the expects for 'export { type Foo', 'type Foo = string', 'type Bar = number', and 'export const value = 1'), add a negative assertion that the result does not contain an empty export (e.g. assert result does not match something like "export {}" or "export {};") so that any leftover empty export from the transform is caught.packages/react-start-client/src/GenericHydrate.tsx (1)
224-235: 💤 Low value
dangerouslySetInnerHTMLfor hydration fallback replay.The static analyzer flagged this, but the usage is intentional—
getFallbackHtmlretrieves HTML that was previously captured from the same boundary's server-rendered content viasaveFallbackHtml. This is internal hydration machinery rather than user-controlled input.Consider adding a brief comment documenting this is safe because it replays trusted server-rendered HTML:
+ // Safe: replays server-rendered HTML captured from this same boundary return ( <div style={{ display: 'contents' }} dangerouslySetInnerHTML={{ __html: html }} /> )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-start-client/src/GenericHydrate.tsx` around lines 224 - 235, The analyzer flagged the use of dangerouslySetInnerHTML in HydrationFallback; update the HydrationFallback function to include a brief inline comment above the returned JSX explaining that the HTML comes from getFallbackHtml (which replays trusted server-rendered HTML saved via saveFallbackHtml) and is not user-controlled, so this usage is intentional and safe—reference HydrationFallback, getFallbackHtml, and saveFallbackHtml in the comment to make the trust boundary clear for future reviewers.packages/react-start-client/src/hydration/visible.tsx (1)
119-127: 💤 Low valueAdd a guard before accessing
entries[0].While
IntersectionObservertypically provides at least one entry when observing a single element, adding a defensive check prevents potential runtime errors if the callback fires with an empty array (e.g., during rapid observe/disconnect cycles).🛡️ Suggested defensive check
const setup = (context: any) => { const callback = context.prefetch ?? context.gate.resolve const observer = new IntersectionObserver((entries) => { - if (!entries[0]!.isIntersecting) return + const entry = entries[0] + if (!entry?.isIntersecting) return observer.disconnect() callback() }, observerOptions)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-start-client/src/hydration/visible.tsx` around lines 119 - 127, Add a defensive guard before accessing entries[0] in the IntersectionObserver callback: inside the observer created in this file (the callback assigned to new IntersectionObserver) check that entries.length > 0 (or find an entry with isIntersecting) before reading entries[0].isIntersecting; if the array is empty, simply return. This change affects the observer callback that calls callback = context.prefetch ?? context.gate.resolve and observes context.element with observerOptions.packages/start-plugin-core/src/hydrate-when-transform.ts (1)
829-829: 💤 Low valueHardcoded
'Route'binding removal.The
removeModuleLevelBindings(ast, new Set(['Route']))call removes theRoutebinding unconditionally. If this is intentional for TanStack Router conventions, consider adding a comment explaining why. If it should be configurable, consider making it a parameter.+ // Remove Route binding to avoid including route definition boilerplate in extracted chunks removeModuleLevelBindings(ast, new Set(['Route']))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/src/hydrate-when-transform.ts` at line 829, The call removeModuleLevelBindings(ast, new Set(['Route'])) unconditionally strips the 'Route' module-level binding; either document the TanStack Router convention with an inline comment near that call explaining why 'Route' must be removed, or make it configurable by adding a parameter (e.g., routeBindings or removeBindings) to the containing transform function so callers can supply the set (default to new Set(['Route']) for backward compatibility); update references in hydrate-when-transform.ts where removeModuleLevelBindings is invoked and ensure any tests or callers pass through the new parameter or rely on the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/solid-start/deferred-hydration/tests/hydration.spec.ts`:
- Around line 199-201: The helper htmlContainsText currently interpolates raw
text into a RegExp, which breaks for regex metacharacters; update the function
(htmlContainsText) to first escape regex-special characters in the input text
(e.g., using text.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&')) before splitting
spaces and joining into the pattern variable, then construct the RegExp from
that escaped pattern so assertions are stable and literal characters like +, ?
or () are treated as text.
In `@packages/react-start-client/src/hydration/never.tsx`:
- Around line 33-36: HydrationFallback currently treats both '' and null from
getFallbackHtml as cache-miss because it checks `if (!html)`; change the check
to specifically detect null (`if (html === null)`) so an intentionally empty
server-rendered snapshot (empty string) is preserved instead of rendering
`props.fallback`. Update the return logic in HydrationFallback (and any branches
that rely on truthiness) to use the explicit null check while keeping the use of
getFallbackHtml and the props.id/props.fallback variables unchanged.
In `@packages/start-client-core/src/hydration/condition.ts`:
- Around line 17-20: The current condition() setup only reads the predicate once
via readCondition in the _d and _s handlers so if the predicate returns false at
setup it is never re-evaluated; update the logic so the predicate is
observed/reactive and rechecked when its source changes: replace the plain
synchronous readCondition calls in the _d and _s handlers with a subscription or
reactive watcher that re-invokes readCondition when its dependencies change, and
when readCondition(condition) transitions to true call gate!.resolve() (and
clear the watcher after resolution) — modify the functions referenced (_d, _s,
readCondition, condition, gate.resolve) to add that reactive re-evaluation and
cleanup.
In `@packages/start-client-core/src/hydration/visible.ts`:
- Around line 24-45: getObserver() currently constructs a new
IntersectionObserver unconditionally and will throw in environments without that
API; change it to check typeof IntersectionObserver === 'function' before
instantiating and only set entry.observer when that check passes, otherwise set
entry.observer to null/undefined as a sentinel. Keep the rest of the
VisibleObserverEntry creation (key, elements) the same so existing code (e.g.,
observeVisible()/visible() which already fall back for element === null) can
detect a missing observer and invoke callbacks immediately or handle observation
without an IntersectionObserver; reference getObserver, observerRegistry,
VisibleObserverEntry, and resolveVisibleElement when making this change. Ensure
observerRegistry.set(key, entry) still happens for the fallback path.
---
Nitpick comments:
In `@packages/react-start-client/src/GenericHydrate.tsx`:
- Around line 224-235: The analyzer flagged the use of dangerouslySetInnerHTML
in HydrationFallback; update the HydrationFallback function to include a brief
inline comment above the returned JSX explaining that the HTML comes from
getFallbackHtml (which replays trusted server-rendered HTML saved via
saveFallbackHtml) and is not user-controlled, so this usage is intentional and
safe—reference HydrationFallback, getFallbackHtml, and saveFallbackHtml in the
comment to make the trust boundary clear for future reviewers.
In `@packages/react-start-client/src/hydration/idle.ts`:
- Around line 16-17: The setup function currently uses an untyped parameter
("context: any"); replace that with a proper runtime context type (e.g., an
interface used by this module) that models the two branches accessed: a
possibly-present prefetch function and a gate object with a resolve method so
the compiler checks nullability and property names; update the setup signature
from setup(context: any) to setup(context: RuntimeContext) (or an inline type)
where RuntimeContext defines prefetch?: () => void and gate: { resolve: () =>
void } (mark properties optional or required to match actual runtime usage),
then keep calling scheduleIdle(context.prefetch ?? context.gate.resolve,
timeout) so the TypeScript compiler can validate the selection between
context.prefetch and context.gate.resolve.
In `@packages/react-start-client/src/hydration/visible.tsx`:
- Around line 119-127: Add a defensive guard before accessing entries[0] in the
IntersectionObserver callback: inside the observer created in this file (the
callback assigned to new IntersectionObserver) check that entries.length > 0 (or
find an entry with isIntersecting) before reading entries[0].isIntersecting; if
the array is empty, simply return. This change affects the observer callback
that calls callback = context.prefetch ?? context.gate.resolve and observes
context.element with observerOptions.
In `@packages/router-utils/tests/stripTypeExports.test.ts`:
- Around line 135-138: Add an assertion to ensure a leftover empty export is
removed after stripping type-only specifiers: in stripTypeExports.test.ts, after
the existing checks (the expects for 'export { type Foo', 'type Foo = string',
'type Bar = number', and 'export const value = 1'), add a negative assertion
that the result does not contain an empty export (e.g. assert result does not
match something like "export {}" or "export {};") so that any leftover empty
export from the transform is caught.
In `@packages/solid-start-client/src/hydration/idle.ts`:
- Around line 14-15: The setup function currently types its parameter as
context: any which disables strict checking; change it to the proper hydration
context type from the core hydration module (e.g., import and use the
HydrationContext or equivalent exported type) and update the signature of
setup(context: HydrationContext) so usages of context.prefetch and
context.gate.resolve keep their correct types; ensure the import is added from
the core hydration module and adjust any downstream references or tests if the
name differs (HydrationContext, HydrationState, or similar) so TypeScript strict
mode passes.
In `@packages/start-plugin-core/src/hydrate-when-transform.ts`:
- Line 829: The call removeModuleLevelBindings(ast, new Set(['Route']))
unconditionally strips the 'Route' module-level binding; either document the
TanStack Router convention with an inline comment near that call explaining why
'Route' must be removed, or make it configurable by adding a parameter (e.g.,
routeBindings or removeBindings) to the containing transform function so callers
can supply the set (default to new Set(['Route']) for backward compatibility);
update references in hydrate-when-transform.ts where removeModuleLevelBindings
is invoked and ensure any tests or callers pass through the new parameter or
rely on the default.
In
`@packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts`:
- Line 645: Tests are using liberal "as any" casts on route-tree fixtures (e.g.
the object passed as routeTreeRoutes to buildStartManifest) which hides type
regressions; remove the "as any" casts and instead create a local alias like
type RouteTreeRoutesArg = Parameters<typeof
buildStartManifest>[0]['routeTreeRoutes'] and annotate each fixture with
"satisfies RouteTreeRoutesArg" so the fixture stays strictly typed (replace
instances such as "__root__: { children: ['/about'] } as any" with the same
object followed by "satisfies RouteTreeRoutesArg").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b76c5f42-53db-46c2-815c-aedaa15b54c3
📒 Files selected for processing (68)
benchmarks/bundle-size/scenarios/react-start-deferred-hydration/src/routes/index.tsxbenchmarks/bundle-size/scenarios/solid-start-deferred-hydration/src/routes/index.tsxe2e/react-start/deferred-hydration/package.jsone2e/react-start/deferred-hydration/playwright.config.tse2e/solid-start/deferred-hydration/package.jsone2e/solid-start/deferred-hydration/playwright.config.tse2e/solid-start/deferred-hydration/tests/hydration.spec.tsexamples/react/start-large/src/routeTree.gen.tspackages/react-start-client/src/GenericHydrate.tsxpackages/react-start-client/src/Hydrate.tsxpackages/react-start-client/src/hydration.tspackages/react-start-client/src/hydration/generic.tspackages/react-start-client/src/hydration/idle.tspackages/react-start-client/src/hydration/load.tsxpackages/react-start-client/src/hydration/never.tsxpackages/react-start-client/src/hydration/visible.tsxpackages/react-start-client/src/index.tsxpackages/react-start-client/src/tests/Hydrate.test.tsxpackages/react-start/src/index.tspackages/router-utils/src/compiler-helpers.tspackages/router-utils/src/index.tspackages/router-utils/tests/compiler-helpers.test.tspackages/router-utils/tests/stripTypeExports.test.tspackages/solid-start-client/src/GenericHydrate.tsxpackages/solid-start-client/src/Hydrate.tsxpackages/solid-start-client/src/hydration.tspackages/solid-start-client/src/hydration/generic.tspackages/solid-start-client/src/hydration/idle.tspackages/solid-start-client/src/hydration/load.tsxpackages/solid-start-client/src/hydration/never.tspackages/solid-start-client/src/hydration/visible.tsxpackages/solid-start-client/src/index.tsxpackages/solid-start-client/vite.config.tspackages/solid-start/src/index.tspackages/start-client-core/src/hydration.tspackages/start-client-core/src/hydration/condition.tspackages/start-client-core/src/hydration/idle.tspackages/start-client-core/src/hydration/interaction.tspackages/start-client-core/src/hydration/load.tspackages/start-client-core/src/hydration/media.tspackages/start-client-core/src/hydration/never.tspackages/start-client-core/src/hydration/runtime.tspackages/start-client-core/src/hydration/types.tspackages/start-client-core/src/hydration/visible.tspackages/start-plugin-core/src/hydrate-when-transform.tspackages/start-plugin-core/src/start-compiler/compiler.tspackages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.tspackages/start-plugin-core/src/vite/start-compiler-plugin/plugin.tspackages/start-plugin-core/tests/compiler.test.tspackages/start-plugin-core/tests/hydrate-when-transform.test.tspackages/start-plugin-core/tests/hydrateWhen/hydrateWhen.test.tspackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenBasic.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenMultiple.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNested.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNever.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenObjectFallback.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenRenamed.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenBasic.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenMultiple.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenNested.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenNever.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenObjectFallback.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenRenamed.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.H0.client.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.H0.tsxpackages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.H2.tsxpackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.tspackages/start-plugin-core/tests/vite-start-compiler-plugin.test.ts
✅ Files skipped from review due to trivial changes (11)
- packages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.H2.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.H0.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/virtual/hydrateWhenNested.tsx.H0.client.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenMultiple.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenRenamed.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenMultiple.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenObjectFallback.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenObjectFallback.tsx
- packages/start-plugin-core/tests/hydrateWhen/snapshots/client/hydrateWhenNested.tsx
- e2e/solid-start/deferred-hydration/package.json
- packages/start-plugin-core/tests/hydrateWhen/snapshots/server/hydrateWhenNever.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/start-client-core/src/hydration/media.ts
- packages/react-start-client/src/hydration.ts
- packages/solid-start-client/src/hydration/never.ts
- packages/react-start-client/src/index.tsx
- packages/start-client-core/src/hydration/never.ts
- packages/start-plugin-core/tests/vite-start-compiler-plugin.test.ts
- e2e/react-start/deferred-hydration/package.json
- packages/start-client-core/src/hydration/load.ts
- packages/start-client-core/src/hydration/types.ts
- packages/start-client-core/src/hydration.ts
- packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts
- packages/react-start-client/src/tests/Hydrate.test.tsx
- packages/start-client-core/src/hydration/runtime.ts
- packages/solid-start-client/src/GenericHydrate.tsx
Comment on lines
+199
to
+201
| function htmlContainsText(html: string, text: string) { | ||
| const pattern = text.split(' ').join('(?:\\s|<!-- -->)+') | ||
| expect(html).toMatch(new RegExp(pattern)) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n e2e/solid-start/deferred-hydration/tests/hydration.spec.ts | sed -n '195,210p'Repository: TanStack/router
Length of output: 640
🏁 Script executed:
rg "htmlContainsText" e2e/solid-start/deferred-hydration/tests/hydration.spec.ts -A 1 -B 1Repository: TanStack/router
Length of output: 503
Escape text before constructing RegExp in htmlContainsText.
text is interpolated directly as regex source; metacharacters (e.g. +, (, ?) will change match behavior and make assertions fragile.
Proposed fix
function htmlContainsText(html: string, text: string) {
- const pattern = text.split(' ').join('(?:\\s|<!-- -->)+')
+ const escaped = text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+ const pattern = escaped.split(' ').join('(?:\\s|<!-- -->)+')
expect(html).toMatch(new RegExp(pattern))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function htmlContainsText(html: string, text: string) { | |
| const pattern = text.split(' ').join('(?:\\s|<!-- -->)+') | |
| expect(html).toMatch(new RegExp(pattern)) | |
| function htmlContainsText(html: string, text: string) { | |
| const escaped = text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') | |
| const pattern = escaped.split(' ').join('(?:\\s|<!-- -->)+') | |
| expect(html).toMatch(new RegExp(pattern)) | |
| } |
🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 200-200: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/solid-start/deferred-hydration/tests/hydration.spec.ts` around lines 199
- 201, The helper htmlContainsText currently interpolates raw text into a
RegExp, which breaks for regex metacharacters; update the function
(htmlContainsText) to first escape regex-special characters in the input text
(e.g., using text.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&')) before splitting
spaces and joining into the pattern variable, then construct the RegExp from
that escaped pattern so assertions are stable and literal characters like +, ?
or () are treated as text.
Comment on lines
+33
to
+36
| function HydrationFallback(props: { id: string; fallback: React.ReactNode }) { | ||
| const html = getFallbackHtml(props.id) | ||
|
|
||
| if (!html) return props.fallback as React.JSX.Element | null |
Contributor
There was a problem hiding this comment.
Treat cached empty HTML as preserved content, not a cache miss.
if (!html) collapses '' and null together. If the server-rendered boundary is intentionally empty, this path will incorrectly render props.fallback on the client instead of preserving the empty snapshot.
Small fix
- if (!html) return props.fallback as React.JSX.Element | null
+ if (html == null) return props.fallback as React.JSX.Element | null🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react-start-client/src/hydration/never.tsx` around lines 33 - 36,
HydrationFallback currently treats both '' and null from getFallbackHtml as
cache-miss because it checks `if (!html)`; change the check to specifically
detect null (`if (html === null)`) so an intentionally empty server-rendered
snapshot (empty string) is preserved instead of rendering `props.fallback`.
Update the return logic in HydrationFallback (and any branches that rely on
truthiness) to use the explicit null check while keeping the use of
getFallbackHtml and the props.id/props.fallback variables unchanged.
Comment on lines
+17
to
+20
| _d: () => !readCondition(condition), | ||
| _s: ({ gate }) => { | ||
| if (readCondition(condition)) gate!.resolve() | ||
| }, |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
condition() currently acts like “immediate or never,” not deferred hydration.
Lines 17-19 only do a synchronous read. If the predicate is false at setup time, nothing ever re-checks it, so condition(() => someMutableFlag) will never hydrate after the flag flips. For a public deferred-hydration strategy, that’s a pretty sharp edge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/start-client-core/src/hydration/condition.ts` around lines 17 - 20,
The current condition() setup only reads the predicate once via readCondition in
the _d and _s handlers so if the predicate returns false at setup it is never
re-evaluated; update the logic so the predicate is observed/reactive and
rechecked when its source changes: replace the plain synchronous readCondition
calls in the _d and _s handlers with a subscription or reactive watcher that
re-invokes readCondition when its dependencies change, and when
readCondition(condition) transitions to true call gate!.resolve() (and clear the
watcher after resolution) — modify the functions referenced (_d, _s,
readCondition, condition, gate.resolve) to add that reactive re-evaluation and
cleanup.
Comment on lines
+24
to
+45
| function getObserver(rootMargin: string, threshold: number | Array<number>) { | ||
| const key = getVisibleKey(rootMargin, threshold) | ||
| const existing = observerRegistry.get(key) | ||
| if (existing) return existing | ||
|
|
||
| const entry: VisibleObserverEntry = { | ||
| key, | ||
| elements: new Map<Element, Set<() => void>>(), | ||
| observer: new IntersectionObserver( | ||
| (entries) => { | ||
| for (const observerEntry of entries) { | ||
| if (!observerEntry.isIntersecting) continue | ||
| resolveVisibleElement(entry, observerEntry.target) | ||
| } | ||
| }, | ||
| { rootMargin, threshold }, | ||
| ), | ||
| } | ||
|
|
||
| observerRegistry.set(key, entry) | ||
| return entry | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI 'package.json|.browserslistrc|README.md|setupTests.ts|setupTests.tsx' .
rg -n --hidden -S 'IntersectionObserver|polyfill|browserslist|not dead|last [0-9]+ (chrome|firefox|safari|edge)' .Repository: TanStack/router
Length of output: 35426
🏁 Script executed:
cat -n packages/start-client-core/src/hydration/visible.tsRepository: TanStack/router
Length of output: 3990
🏁 Script executed:
# Check the type definition of VisibleObserverEntry and how it's used
head -20 packages/start-client-core/src/hydration/visible.ts | tail -10
# Compare with how react-router handles this
grep -A 10 "typeof IntersectionObserver" packages/react-router/src/utils.ts | head -15Repository: TanStack/router
Length of output: 627
🏁 Script executed:
# Get a clearer view of how react-router guards IntersectionObserver
sed -n '83,110p' packages/react-router/src/utils.tsRepository: TanStack/router
Length of output: 771
🏁 Script executed:
# Check how solid-router and vue-router handle the optional IntersectionObserver
grep -B 5 -A 15 "typeof IntersectionObserver" packages/solid-router/src/utils.tsRepository: TanStack/router
Length of output: 734
Add a runtime fallback when IntersectionObserver is missing.
The getObserver() function instantiates IntersectionObserver unconditionally, causing visible() to hard-crash in environments without that API or polyfills. Other packages in this repo (react-router, solid-router, vue-router) already guard against this with typeof IntersectionObserver === 'function' checks. Since observeVisible() already falls back to immediate callback execution for element === null, extend that pattern to handle missing IntersectionObserver as well.
Proposed fix
function getObserver(rootMargin: string, threshold: number | Array<number>) {
+ if (typeof IntersectionObserver === 'undefined') {
+ return null
+ }
+
const key = getVisibleKey(rootMargin, threshold)
const existing = observerRegistry.get(key)
if (existing) return existing
@@
function observeVisible(
element: Element | null,
callback: () => void,
rootMargin: string,
threshold: number | Array<number>,
) {
if (!element) {
callback()
return
}
const observerEntry = getObserver(rootMargin, threshold)
+ if (!observerEntry) {
+ callback()
+ return
+ }
+
let callbacks = observerEntry.elements.get(element)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/start-client-core/src/hydration/visible.ts` around lines 24 - 45,
getObserver() currently constructs a new IntersectionObserver unconditionally
and will throw in environments without that API; change it to check typeof
IntersectionObserver === 'function' before instantiating and only set
entry.observer when that check passes, otherwise set entry.observer to
null/undefined as a sentinel. Keep the rest of the VisibleObserverEntry creation
(key, elements) the same so existing code (e.g., observeVisible()/visible()
which already fall back for element === null) can detect a missing observer and
invoke callbacks immediately or handle observation without an
IntersectionObserver; reference getObserver, observerRegistry,
VisibleObserverEntry, and resolveVisibleElement when making this change. Ensure
observerRegistry.set(key, entry) still happens for the fallback path.
8567b14 to
13f5e82
Compare
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We renamed the local exports parameter and variable in compiler-helpers.ts to moduleExports to fix the 7 import/no-commonjs ESLint errors. The rule flags any write to an identifier named exports as a CommonJS module export assignment, even when it is a locally-scoped Map. The returned object still exposes exports: moduleExports to satisfy the ExtractedModuleInfo interface shape.
Tip
✅ We verified this fix by re-running @tanstack/router-utils:test:eslint.
Suggested Fix changes
diff --git a/packages/router-utils/src/compiler-helpers.ts b/packages/router-utils/src/compiler-helpers.ts
index 8f4a07b5..fd165872 100644
--- a/packages/router-utils/src/compiler-helpers.ts
+++ b/packages/router-utils/src/compiler-helpers.ts
@@ -30,7 +30,7 @@ function getModuleExportName(node: t.Identifier | t.StringLiteral) {
function addVariableDeclarationModuleInfo(
declaration: t.VariableDeclaration,
bindings: Map<string, ModuleInfoBinding>,
- exports?: Map<string, string>,
+ moduleExports?: Map<string, string>,
) {
for (const declarator of declaration.declarations) {
for (const name of collectIdentifiersFromPattern(declarator.id)) {
@@ -38,7 +38,7 @@ function addVariableDeclarationModuleInfo(
type: 'var',
init: declarator.init ?? null,
})
- exports?.set(name, name)
+ moduleExports?.set(name, name)
}
}
}
@@ -46,10 +46,10 @@ function addVariableDeclarationModuleInfo(
function addDeclarationModuleInfo(
declaration: t.Declaration,
bindings: Map<string, ModuleInfoBinding>,
- exports?: Map<string, string>,
+ moduleExports?: Map<string, string>,
) {
if (t.isVariableDeclaration(declaration)) {
- addVariableDeclarationModuleInfo(declaration, bindings, exports)
+ addVariableDeclarationModuleInfo(declaration, bindings, moduleExports)
return
}
@@ -62,7 +62,7 @@ function addDeclarationModuleInfo(
type: 'var',
init: null,
})
- exports?.set(declaration.id.name, declaration.id.name)
+ moduleExports?.set(declaration.id.name, declaration.id.name)
}
}
@@ -402,7 +402,7 @@ export function collectLocalBindingsFromStatement(
export function extractModuleInfoFromAst(ast: t.File): ExtractedModuleInfo {
const bindings = new Map<string, ModuleInfoBinding>()
- const exports = new Map<string, string>()
+ const moduleExports = new Map<string, string>()
const reExportAllSources: Array<string> = []
for (const node of ast.program.body) {
@@ -444,13 +444,13 @@ export function extractModuleInfoFromAst(ast: t.File): ExtractedModuleInfo {
if (t.isExportNamedDeclaration(node)) {
if (node.declaration) {
- addDeclarationModuleInfo(node.declaration, bindings, exports)
+ addDeclarationModuleInfo(node.declaration, bindings, moduleExports)
}
for (const specifier of node.specifiers) {
if (t.isExportNamespaceSpecifier(specifier)) {
const exported = getModuleExportName(specifier.exported)
- exports.set(exported, exported)
+ moduleExports.set(exported, exported)
if (node.source) {
bindings.set(exported, {
type: 'import',
@@ -461,7 +461,7 @@ export function extractModuleInfoFromAst(ast: t.File): ExtractedModuleInfo {
} else if (t.isExportSpecifier(specifier)) {
const local = getModuleExportName(specifier.local)
const exported = getModuleExportName(specifier.exported)
- exports.set(exported, local)
+ moduleExports.set(exported, local)
if (node.source) {
bindings.set(local, {
@@ -478,7 +478,7 @@ export function extractModuleInfoFromAst(ast: t.File): ExtractedModuleInfo {
if (t.isExportDefaultDeclaration(node)) {
const declaration = node.declaration
if (t.isIdentifier(declaration)) {
- exports.set('default', declaration.name)
+ moduleExports.set('default', declaration.name)
} else if (
(t.isFunctionDeclaration(declaration) ||
t.isClassDeclaration(declaration)) &&
@@ -488,14 +488,14 @@ export function extractModuleInfoFromAst(ast: t.File): ExtractedModuleInfo {
type: 'var',
init: null,
})
- exports.set('default', declaration.id.name)
+ moduleExports.set('default', declaration.id.name)
} else {
const synth = '__default_export__'
bindings.set(synth, {
type: 'var',
init: t.isExpression(declaration) ? declaration : null,
})
- exports.set('default', synth)
+ moduleExports.set('default', synth)
}
continue
}
@@ -507,7 +507,7 @@ export function extractModuleInfoFromAst(ast: t.File): ExtractedModuleInfo {
return {
bindings,
- exports,
+ exports: moduleExports,
reExportAllSources,
}
}
Or Apply changes locally with:
npx nx-cloud apply-locally S062-2Zod
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/e2e-utils/src/hmrFileEditor.ts`:
- Around line 30-43: captureOriginals currently overwrites the true
pre-normalization file with the normalized version by assigning
originalContents[key] = normalized; preserve the raw file contents by recording
current into a new map (e.g., rawOriginals[key] = current) before calling
normalizeSource, keep originalContents[key] = normalized for between-test
resets, and ensure teardown/restore logic (the code that uses originalContents
and pendingRestoreKeys) is updated to prefer rawOriginals when restoring the
final state so local edits are returned to their original raw form.
- Around line 46-94: The public helpers replaceText, rewriteFile, and
restoreFiles currently assume callers awaited capturePromise; make capture
ordering internal by awaiting the existing capturePromise at the start of each
of these functions (use the capturePromise variable) so the original contents
are guaranteed captured before any read/write operations; keep existing behavior
otherwise (preserve parameters and error handling) and ensure awaiting
capturePromise is done before calling readFile/writeFile or accessing
originalContents.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07e7ad50-729d-44e6-81c9-4f3da8be2b28
📒 Files selected for processing (13)
e2e/e2e-utils/src/hmrFileEditor.tse2e/e2e-utils/src/index.tse2e/react-start/deferred-hydration/package.jsone2e/react-start/deferred-hydration/playwright.config.tse2e/react-start/deferred-hydration/tests/hydration.spec.tse2e/react-start/deferred-hydration/tests/setup/global.setup.tse2e/react-start/deferred-hydration/tests/setup/global.teardown.tse2e/react-start/hmr/tests/app.spec.tspackages/start-client-core/src/client/hydrateStart.tspackages/start-client-core/vite.config.tspackages/start-plugin-core/src/hydrate-when-transform.tspackages/start-plugin-core/src/vite/start-compiler-plugin/plugin.tspackages/start-plugin-core/tests/vite-start-compiler-plugin.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/react-start/deferred-hydration/package.json
Comment on lines
+30
to
+43
| async function captureOriginals() { | ||
| for (const [key, filePath] of Object.entries(files) as Array< | ||
| [TFileKey, string] | ||
| >) { | ||
| const current = await readFile(filePath, 'utf8') | ||
| const normalized = normalizeSource(key, current) | ||
|
|
||
| if (normalized !== current) { | ||
| await writeFile(filePath, normalized) | ||
| pendingRestoreKeys.add(key) | ||
| } | ||
|
|
||
| originalContents[key] = normalized | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve the pre-normalization file contents for final cleanup.
originalContents[key] = normalized means the editor can only restore the normalized baseline, not the actual file content that existed when the suite started. If someone runs these tests with local edits in one of the fixture files, those edits get overwritten and never come back. Keep a separate raw snapshot for teardown, and use the normalized snapshot only for between-test resets.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/e2e-utils/src/hmrFileEditor.ts` around lines 30 - 43, captureOriginals
currently overwrites the true pre-normalization file with the normalized version
by assigning originalContents[key] = normalized; preserve the raw file contents
by recording current into a new map (e.g., rawOriginals[key] = current) before
calling normalizeSource, keep originalContents[key] = normalized for
between-test resets, and ensure teardown/restore logic (the code that uses
originalContents and pendingRestoreKeys) is updated to prefer rawOriginals when
restoring the final state so local edits are returned to their original raw
form.
Comment on lines
+46
to
+94
| const capturePromise = captureOriginals() | ||
|
|
||
| async function restoreFiles(forceFileKeys: Iterable<TFileKey> = []) { | ||
| const forceRestoreKeys = new Set(forceFileKeys) | ||
| const restoredFileKeys: Array<TFileKey> = [] | ||
|
|
||
| for (const [key, filePath] of Object.entries(files) as Array< | ||
| [TFileKey, string] | ||
| >) { | ||
| const content = originalContents[key] | ||
| if (content === undefined) continue | ||
|
|
||
| const current = await readFile(filePath, 'utf8') | ||
|
|
||
| if (current !== content || forceRestoreKeys.has(key)) { | ||
| await writeFile(filePath, content) | ||
| restoredFileKeys.push(key) | ||
| } | ||
| } | ||
|
|
||
| return restoredFileKeys | ||
| } | ||
|
|
||
| async function replaceText(fileKey: TFileKey, from: string, to: string) { | ||
| const filePath = files[fileKey] | ||
| const source = await readFile(filePath, 'utf8') | ||
|
|
||
| if (!source.includes(from)) { | ||
| throw new Error(`Expected file to include ${JSON.stringify(from)}`) | ||
| } | ||
|
|
||
| await writeFile(filePath, source.replace(from, to)) | ||
| } | ||
|
|
||
| async function rewriteFile( | ||
| fileKey: TFileKey, | ||
| updater: (source: string) => string, | ||
| options: { allowNoop?: boolean } = {}, | ||
| ) { | ||
| const filePath = files[fileKey] | ||
| const source = await readFile(filePath, 'utf8') | ||
| const updated = updater(source) | ||
|
|
||
| if (updated === source && !options.allowNoop) { | ||
| throw new Error(`Expected ${filePath} to change during rewrite`) | ||
| } | ||
|
|
||
| await writeFile(filePath, updated) | ||
| } |
Contributor
There was a problem hiding this comment.
Make capture ordering an internal guarantee.
Right now every caller has to remember to await capturePromise before calling replaceText, rewriteFile, or restoreFiles. If a caller forgets, the first edit can be captured as the “original” and subsequent restores silently stop working. Since this is now a shared utility, it’s safer to await capturePromise inside the public methods.
Suggested hardening
const capturePromise = captureOriginals()
async function restoreFiles(forceFileKeys: Iterable<TFileKey> = []) {
+ await capturePromise
const forceRestoreKeys = new Set(forceFileKeys)
const restoredFileKeys: Array<TFileKey> = []
for (const [key, filePath] of Object.entries(files) as Array<
[TFileKey, string]
>) {
@@
async function replaceText(fileKey: TFileKey, from: string, to: string) {
+ await capturePromise
const filePath = files[fileKey]
const source = await readFile(filePath, 'utf8')
@@
async function rewriteFile(
fileKey: TFileKey,
updater: (source: string) => string,
options: { allowNoop?: boolean } = {},
) {
+ await capturePromise
const filePath = files[fileKey]
const source = await readFile(filePath, 'utf8')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/e2e-utils/src/hmrFileEditor.ts` around lines 46 - 94, The public helpers
replaceText, rewriteFile, and restoreFiles currently assume callers awaited
capturePromise; make capture ordering internal by awaiting the existing
capturePromise at the start of each of these functions (use the capturePromise
variable) so the original contents are guaranteed captured before any read/write
operations; keep existing behavior otherwise (preserve parameters and error
handling) and ensure awaiting capturePromise is done before calling
readFile/writeFile or accessing originalContents.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests / E2E
Chores