Skip to content

Conversation

@schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Nov 29, 2025

fixes #5992

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced error handling and graceful recovery in server-side rendering streams
    • Improved state management during asynchronous rendering to prevent data loss
    • Fixed resource cleanup and finalization on error paths
    • Better buffer and data management across streaming chunks
  • Refactoring

    • Optimized internal stream resource handling for improved reliability

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 29, 2025

Walkthrough

Single-file update to SSR stream handling that guards post-destruction writes to injected HTML, consolidates reader management with proper cleanup, and refines state handling for deferred queries to prevent duplicate dehydration during React flushes.

Changes

Cohort / File(s) Change Summary
SSR Stream Lifecycle & Injection Handling
packages/router-core/src/ssr/transformStreamWithRouter.ts
Added guards preventing writes to final PassThrough after stream destruction; consolidated duplicate reader creation into single constant with finally-block cleanup; relaxed boolean type assertions; expanded error/end-path cleanup (rendering finished mark, timeout/buffer clear, pass-through destroy); preserved and reused leftoverHtml state across flushes; reset RegExp lastIndex for correct chunk re-scanning; ensured early exit on destroyed stream to prevent duplicate dehydration.

Sequence Diagram(s)

Not applicable for this change.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Stream lifecycle state management and the guards preventing post-destruction writes
  • Interaction between injected HTML promise resolution and cleanup paths (error, end, normal flow)
  • Reader/lock acquisition and release semantics with finally block
  • State reuse logic for leftoverHtml and its interaction with multiple React flushes
  • RegExp lastIndex reset and its correctness across chunk boundaries

Possibly related PRs

Suggested labels

package: router-core

Suggested reviewers

  • brenelz

Poem

🐰 A stream once dehydrated, now dehydrated thrice,
But guards and cleanup made things oh-so-nice!
One reader reigns, state flows just right,
No duplicates dancing in the HTML night!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'fix: fix streaming' is overly vague and does not specify what streaming issue is being fixed, making it unclear without checking the PR description. Use a more specific title that describes the core problem, e.g., 'fix: prevent duplicate query dehydration during streaming with React flushes' or 'fix: dehydrate queries only once in streaming mode'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR addresses the root cause of #5992 by implementing comprehensive streaming fixes including proper cleanup, state management, and error handling to prevent duplicate query dehydrations during React flushes.
Out of Scope Changes check ✅ Passed All changes in transformStreamWithRouter.ts are directly scoped to fixing streaming and query dehydration issues as specified in #5992, with no unrelated modifications detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-5992

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af02265 and 20947e5.

📒 Files selected for processing (1)
  • packages/router-core/src/ssr/transformStreamWithRouter.ts (8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety throughout the codebase

Files:

  • packages/router-core/src/ssr/transformStreamWithRouter.ts
🧠 Learnings (3)
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Applies to packages/solid-router/**/*.{ts,tsx} : Solid Router components and primitives should use the tanstack/solid-router package

Applied to files:

  • packages/router-core/src/ssr/transformStreamWithRouter.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

  • packages/router-core/src/ssr/transformStreamWithRouter.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.

Applied to files:

  • packages/router-core/src/ssr/transformStreamWithRouter.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test
  • GitHub Check: Preview
🔇 Additional comments (10)
packages/router-core/src/ssr/transformStreamWithRouter.ts (10)

67-70: LGTM!

Setting destroyed = true before calling controller.error() ensures that any error handlers or callbacks triggered will correctly see the destroyed state, consistent with the pattern used in the end method.


85-96: LGTM!

Consolidating reader creation and adding finally block to release the lock is a proper resource management pattern. This ensures the reader lock is released in all code paths (success, error, or exception), preventing potential stream locking issues.


118-123: LGTM!

Type inference handles these boolean literals correctly; explicit casts were unnecessary.


151-182: LGTM!

The destroyed guards correctly prevent:

  1. Writing to a destroyed/cancelled stream (lines 157-160)
  2. Rejecting the promise after the stream is already torn down (lines 169-171)

This is essential for clean shutdown semantics and prevents errors when the client disconnects mid-stream.


184-199: LGTM!

The buffer ordering (leftover + leftoverHtml + getBufferedRouterStream() + pendingClosingTags) correctly assembles remaining content, and clearing the buffers afterward ensures they're consumed exactly once—directly addressing the duplication issue.


235-243: LGTM!

Including leftoverHtml in the flush and clearing it afterward ensures all buffered content is written exactly once when closing tags are encountered.


246-252: Critical correctness fix for global regex state.

Since patternClosingTag has the /g flag and is module-scoped, its lastIndex persists across exec() calls and could carry stale state from previous chunks or even previous requests. Resetting to 0 ensures consistent matching from the start of each chunk.


254-266: LGTM!

Clearing leftoverHtml after it's included in the processed output (line 262) prevents duplicate inclusion in subsequent chunks.


268-288: LGTM!

The early return guard prevents processing the end sequence on an already-destroyed stream, avoiding unnecessary state mutations and potential errors.


290-303: LGTM!

Comprehensive error handling that:

  1. Updates rendering state for router consistency
  2. Clears the timeout to prevent late firing
  3. Clears all string buffers to prevent memory leaks
  4. Properly destroys the stream and rejects the promise

This ensures clean teardown and consistent state on errors.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Nov 29, 2025

View your CI Pipeline Execution ↗ for commit 20947e5

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 12m 31s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 24s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-29 12:40:41 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 29, 2025

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5994

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@5994

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@5994

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5994

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@5994

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@5994

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@5994

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@5994

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@5994

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@5994

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@5994

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@5994

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@5994

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@5994

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@5994

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@5994

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@5994

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@5994

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5994

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@5994

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@5994

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@5994

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@5994

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@5994

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@5994

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@5994

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@5994

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@5994

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@5994

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@5994

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@5994

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@5994

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5994

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@5994

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@5994

commit: 20947e5

@schiller-manuel schiller-manuel merged commit 95d88a2 into main Nov 29, 2025
6 checks passed
@schiller-manuel schiller-manuel deleted the fix-5992 branch November 29, 2025 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Same query dehydrated many times when React flushes

2 participants