-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
test(solid-start): fix solid-query when used in ssr #5665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughEnable QueryClient's experimental_prefetchInRender by default in multiple Solid Start router setups and change several route SSR configurations from the string Changes
Sequence Diagram(s)sequenceDiagram
participant Router as Router Setup
participant QC as QueryClient
participant Component as Route Render
rect #E6F7FF
Router->>QC: create QueryClient(defaultOptions.queries.experimental_prefetchInRender = true)
end
rect #F0FFF4
Component->>QC: request/prefetch during render
QC-->>Component: return prefetched data (sync/SSR-aware)
note right of QC: Prefetch-in-render always enabled
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/src/routes/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
e2e/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 3ac02b8
☁️ 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-router-ssr-query
@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: |
0cdc0ea to
ebe55c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
e2e/solid-start/query-integration/src/router.tsx (1)
7-13: QueryClient configuration is correct but note code duplication.The QueryClient configuration with
experimental_prefetchInRender: truealigns with the PR objective. However, this identical configuration appears across multiple test router files (server-functions, basic-solid-query, and query-integration). While this duplication is reasonable for isolated e2e test suites, consider whether a shared factory function could reduce maintenance if this pattern expands.Minor style note: Line 13 has a semicolon while the equivalent line in
e2e/solid-start/server-functions/src/router.tsxdoes not. Consider standardizing for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
e2e/solid-start/basic-solid-query/src/router.tsx(1 hunks)e2e/solid-start/query-integration/src/router.tsx(1 hunks)e2e/solid-start/query-integration/src/routes/useQuery.tsx(1 hunks)e2e/solid-start/server-functions/src/router.tsx(1 hunks)e2e/solid-start/server-functions/src/routes/primitives/index.tsx(1 hunks)
🧰 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/solid-start/query-integration/src/router.tsxe2e/solid-start/server-functions/src/routes/primitives/index.tsxe2e/solid-start/query-integration/src/routes/useQuery.tsxe2e/solid-start/basic-solid-query/src/router.tsxe2e/solid-start/server-functions/src/router.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/query-integration/src/router.tsxe2e/solid-start/server-functions/src/routes/primitives/index.tsxe2e/solid-start/query-integration/src/routes/useQuery.tsxe2e/solid-start/basic-solid-query/src/router.tsxe2e/solid-start/server-functions/src/router.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/server-functions/src/routes/primitives/index.tsxe2e/solid-start/query-integration/src/routes/useQuery.tsx
⏰ 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). (1)
- GitHub Check: Preview
🔇 Additional comments (4)
e2e/solid-start/server-functions/src/router.tsx (1)
9-15: LGTM! Configuration enables prefetch-in-render for SSR testing.The QueryClient configuration with
experimental_prefetchInRender: truealigns with the PR objective to fix solid-query SSR behavior. This is appropriate for an e2e test environment where consistent SSR/CSR prefetch behavior is desired.e2e/solid-start/basic-solid-query/src/router.tsx (1)
12-12: Enabling prefetch-in-render for SSR is appropriate for this test.The change from
typeof window !== 'undefined'totrueenables prefetching during server-side rendering, which is consistent with the PR's objective to fix solid-query SSR behavior. This unconditional enablement is appropriate for e2e testing.e2e/solid-start/query-integration/src/routes/useQuery.tsx (1)
10-10: No issues found. The SSR configuration change is valid.The
ssrproperty is typed asSSROption = boolean | 'data-only'in the router core package. Bothtrueand'data-only'are valid SSROption values, making the change from'data-only'totruea properly typed transformation.e2e/solid-start/server-functions/src/routes/primitives/index.tsx (1)
8-8: No issues found after verification.The change from
ssr: 'data-only'tossr: trueis valid and intentional. Both values are supported by TanStack Start'screateFileRouteAPI—trueenables full server-side rendering (both data loading and component rendering), while'data-only'only renders data on the server. For a route dedicated to testing server functions with primitive types, full SSR is semantically appropriate and ensures the component is properly rendered with server function results server-side.
closes #5639
ssr: true#5639Summary by CodeRabbit