Skip to content

Conversation

@birkskyum
Copy link
Member

@birkskyum birkskyum commented Oct 26, 2025

test for

pass for react, fails for solid

Summary by CodeRabbit

  • New Features

    • Added redirect test routes and server-side redirect functionality to test environments
  • Tests

    • Added end-to-end tests to verify redirect behavior during direct navigation
    • Tests confirm successful redirect completion and proper URL updates across environments

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 26, 2025

Walkthrough

Adds end-to-end test coverage for server-function redirects on direct navigation to both React and Solid start examples. Introduces /redirect-test/ and /redirect-test/target routes with server functions implementing redirect behavior, route tree regeneration, and corresponding UI components.

Changes

Cohort / File(s) Change Summary
React Start Route Tree
e2e/react-start/server-functions/src/routeTree.gen.ts
Auto-generated route tree updated to include RedirectTestIndexRoute and RedirectTestTargetRoute; exports route mappings, type definitions, and module augmentation for new redirect-test routes
React Start Redirect Routes
e2e/react-start/server-functions/src/routes/redirect-test/index.tsx, e2e/react-start/server-functions/src/routes/redirect-test/target.tsx
New route files for /redirect-test/ index (triggers server function that redirects to target) and /redirect-test/target (landing page after redirect); includes server function definition and Suspense-wrapped async rendering
React Start E2E Tests
e2e/react-start/server-functions/tests/server-functions.spec.ts
New test case: "redirect in server function on direct navigation" verifies that direct navigation to /redirect-test triggers server-function redirect and lands on /redirect-test/target
Solid Start Route Tree
e2e/solid-start/server-functions/src/routeTree.gen.ts
Auto-generated route tree updated to include Solid Start equivalents of redirect-test routes; exports route mappings, type definitions, and module augmentation for new routes
Solid Start Redirect Routes
e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx, e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx
New Solid.js route files for /redirect-test/ index (with SSR data-only mode) and /redirect-test/target; implements server function redirect and query-based async rendering
Solid Start E2E Tests
e2e/solid-start/server-functions/tests/server-functions.spec.ts
New test case: "redirect in server function on direct navigation" verifies redirect behavior on direct navigation in Solid Start context

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Router
    participant Server
    participant Client

    User->>Router: Navigate to /redirect-test/
    Router->>Client: Render index route component
    Client->>Server: Call useServerFn($redirectServerFn) via useQuery
    Server->>Server: Execute server function handler
    Server-->>Server: throw redirect('/redirect-test/target')
    Server-->>Client: Return redirect response
    Client->>Router: Process redirect
    Router->>Router: Navigate to /redirect-test/target
    Router->>Client: Render target route component
    Client->>User: Display "Successfully redirected!"
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring attention:
    • Verify server function redirect behavior is correctly implemented in both React and Solid implementations
    • Confirm route tree generation (routeTree.gen.ts) accurately reflects new routes across both frameworks
    • Validate SSR handling in Solid Start (ssr: 'data-only' mode) and async rendering patterns in React (Suspense boundary)
    • Test coverage adequacy for redirect edge cases and URL verification

Possibly related issues

Suggested labels

package: react-router, package: solid-router

Suggested reviewers

  • schiller-manuel
  • brenelz

Poem

🐰 A hop, skip, and a redirect so fine,
Test routes dance in neat design,
From source to target they gracefully leap,
Server functions redirect, promises to keep!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The pull request title "fix(solid-start): redirect from server function" is partially related to the changeset but is misleading about scope. While the PR does address redirect functionality from server functions, the title's emphasis on "solid-start" suggests the changes are specific to the Solid implementation. However, the actual changeset includes identical redirect test routes, server functions, and test cases added to both the React and Solid examples. The title fails to capture that this is a framework-agnostic addition applied to both implementations, making it misleading regarding the true scope of the PR. Consider revising the title to accurately reflect that the changes apply to both frameworks, such as "feat(router): add server function redirect test coverage" or "feat: add redirect from server function test cases for React and Solid". This would make it clear that the redirect functionality is being added to both implementations rather than suggesting it's a Solid-specific fix.
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.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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(solid-start)--redirect-from-server-function

📜 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 0bda974 and 76b9c9c.

📒 Files selected for processing (8)
  • e2e/react-start/server-functions/src/routeTree.gen.ts (13 hunks)
  • e2e/react-start/server-functions/src/routes/redirect-test/index.tsx (1 hunks)
  • e2e/react-start/server-functions/src/routes/redirect-test/target.tsx (1 hunks)
  • e2e/react-start/server-functions/tests/server-functions.spec.ts (1 hunks)
  • e2e/solid-start/server-functions/src/routeTree.gen.ts (13 hunks)
  • e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx (1 hunks)
  • e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx (1 hunks)
  • e2e/solid-start/server-functions/tests/server-functions.spec.ts (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/server-functions/tests/server-functions.spec.ts
  • e2e/react-start/server-functions/src/routes/redirect-test/index.tsx
  • e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx
  • e2e/react-start/server-functions/src/routeTree.gen.ts
  • e2e/react-start/server-functions/tests/server-functions.spec.ts
  • e2e/react-start/server-functions/src/routes/redirect-test/target.tsx
  • e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx
  • e2e/solid-start/server-functions/src/routeTree.gen.ts
e2e/**

📄 CodeRabbit inference engine (AGENTS.md)

Store end-to-end tests under the e2e/ directory

Files:

  • e2e/solid-start/server-functions/tests/server-functions.spec.ts
  • e2e/react-start/server-functions/src/routes/redirect-test/index.tsx
  • e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx
  • e2e/react-start/server-functions/src/routeTree.gen.ts
  • e2e/react-start/server-functions/tests/server-functions.spec.ts
  • e2e/react-start/server-functions/src/routes/redirect-test/target.tsx
  • e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx
  • e2e/solid-start/server-functions/src/routeTree.gen.ts
**/src/routes/**

📄 CodeRabbit inference engine (AGENTS.md)

Place file-based routes under src/routes/ directories

Files:

  • e2e/react-start/server-functions/src/routes/redirect-test/index.tsx
  • e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx
  • e2e/react-start/server-functions/src/routes/redirect-test/target.tsx
  • e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx
🧬 Code graph analysis (4)
e2e/react-start/server-functions/src/routes/redirect-test/index.tsx (1)
e2e/react-start/server-functions/src/routes/redirect-test/target.tsx (1)
  • Route (3-5)
e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx (1)
e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx (1)
  • Route (10-13)
e2e/react-start/server-functions/src/routes/redirect-test/target.tsx (1)
e2e/react-start/server-functions/src/routes/redirect-test/index.tsx (1)
  • Route (10-12)
e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx (1)
e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx (1)
  • Route (3-5)
🔇 Additional comments (12)
e2e/solid-start/server-functions/src/routes/redirect-test/target.tsx (1)

1-14: LGTM!

The redirect target route is correctly implemented with proper Solid.js conventions and the necessary test identifier.

e2e/react-start/server-functions/src/routes/redirect-test/target.tsx (1)

1-14: LGTM!

The redirect target route is correctly implemented with proper React conventions and the necessary test identifier.

e2e/react-start/server-functions/src/routes/redirect-test/index.tsx (3)

1-8: LGTM!

The imports and server function are correctly structured. The redirect is properly thrown using the TanStack Router redirect API.


10-12: LGTM!

The route configuration is correctly set up for the redirect test scenario.


14-29: LGTM!

The component correctly integrates the server function with React Query. The Suspense boundary appropriately wraps the async content. Note that query.data will likely be undefined since the server function redirects rather than returns data, but this fallback is acceptable.

e2e/solid-start/server-functions/src/routeTree.gen.ts (1)

1-579: LGTM!

The auto-generated route tree correctly incorporates the new redirect-test routes with proper type definitions, module augmentations, and route hierarchy. All additions are consistent with the existing route structure.

e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx (3)

1-8: LGTM!

The imports and server function are correctly structured for Solid.js. The redirect is properly thrown using the TanStack Router redirect API.


10-13: Verify the ssr: 'data-only' option.

The React version of this route doesn't include the ssr: 'data-only' option. Since the PR description states that the test passes for React but fails for Solid, this difference in SSR handling could be the root cause. The 'data-only' mode might prevent the redirect from being properly intercepted during server-side rendering on direct navigation.

Consider removing the ssr option or changing it to match the React implementation to see if this resolves the test failure. You may also want to verify whether server-function redirects are expected to work with ssr: 'data-only' mode.


15-30: LGTM!

The component correctly uses Solid.js patterns, including the proper useQuery API syntax for Solid Query (function returning config object). The Suspense boundary and test identifiers are properly configured.

e2e/react-start/server-functions/tests/server-functions.spec.ts (1)

472-479: LGTM!

The test correctly validates server-function redirect behavior on direct navigation. The assertions properly verify both the visual presence of the redirect target and the URL change.

e2e/solid-start/server-functions/tests/server-functions.spec.ts (1)

478-485: Test implementation is correct, but expected to fail with current route configuration.

The test is properly structured and identical to the React version. However, as noted in the PR description, this test currently fails for Solid. This is likely due to the ssr: 'data-only' option in the corresponding route configuration (e2e/solid-start/server-functions/src/routes/redirect-test/index.tsx line 12), which may prevent proper redirect handling during direct navigation.

e2e/react-start/server-functions/src/routeTree.gen.ts (1)

1-579: LGTM!

The auto-generated route tree correctly incorporates the new redirect-test routes with proper type definitions, module augmentations, and route hierarchy. All additions are consistent with the existing route structure and parallel the Solid implementation.


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.

❤️ Share

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

@nx-cloud
Copy link

nx-cloud bot commented Oct 26, 2025

View your CI Pipeline Execution ↗ for commit 76b9c9c

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

☁️ Nx Cloud last updated this comment at 2025-10-26 19:33:11 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 26, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/zod-adapter

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

commit: 76b9c9c

@birkskyum birkskyum marked this pull request as ready for review October 26, 2025 19:33
@birkskyum birkskyum merged commit 78892de into main Oct 26, 2025
5 of 6 checks passed
@birkskyum birkskyum deleted the fix(solid-start)--redirect-from-server-function branch October 26, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant