fix: unset extends of serialization adapters after deduping
#5316
Conversation
WalkthroughAdds UI and route files to surface nested serialized data from SSR and a new server-function route; introduces a RenderNestedData component and tests; renames a parameter in dedupeSerializationAdapters; and stops propagating nested serialization adapters during plugin construction in the router-core serializer. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser / E2E test
participant R as RouteComponent (client)
participant S as Server (createServerFn)
participant U as RenderNestedData (UI)
B->>R: Navigate to /server-function/nested
R->>B: Render button + "waiting" UI
B->>R: Click "call server function"
R->>S: Invoke server fn -> makeNested()
S-->>R: Return nested data (serialized)
R->>U: Pass nested prop to RenderNestedData
U-->>B: Render expected vs actual shout/whisper states
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/{*-start,start-*}/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
e2e/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/src/routes/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (2)packages/start-client-core/src/createStart.ts (2)
e2e/react-start/serialization-adapters/src/routeTree.gen.ts (2)
⏰ 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)
🔇 Additional comments (8)
Comment |
|
View your CI Pipeline Execution ↗ for commit 4e42139
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/start-client-core/src/createStart.ts (2)
79-79: Consider documenting the mutation side-effect.Setting
extends = undefinedis a deliberate mutation that prevents further propagation during deduplication. While this achieves the PR objective, the side-effect isn't immediately obvious from the function signature or name.Consider adding a brief comment explaining why this mutation is necessary, for example:
if (current.extends) { dedupeSerializationAdapters(deduped, current.extends) } + // Unset extends to prevent further propagation after deduplication current.extends = undefinedThis would help future maintainers understand the intent.
68-82: Add documentation for theextendsmutation indedupeSerializationAdapters
This function intentionally clears each adapter’sextendsproperty to prevent duplicate propagation—please add a JSDoc or inline comment clarifying that this side-effect is by design.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/start-client-core/src/createStart.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/start-client-core/src/createStart.ts
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-client-core/src/createStart.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
schiller-manuel
force-pushed
the
unset-extends
branch
from
a69d57c to
a2a3f9d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/react-start/serialization-adapters/src/data.tsx (1)
161-205: Optional: Remove unnecessary block scope.The extra block scope starting at line 162 is valid but unusual. Consider removing it for clarity.
Apply this diff:
export function RenderNestedData({ nested }: { nested: NestedOuter }) { - { - const localData = makeNested() + const localData = makeNested() + const expectedShoutState = localData.inner.shout() + const expectedWhisperState = localData.whisper() + const shoutState = nested.inner.shout() + const whisperState = nested.whisper() + + return ( ... - } + ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
e2e/react-start/serialization-adapters/src/data.tsx(1 hunks)e2e/react-start/serialization-adapters/src/routeTree.gen.ts(8 hunks)e2e/react-start/serialization-adapters/src/routes/index.tsx(2 hunks)e2e/react-start/serialization-adapters/src/routes/server-function/nested.tsx(1 hunks)e2e/react-start/serialization-adapters/src/routes/ssr/nested.tsx(2 hunks)e2e/react-start/serialization-adapters/tests/app.spec.ts(3 hunks)packages/router-core/src/ssr/serializer/transformer.ts(0 hunks)packages/start-client-core/src/createStart.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/router-core/src/ssr/serializer/transformer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/start-client-core/src/createStart.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/serialization-adapters/src/routes/index.tsxe2e/react-start/serialization-adapters/tests/app.spec.tse2e/react-start/serialization-adapters/src/data.tsxe2e/react-start/serialization-adapters/src/routes/server-function/nested.tsxe2e/react-start/serialization-adapters/src/routes/ssr/nested.tsxe2e/react-start/serialization-adapters/src/routeTree.gen.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/serialization-adapters/src/routes/index.tsxe2e/react-start/serialization-adapters/src/routes/server-function/nested.tsxe2e/react-start/serialization-adapters/src/routes/ssr/nested.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/serialization-adapters/src/routes/index.tsxe2e/react-start/serialization-adapters/tests/app.spec.tse2e/react-start/serialization-adapters/src/data.tsxe2e/react-start/serialization-adapters/src/routes/server-function/nested.tsxe2e/react-start/serialization-adapters/src/routes/ssr/nested.tsxe2e/react-start/serialization-adapters/src/routeTree.gen.ts
🧬 Code graph analysis (3)
e2e/react-start/serialization-adapters/src/routes/server-function/nested.tsx (2)
e2e/react-start/serialization-adapters/src/data.tsx (3)
makeNested(120-122)NestedOuter(91-96)RenderNestedData(161-205)e2e/react-start/serialization-adapters/src/routes/ssr/nested.tsx (1)
Route(4-14)
e2e/react-start/serialization-adapters/src/routes/ssr/nested.tsx (1)
e2e/react-start/serialization-adapters/src/data.tsx (1)
RenderNestedData(161-205)
e2e/react-start/serialization-adapters/src/routeTree.gen.ts (2)
e2e/react-start/server-functions/src/routeTree.gen.ts (1)
FileRoutesByTo(165-187)e2e/react-router/js-only-file-based/src/routeTree.gen.js (1)
IndexRoute(30-34)
⏰ 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: Preview
- GitHub Check: Test
🔇 Additional comments (7)
e2e/react-start/serialization-adapters/src/routes/index.tsx (1)
27-30: LGTM: SSR nested link added correctly.The new link properly navigates to the nested SSR route with appropriate test ID and reload behavior.
e2e/react-start/serialization-adapters/src/routes/ssr/nested.tsx (1)
1-14: LGTM: Clean refactor to shared rendering component.The extraction of rendering logic to
RenderNestedDatafollows DRY principles and maintains the same functionality while improving code reusability.e2e/react-start/serialization-adapters/tests/app.spec.ts (2)
25-41: LGTM: Well-structured test helper.The
checkNestedDatahelper extracts common assertion logic and follows DRY principles, making the tests more maintainable.
101-111: LGTM: Server function nested test is comprehensive.The test properly verifies the waiting state before triggering the server function and validates the nested data serialization after the response.
e2e/react-start/serialization-adapters/src/routes/server-function/nested.tsx (2)
7-9: LGTM: Server function correctly returns nested data.The server function is properly defined and returns the nested data structure that will be serialized.
15-34: LGTM: Clean component implementation with proper state management.The component correctly manages the async server function call, provides user feedback during the waiting state, and renders the nested data once available.
e2e/react-start/serialization-adapters/src/routeTree.gen.ts (1)
16-163: New nested route wiring is consistent.Import, instance update, type unions, and module declaration all mirror the surrounding routes, so the generated typings stay aligned with the new
/server-function/nestedentry.
schiller-manuel
force-pushed
the
unset-extends
branch
from
a2a3f9d to
4e42139
Compare
Summary by CodeRabbit
New Features
Bug Fixes
Tests