refactor: streaming perf #6496
refactor: streaming perf #6496
Conversation
📝 WalkthroughWalkthroughAdds e2e assertions for TSR markers and a stream barrier; introduces production-aware manifest filtering with caching and script-wrapping changes in ssr-server; and massively refactors transformStreamWithRouter with a fast-path, buffered injection, barrier detection, robust cleanup, and lifetime/error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant AppStream as App Stream
participant Transform as transformStreamWithRouter
participant ServerSSR as SSR Server
participant Consumer as Downstream
alt Serialization finished & no buffered HTML (fast path)
AppStream->>Transform: read chunks
Transform->>Consumer: passthrough bytes
Transform->>ServerSSR: minimal cleanup
else Serialization in progress (buffered path)
AppStream->>Transform: stream chunks
Transform->>Transform: decode & accumulate leftover
Transform->>ServerSSR: onInjectedHtml / onSerializationFinished events
Transform->>Transform: detect $tsr-stream-barrier and closing tags
Transform->>Consumer: flush buffered HTML + injected scripts
Transform->>ServerSSR: flush scripts synchronously
Transform->>Transform: perform idempotent cleanup
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
View your CI Pipeline Execution ↗ for commit 484be5e
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| ;(async () => { | ||
| const reader = appStream.getReader() | ||
| try { | ||
| while (true) { | ||
| const { done, value } = await reader.read() | ||
| if (done) break | ||
| if (cleanedUp || isStreamClosed) return | ||
| controller?.enqueue(value as unknown as Uint8Array) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the file structure and line count
wc -l packages/router-core/src/ssr/transformStreamWithRouter.tsRepository: TanStack/router
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Read the relevant section of the file to understand context
sed -n '170,200p' packages/router-core/src/ssr/transformStreamWithRouter.tsRepository: TanStack/router
Length of output: 841
🏁 Script executed:
#!/bin/bash
# Find all call sites for transformStreamWithRouter and related functions
rg -n --type=ts --type=tsx 'transformStreamWithRouter|transformReadableStreamWithRouter|transformPipeableStreamWithRouter' --max-count=50Repository: TanStack/router
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Find all call sites - try without tsx type specification
rg -n 'transformStreamWithRouter|transformReadableStreamWithRouter|transformPipeableStreamWithRouter' --max-count=50Repository: TanStack/router
Length of output: 2469
🏁 Script executed:
#!/bin/bash
# Look at the beginning of the file to understand imports and the function signature
head -80 packages/router-core/src/ssr/transformStreamWithRouter.tsRepository: TanStack/router
Length of output: 2455
🏁 Script executed:
#!/bin/bash
# Check where appStream comes from - look for the function definition
sed -n '1,120p' packages/router-core/src/ssr/transformStreamWithRouter.ts | tail -100Repository: TanStack/router
Length of output: 3260
🏁 Script executed:
#!/bin/bash
# Continue reading the transformStreamWithRouter function, especially around the fast path
sed -n '100,250p' packages/router-core/src/ssr/transformStreamWithRouter.tsRepository: TanStack/router
Length of output: 4515
🏁 Script executed:
#!/antml:bin/bash
# Check the call sites to understand what stream types are being passed
grep -A5 'transformReadableStreamWithRouter\|transformPipeableStreamWithRouter' packages/react-router/src/ssr/renderRouterToStream.tsx | head -40Repository: TanStack/router
Length of output: 739
🏁 Script executed:
#!/bin/bash
# Check what ReadableStream type is being used and its chunk types
grep -B5 -A10 'appStream\.getReader\|appStream\.read' packages/router-core/src/ssr/transformStreamWithRouter.tsRepository: TanStack/router
Length of output: 819
🏁 Script executed:
#!/bin/bash
# Search for tests related to transformStreamWithRouter to see if non-Uint8Array types are tested
rg -n 'transformStreamWithRouter|getReader|\.read\(' --type=ts packages/router-core/src/ssr -A3 | head -80Repository: TanStack/router
Length of output: 4794
🏁 Script executed:
#!/bin/bash
# Check if there are any test files for this module
find . -name '*transform*stream*' -o -name '*ssr*test*' | grep -i testRepository: TanStack/router
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Look for how renderToReadableStream or renderToPipeableStream work - these feed the appStream
rg -n 'renderToReadableStream|renderToPipeableStream|toWeb\(' packages/react-router/src/ssr -B2 -A2 | head -60Repository: TanStack/router
Length of output: 2940
🏁 Script executed:
#!/bin/bash
# Check the ReadableStream generic type in the slow path more carefully
sed -n '245,400p' packages/router-core/src/ssr/transformStreamWithRouter.ts | grep -A10 "const stream = new ReadableStream"Repository: TanStack/router
Length of output: 249
🏁 Script executed:
#!/antml:bin/bash
# Search for RawStream usage to see if it handles different chunk types
sed -n '190,220p' packages/router-core/src/ssr/serializer/RawStream.tsRepository: TanStack/router
Length of output: 933
🏁 Script executed:
#!/bin/bash
# Check the slow path implementation to see exactly how it handles value type
sed -n '384,410p' packages/router-core/src/ssr/transformStreamWithRouter.tsRepository: TanStack/router
Length of output: 919
🏁 Script executed:
#!/antml:bin/bash
# Verify the ReadableStream input type annotation - no generic parameter?
sed -n '100,110p' packages/router-core/src/ssr/transformStreamWithRouter.tsRepository: TanStack/router
Length of output: 475
Validate fast-path chunk types to match output stream contract.
The fast-path passthrough enqueues chunks with an unsafe as unknown as Uint8Array cast, while the slow path explicitly guards with instanceof Uint8Array. Since the output stream is declared as ReadableStream<Uint8Array>, the fast path should validate incoming chunk types to ensure type safety consistency.
🔧 Proposed fix
- controller?.enqueue(value as unknown as Uint8Array)
+ if (typeof value === 'string') {
+ controller?.enqueue(textEncoder.encode(value))
+ } else if (value instanceof Uint8Array) {
+ controller?.enqueue(value)
+ } else if (value instanceof ArrayBuffer) {
+ controller?.enqueue(new Uint8Array(value))
+ } else {
+ controller?.enqueue(textEncoder.encode(String(value)))
+ }
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.