Skip to content

Conversation

@nlynzaad
Copy link
Contributor

@nlynzaad nlynzaad commented Dec 25, 2025

This PR resolves #6023 by ensuring trailing slashes is stripped from the base-url setting in vite before pre-rendering is executed.

Also added pre-rendering modes to react-start/custom-basepath with and without trailing slashes in the base url.

Summary by CodeRabbit

  • New Features

    • Added prerender build variants, including optional trailing-slash mode.
    • E2E runs now cover SSR, prerender, and prerender-with-trailing-slash.
    • Test runner now selects server mode at runtime and exposes mode/port env for tests.
    • App routes and API calls now derive the runtime base URL from the running router origin.
  • Chores

    • E2E scripts reorganized into modular start/stop and mode-specific commands.
    • Tests include a mode-aware utility and updated redirect assertions for basepath changes.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 25, 2025

📝 Walkthrough

Walkthrough

This PR makes e2e suites and core prerender logic aware of MODE=prerender and optional TRAILING_SLASH: adds prerender/trailing build & test scripts, switches Playwright webServer command selection by mode, derives base URLs at runtime from router origin/import.meta.env.BASE_URL, removes static basepath exports, and normalizes preview base for prerender.

Changes

Cohort / File(s) Summary
Build & Test Scripts
e2e/*-start/custom-basepath/package.json
Add build:prerender / build:prerender:trailing; split test:e2e into test:e2e:ssrMode, test:e2e:prerender, test:e2e:prerender:trailing, add test:e2e:startDummyServer/stopDummyServer; compose aggregated test:e2e.
Playwright / Test Server Config
e2e/*-start/custom-basepath/playwright.config.ts
Replace static webServer.command with getCommand() that selects SSR/prerender/prerender-with-trailing based on isPrerender and trailing flag; expand webServer.env to expose MODE/TRAILING_SLASH/VITE_*/START_PORT/PORT and log chosen mode.
Vite Config / Prerender Wiring
e2e/*-start/custom-basepath/vite.config.ts
Add isPrerender conditional prerenderConfiguration (enabled/filter/onSuccess); conditionally pass prerender to tanstackStart(); compute base with optional trailing slash via TRAILING_SLASH.
Router basepath resolution
e2e/*-start/custom-basepath/src/router.tsx
Remove static basepath import; set router basepath from import.meta.env.BASE_URL at runtime.
Route loaders / API calls
e2e/*-start/custom-basepath/src/routes/users.tsx, e2e/*-start/custom-basepath/src/routes/users.$userId.tsx
Remove basepath import/use; loaders await getRouterInstance() and use router.options.basepath for paths and router.origin as axios baseURL.
Removed static basepath util
e2e/*-start/custom-basepath/src/utils/basepath.ts
Delete exported basepath constant ('/custom/basepath').
E2E tests / utils
e2e/*-start/custom-basepath/tests/utils/isPrerender.ts, e2e/*-start/custom-basepath/tests/navigation.spec.ts
Add isPrerender (process.env.MODE === 'prerender'); update navigation redirect assertions to expect '/custom/basepath/posts/1'.
Prerender core normalization
packages/start-plugin-core/src/prerender.ts
Apply removeTrailingSlash to the preview server base option so preview requests use a normalized base without trailing slash.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Runner (CI / Dev)
    participant Vite as Vite Build
    participant Preview as Vite Preview Server
    participant Prerender as Prerenderer
    participant Router as App Router (getRouterInstance)
    participant API as Backend API

    Dev->>Vite: build (MODE=prerender, TRAILING_SLASH=?)
    activate Vite
    Vite->>Vite: resolve base from env (import.meta.env.BASE_URL +/- trailing slash)
    Vite->>Preview: start preview (base = removeTrailingSlash(base))
    activate Preview
    Vite->>Prerender: start crawl (uses preview origin)
    activate Prerender
    Prerender->>Preview: HTTP request to <base>/...
    Preview->>Router: initialize route, call loader
    Router->>Router: await getRouterInstance()
    Router->>API: axios GET /api/... (baseURL = router.origin)
    API-->>Router: data
    Router-->>Preview: rendered HTML
    Preview-->>Prerender: HTML response
    Prerender->>Prerender: onSuccess hook (log/output)
    deactivate Prerender
    Vite-->>Dev: build + prerender artifacts
    deactivate Vite
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

Suggested reviewers

  • schiller-manuel
  • birkskyum

Poem

🐰 Hop, hop — flags set, the crawl begins,

Basepaths found where the server wins,
Trailing slashes trimmed from the trail,
Tests tumble through without fail,
A tiny carrot cheer for every passing bin!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
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.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding support for trailing slashes in basepaths during pre-rendering, which directly addresses issue #6023.
Linked Issues check ✅ Passed The PR comprehensively addresses issue #6023 by: (1) stripping trailing slashes in prerender.ts for proper base URL handling [#6023], (2) adding prerender mode detection and conditional build scripts [#6023], (3) updating route loaders to use dynamic router-based basepaths instead of static imports [#6023], (4) adding E2E tests for prerender configurations with and without trailing slashes [#6023], and (5) updating test expectations to reflect corrected redirect behavior [#6023].
Out of Scope Changes check ✅ Passed All changes are directly related to resolving the prerender basepath issue: package.json scripts for prerender workflows, playwright config for mode-aware testing, vite configs for prerender setup, router/route updates for dynamic basepath handling, and test utilities/specs for prerender validation. 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 prerender_basepath

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

@nx-cloud
Copy link

nx-cloud bot commented Dec 25, 2025

View your CI Pipeline Execution ↗ for commit 555faee

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

☁️ Nx Cloud last updated this comment at 2025-12-28 05:09:21 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 25, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-fn-stubs

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-fn-stubs@6215

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/vue-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router@6215

@tanstack/vue-router-devtools

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

@tanstack/vue-router-ssr-query

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

@tanstack/vue-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start@6215

@tanstack/vue-start-client

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

@tanstack/vue-start-server

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

@tanstack/zod-adapter

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

commit: 555faee

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
e2e/react-start/custom-basepath/vite.config.ts (1)

6-6: Consider using a logger for prerender success messages.

The console.log on line 19 for prerender success could be replaced with a structured logger for consistency with production code patterns.

Optional improvement
  onSuccess: ({ page }: { page: { path: string } }) => {
-    console.log(`Rendered ${page.path}!`)
+    // Optional: Use a structured logger if available in test context
+    console.log(`Rendered ${page.path}!`)
  },

Note: This is a test configuration file, so console.log is acceptable. This suggestion is purely optional for consistency.

Also applies to: 8-21

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e4cf5b and b00df22.

📒 Files selected for processing (10)
  • e2e/react-start/custom-basepath/package.json
  • e2e/react-start/custom-basepath/playwright.config.ts
  • e2e/react-start/custom-basepath/src/router.tsx
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/routes/users.tsx
  • e2e/react-start/custom-basepath/src/utils/basepath.ts
  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/react-start/custom-basepath/vite.config.ts
  • packages/start-plugin-core/src/prerender.ts
💤 Files with no reviewable changes (1)
  • e2e/react-start/custom-basepath/src/utils/basepath.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety for all code

Files:

  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/routes/users.tsx
  • e2e/react-start/custom-basepath/src/router.tsx
  • packages/start-plugin-core/src/prerender.ts
  • e2e/react-start/custom-basepath/playwright.config.ts
  • e2e/react-start/custom-basepath/vite.config.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Implement ESLint rules for router best practices using the ESLint plugin router

Files:

  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/routes/users.tsx
  • e2e/react-start/custom-basepath/src/router.tsx
  • packages/start-plugin-core/src/prerender.ts
  • e2e/react-start/custom-basepath/playwright.config.ts
  • e2e/react-start/custom-basepath/vite.config.ts
**/package.json

📄 CodeRabbit inference engine (AGENTS.md)

Use workspace protocol workspace:* for internal dependencies in package.json files

Files:

  • e2e/react-start/custom-basepath/package.json
🧠 Learnings (8)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
📚 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:

  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/router.tsx
  • packages/start-plugin-core/src/prerender.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:

  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
📚 Learning: 2025-10-09T12:59:02.129Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.

Applied to files:

  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
📚 Learning: 2025-12-21T12:52:35.231Z
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.

Applied to files:

  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/src/router.tsx
  • packages/start-plugin-core/src/prerender.ts
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.

Applied to files:

  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/router.tsx
  • packages/start-plugin-core/src/prerender.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router

Applied to files:

  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/router.tsx
  • packages/start-plugin-core/src/prerender.ts
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.

Applied to files:

  • e2e/react-start/custom-basepath/src/router.tsx
  • packages/start-plugin-core/src/prerender.ts
🧬 Code graph analysis (6)
e2e/react-start/custom-basepath/tests/navigation.spec.ts (1)
e2e/react-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/react-start/custom-basepath/src/routes/users.$userId.tsx (1)
e2e/react-start/custom-basepath/src/routes/users.tsx (1)
  • Route (6-19)
e2e/react-start/custom-basepath/src/routes/users.tsx (1)
e2e/react-start/custom-basepath/src/routes/users.$userId.tsx (1)
  • Route (8-25)
packages/start-plugin-core/src/prerender.ts (3)
packages/router-core/src/path.ts (1)
  • removeTrailingSlash (45-50)
packages/router-core/src/index.ts (1)
  • removeTrailingSlash (100-100)
packages/router-generator/src/index.ts (1)
  • removeTrailingSlash (19-19)
e2e/react-start/custom-basepath/playwright.config.ts (1)
e2e/react-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/react-start/custom-basepath/vite.config.ts (1)
e2e/react-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
⏰ 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 (10)
e2e/react-start/custom-basepath/tests/utils/isPrerender.ts (1)

1-1: LGTM!

Clean utility for prerender mode detection. The environment-based flag pattern is appropriate for test configuration.

e2e/react-start/custom-basepath/src/router.tsx (1)

13-13: LGTM!

Using import.meta.env.BASE_URL for dynamic basepath configuration aligns with Vite conventions and supports the prerender functionality improvements in this PR.

e2e/react-start/custom-basepath/tests/navigation.spec.ts (1)

2-2: LGTM!

The conditional URL assertion correctly handles the different trailing slash behavior between prerender and SSR modes. This aligns with the PR objectives to support both URL formats in testing.

Also applies to: 63-67

e2e/react-start/custom-basepath/src/routes/users.$userId.tsx (1)

3-3: LGTM!

The shift from static basepath to dynamic router.options.origin for API calls is consistent with the PR's approach to support runtime base URL determination. The pattern matches the implementation in users.tsx.

Also applies to: 10-14

e2e/react-start/custom-basepath/src/routes/users.tsx (1)

3-3: LGTM!

Consistent implementation of dynamic baseURL using router.options.origin. This aligns with the pattern in users.$userId.tsx and supports the PR's runtime base path determination.

Also applies to: 8-12

e2e/react-start/custom-basepath/vite.config.ts (1)

24-24: LGTM!

The dynamic base path construction with optional trailing slash and conditional prerender configuration effectively support the PR's objectives for handling different base path formats.

Also applies to: 35-35

e2e/react-start/custom-basepath/playwright.config.ts (2)

7-7: LGTM!

The command selection logic and debugging output provide good visibility into test mode execution. The structure cleanly supports SSR, prerender, and prerender-with-trailing-slash scenarios.

Also applies to: 10-10, 14-35


55-68: LGTM!

The expanded environment variable configuration properly supports the dynamic test modes and port management. The environment setup aligns well with the prerender and base path handling changes throughout the PR.

packages/start-plugin-core/src/prerender.ts (1)

5-5: The original review incorrectly referenced the removeTrailingSlash function from packages/router-core/src/path.ts (which takes 2 parameters), but the import at line 5 actually uses the version from @tanstack/router-generator, which is defined in packages/router-generator/src/utils.ts and takes only 1 parameter: removeTrailingSlash(s: string). The usage at line 274 is correct.

Likely an incorrect or invalid review comment.

e2e/react-start/custom-basepath/package.json (1)

13-14: Server setup and lifecycle are properly implemented.

The referenced files exist and are correctly integrated:

  • global.setup.ts and global.teardown.ts delegate to @tanstack/router-e2e-utils
  • The dummy server properly waits for readiness via a Promise that resolves on server.listen() callback, with an additional 500ms buffer
  • Lifecycle is managed through Playwright's globalSetup and globalTeardown configuration, which applies consistently across all test modes (SSR, prerender, and prerender with trailing slash)

@nlynzaad
Copy link
Contributor Author

e2e tasks are passing locally. need to investigate why these are failing in the CI

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (3)
e2e/solid-start/custom-basepath/playwright.config.ts (1)

10-10: Clarify the purpose of START_PORT.

START_PORT is defined but only passed through the environment (line 66). The webServer commands appear to use PORT instead. If START_PORT is unused, consider removing it to reduce confusion. If it's required by the start scripts, add a comment explaining its purpose.

e2e/vue-start/custom-basepath/playwright.config.ts (2)

10-10: Clarify the purpose of START_PORT.

START_PORT is defined but only passed through the environment (line 66). If it's not actively used by the start scripts, consider removing it. If it serves a specific purpose, add a comment to document its role.


14-35: LGTM! Consider extracting shared configuration.

The command selection logic and environment handling are correct. However, this file is nearly identical to e2e/solid-start/custom-basepath/playwright.config.ts. Consider extracting the shared prerender configuration logic into a common utility to reduce duplication across framework test suites.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b00df22 and b4b41dd.

📒 Files selected for processing (19)
  • e2e/react-start/custom-basepath/package.json
  • e2e/solid-start/custom-basepath/package.json
  • e2e/solid-start/custom-basepath/playwright.config.ts
  • e2e/solid-start/custom-basepath/src/router.tsx
  • e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/solid-start/custom-basepath/src/utils/basepath.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/solid-start/custom-basepath/vite.config.ts
  • e2e/vue-start/custom-basepath/package.json
  • e2e/vue-start/custom-basepath/playwright.config.ts
  • e2e/vue-start/custom-basepath/src/router.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.tsx
  • e2e/vue-start/custom-basepath/src/utils/basepath.ts
  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/vue-start/custom-basepath/vite.config.ts
💤 Files with no reviewable changes (2)
  • e2e/solid-start/custom-basepath/src/utils/basepath.ts
  • e2e/vue-start/custom-basepath/src/utils/basepath.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety for all code

Files:

  • e2e/vue-start/custom-basepath/src/router.tsx
  • e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/solid-start/custom-basepath/vite.config.ts
  • e2e/vue-start/custom-basepath/playwright.config.ts
  • e2e/solid-start/custom-basepath/playwright.config.ts
  • e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.tsx
  • e2e/vue-start/custom-basepath/vite.config.ts
  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/src/router.tsx
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Implement ESLint rules for router best practices using the ESLint plugin router

Files:

  • e2e/vue-start/custom-basepath/src/router.tsx
  • e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/solid-start/custom-basepath/vite.config.ts
  • e2e/vue-start/custom-basepath/playwright.config.ts
  • e2e/solid-start/custom-basepath/playwright.config.ts
  • e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts
  • e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.tsx
  • e2e/vue-start/custom-basepath/vite.config.ts
  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/src/router.tsx
**/package.json

📄 CodeRabbit inference engine (AGENTS.md)

Use workspace protocol workspace:* for internal dependencies in package.json files

Files:

  • e2e/vue-start/custom-basepath/package.json
  • e2e/react-start/custom-basepath/package.json
  • e2e/solid-start/custom-basepath/package.json
🧠 Learnings (12)
📓 Common learnings
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:46.897Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.

Applied to files:

  • e2e/vue-start/custom-basepath/src/router.tsx
  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/solid-start/custom-basepath/src/router.tsx
📚 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:

  • e2e/vue-start/custom-basepath/src/router.tsx
  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/src/router.tsx
📚 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:

  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router

Applied to files:

  • e2e/solid-start/custom-basepath/src/routes/users.tsx
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.

Applied to files:

  • e2e/solid-start/custom-basepath/vite.config.ts
  • e2e/vue-start/custom-basepath/vite.config.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/src/router.tsx
📚 Learning: 2025-12-25T13:04:46.897Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:46.897Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.

Applied to files:

  • e2e/solid-start/custom-basepath/vite.config.ts
  • e2e/vue-start/custom-basepath/playwright.config.ts
  • e2e/vue-start/custom-basepath/package.json
  • e2e/solid-start/custom-basepath/playwright.config.ts
  • e2e/react-start/custom-basepath/package.json
  • e2e/solid-start/custom-basepath/package.json
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Always run `pnpm test:eslint`, `pnpm test:types`, and `pnpm test:unit` before committing code

Applied to files:

  • e2e/react-start/custom-basepath/package.json
📚 Learning: 2025-10-09T12:59:14.842Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/public/site.webmanifest:2-3
Timestamp: 2025-10-09T12:59:14.842Z
Learning: In e2e test fixtures (files under e2e directories), empty or placeholder values in configuration files like site.webmanifest are acceptable and should not be flagged unless the test specifically validates those fields.

Applied to files:

  • e2e/react-start/custom-basepath/package.json
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Use file-based routing in `src/routes/` directories or code-based routing with route definitions

Applied to files:

  • e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx
📚 Learning: 2025-12-21T12:52:35.231Z
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.

Applied to files:

  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
📚 Learning: 2025-10-09T12:59:02.129Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.

Applied to files:

  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
🧬 Code graph analysis (11)
e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts (1)
e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/solid-start/custom-basepath/src/routes/users.tsx (2)
e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx (1)
  • Route (8-25)
e2e/vue-start/custom-basepath/src/routes/users.tsx (1)
  • Route (6-19)
e2e/solid-start/custom-basepath/vite.config.ts (1)
e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/vue-start/custom-basepath/playwright.config.ts (2)
e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/solid-start/custom-basepath/playwright.config.ts (2)
e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts (1)
e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx (2)
e2e/solid-start/custom-basepath/src/routes/users.tsx (1)
  • Route (6-19)
e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx (1)
  • Route (8-25)
e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx (3)
e2e/vue-start/custom-basepath/src/routes/users.tsx (1)
  • Route (6-19)
e2e/vue-start/custom-basepath/src/routes/api/users.$userId.ts (1)
  • Route (10-33)
e2e/vue-start/custom-basepath/src/routes/index.tsx (1)
  • Route (4-6)
e2e/vue-start/custom-basepath/src/routes/users.tsx (2)
e2e/solid-start/custom-basepath/src/routes/users.tsx (1)
  • Route (6-19)
e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx (1)
  • Route (8-25)
e2e/vue-start/custom-basepath/tests/navigation.spec.ts (1)
e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/solid-start/custom-basepath/tests/navigation.spec.ts (2)
e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts (1)
  • isPrerender (1-1)
⏰ 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 (23)
e2e/vue-start/custom-basepath/package.json (1)

9-18: LGTM! Well-structured prerender testing support.

The new build and test scripts properly support the PR's objectives of testing prerender behavior with custom base paths and trailing slashes. The scripts correctly use cross-env for environment variables and follow the established pattern from the react-start suite. The separation of dummy server utility scripts from the main test workflow is clear and appropriate.

e2e/react-start/custom-basepath/package.json (2)

9-10: Build scripts look good—previous cross-platform issue resolved.

The build scripts now correctly use cross-env for environment variable assignments, addressing the previous review feedback. Both prerender variants properly set their respective environment flags and chain the build commands.


13-18: Test scripts are well-structured and align with CI environment.

The test script organization effectively supports the PR's objective to add prerender test coverage with and without trailing slashes. The Unix-specific commands (&, rm -rf) are intentional and appropriate for the CI environment, as confirmed by project learnings. The aggregator script properly sequences all three test modes (SSR, prerender, prerender with trailing slash).

e2e/solid-start/custom-basepath/package.json (2)

9-10: LGTM: Build scripts correctly configure prerender modes.

The build scripts appropriately set MODE=prerender and optionally TRAILING_SLASH=true to enable prerender-specific behavior in the Vite configuration, followed by type checking.


13-18: LGTM: Test scripts provide comprehensive mode coverage.

The test scripts correctly:

  • Set up and tear down the dummy server using dynamic imports
  • Clean up port files to prevent conflicts
  • Configure environment variables for prerender and SSR modes
  • Run tests sequentially to cover all scenarios (SSR, prerender, prerender with trailing slash)
e2e/vue-start/custom-basepath/src/router.tsx (1)

13-13: LGTM: Router basepath now uses environment-driven configuration.

Switching from a static import to import.meta.env.BASE_URL allows the basepath to be dynamically configured via Vite at build time, which is essential for supporting prerender with custom base paths as described in the PR objectives.

e2e/solid-start/custom-basepath/src/router.tsx (1)

13-13: LGTM: Consistent environment-driven basepath configuration.

This change mirrors the vue-start implementation, using import.meta.env.BASE_URL for runtime basepath configuration, ensuring consistent behavior across framework variants.

e2e/vue-start/custom-basepath/tests/navigation.spec.ts (2)

2-2: LGTM: Import enables mode-specific test assertions.

The isPrerender utility allows tests to adapt expectations based on whether the app is running in prerender or SSR mode.


63-67: LGTM: Conditional assertion correctly handles trailing slash differences.

The conditional URL assertion appropriately expects a trailing slash (/posts/1/) in prerender mode and no trailing slash (/posts/1) in SSR mode, aligning with the PR's goal of supporting configurable trailing slashes during prerender.

e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts (1)

1-1: LGTM: Simple and effective mode detection.

The utility correctly determines prerender mode from the environment variable set by the test scripts. The expression safely evaluates to false when MODE is undefined or set to any value other than 'prerender'.

e2e/solid-start/custom-basepath/tests/navigation.spec.ts (1)

2-2: LGTM: Consistent prerender-aware test implementation.

The test changes mirror the vue-start implementation, using isPrerender to conditionally assert URL formats. This consistency across framework variants ensures uniform behavior validation for prerender trailing slash handling.

Also applies to: 63-67

e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx (1)

2-2: LGTM: Consistent dynamic baseURL pattern.

This route follows the same pattern as users.tsx, using router.options.origin for the API baseURL, ensuring consistent runtime basepath handling across all routes.

Also applies to: 10-14

e2e/solid-start/custom-basepath/src/routes/users.tsx (1)

3-3: router.options.origin contains only the protocol and host, not the basepath.

The getRouterInstance() API is documented and available in @tanstack/solid-start, but the review comment's assumption about what router.options.origin contains is incorrect. Based on the TanStack Router source code, router.options.origin stores only the protocol and host (e.g., http://localhost or https://example.com). The basepath is a separate property (router.options.basepath or router.basepath).

For correct API baseURL configuration with custom basepaths, the basepath should be appended to the origin: new URL(router.options.basepath || '/', router.options.origin).href or similar, not router.options.origin alone.

Likely an incorrect or invalid review comment.

e2e/solid-start/custom-basepath/tests/utils/isPrerender.ts (1)

1-1: LGTM!

The utility correctly derives the prerender flag from the environment. The strict equality check safely handles undefined values.

e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx (1)

9-14: LGTM!

The loader correctly retrieves the router instance and uses router.options.origin as the baseURL, ensuring API calls respect the configured base path in both SSR and prerender modes.

e2e/solid-start/custom-basepath/playwright.config.ts (2)

20-35: LGTM!

The command selection logic and prerender mode logging provide clear visibility into test execution modes. The dynamic command selection based on isPrerender and trailing slash configuration is well-structured.


59-68: Environment configuration looks comprehensive.

The environment variables correctly wire test modes, ports, and configuration flags to the webServer process. This ensures consistent behavior across SSR and prerender test runs.

e2e/solid-start/custom-basepath/vite.config.ts (2)

8-21: Prerender configuration is well-structured.

The filter appropriately excludes redirect and error routes from prerendering. The onSuccess callback provides useful feedback during the prerender process.


24-24: Trailing slash normalization is properly handled by the prerender plugin.

The dynamic trailing slash addition in vite.config.ts aligns with the prerender plugin's base path normalization. The prerender plugin explicitly removes the trailing slash from viteConfig.base at line 274 of packages/start-plugin-core/src/prerender.ts when starting the preview server: base: removeTrailingSlash(viteConfig.base). This ensures that regardless of the TRAILING_SLASH environment variable setting in the vite config, the trailing slash is stripped before prerendering, which satisfies the requirement from issue #6023.

e2e/vue-start/custom-basepath/vite.config.ts (3)

9-22: Prerender configuration looks good.

The filter appropriately excludes routes that shouldn't be prerendered, and the success callback provides useful logging.


25-25: Verify trailing slash handling aligns with fix.

The base path conditionally appends a trailing slash based on TRAILING_SLASH. According to the PR objective (issue #6023), the fix should strip trailing slashes from the base path before prerendering. Confirm this dynamic trailing slash behavior is compatible with the prerender plugin's base path normalization.


7-7: Import path verified as correct.

The file exists at the expected location e2e/vue-start/custom-basepath/tests/utils/isPrerender.ts, confirming the import path './tests/utils/isPrerender' is valid.

e2e/vue-start/custom-basepath/src/routes/users.tsx (1)

7-12: The code change is correct; router.options.origin is properly available during loader execution.

The router's origin is set via router.update() before loaders execute. In the start handler, router.update() with the origin value (line 122-135 in createStartHandler.ts) occurs within the getRouter() function, which is called before router.load() (which executes loaders). When getRouterInstance() is invoked from within a loader, it retrieves the context-bound router that already has the origin populated. This pattern works consistently in both SSR and prerender modes, as the origin is always set regardless of the execution environment.

@nlynzaad nlynzaad marked this pull request as draft December 26, 2025 12:50
@nlynzaad nlynzaad marked this pull request as ready for review December 28, 2025 05:11
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
e2e/solid-start/custom-basepath/tests/navigation.spec.ts (1)

56-77: LGTM! Consider clarifying the comment.

The updated expectation on line 69 correctly verifies that the server performs a full redirect from /redirect/throw-it to /custom/basepath/posts/1 in a single hop, which aligns with the PR's objective to fix basepath handling.

However, the comment on lines 64-65 says "this will just add the base path", which is now slightly misleading since the redirect goes directly to the final destination (/posts/1) rather than just prepending the basepath to the original route.

Optional: Clarify the comment
-  // do not follow redirects since we want to test the Location header
-  // first go to the route WITHOUT the base path, this will just add the base path
+  // do not follow redirects since we want to test the Location header
+  // first go to the route WITHOUT the base path, the server should redirect to the final destination with basepath
e2e/vue-start/custom-basepath/tests/navigation.spec.ts (1)

56-77: Test expectation correctly updated to reflect redirect target.

The change to expect /custom/basepath/posts/1 (line 69) is correct—when requesting the redirect route /redirect/throw-it without the base path, the server should resolve the redirect to /posts/1 and prepend the base path, yielding /custom/basepath/posts/1. This validates the dynamic basepath resolution during server-side redirects.

However, the comment on line 64 could be clarified. It currently states "this will just add the base path," but the server is actually processing the redirect to /posts/1 and applying the base path to the redirect target, not merely prepending the base path to the original URL.

🔎 Suggested comment clarification
-  // do not follow redirects since we want to test the Location header
-  // first go to the route WITHOUT the base path, this will just add the base path
+  // do not follow redirects since we want to test the Location header
+  // first request WITHOUT the base path: server processes redirect and applies base path to target
   await page.request
     .get('/redirect/throw-it', { maxRedirects: 0 })
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 723a69d and 555faee.

📒 Files selected for processing (9)
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/routes/users.tsx
  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
  • e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.tsx
  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/react-start/custom-basepath/src/routes/users.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.tsx
  • e2e/react-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/src/routes/users.tsx
  • e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety for all code

Files:

  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Implement ESLint rules for router best practices using the ESLint plugin router

Files:

  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: Sheraff
Repo: TanStack/router PR: 6171
File: packages/router-core/src/new-process-route-tree.ts:898-898
Timestamp: 2025-12-21T12:52:35.231Z
Learning: In `packages/router-core/src/new-process-route-tree.ts`, the matching logic intentionally allows paths without trailing slashes to match index routes with trailing slashes (e.g., `/a` can match `/a/` route), but not vice-versa (e.g., `/a/` cannot match `/a` layout route). This is implemented via the condition `!pathIsIndex || node.kind === SEGMENT_TYPE_INDEX` and is a deliberate design decision to provide better UX by being permissive with missing trailing slashes.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
📚 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:

  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
  • e2e/solid-start/custom-basepath/tests/navigation.spec.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:

  • e2e/vue-start/custom-basepath/tests/navigation.spec.ts
  • e2e/solid-start/custom-basepath/tests/navigation.spec.ts
📚 Learning: 2025-12-17T02:17:55.086Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 6120
File: packages/router-generator/src/generator.ts:654-657
Timestamp: 2025-12-17T02:17:55.086Z
Learning: In `packages/router-generator/src/generator.ts`, pathless_layout routes must receive a `path` property when they have a `cleanedPath`, even though they are non-path routes. This is necessary because child routes inherit the path from their parent, and without this property, child routes would not have the correct full path at runtime.

Applied to files:

  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router

Applied to files:

  • e2e/react-start/custom-basepath/src/routes/users.$userId.tsx
🧬 Code graph analysis (1)
e2e/react-start/custom-basepath/src/routes/users.$userId.tsx (3)
e2e/react-start/custom-basepath/src/routes/users.tsx (1)
  • Route (6-19)
e2e/solid-start/custom-basepath/src/routes/users.$userId.tsx (1)
  • Route (8-25)
e2e/vue-start/custom-basepath/src/routes/users.$userId.tsx (1)
  • Route (8-25)
🔇 Additional comments (1)
e2e/react-start/custom-basepath/src/routes/users.$userId.tsx (1)

3-14: LGTM! Dynamic basepath resolution implemented correctly.

The changes successfully replace static basepath imports with runtime router instance access, which aligns with the PR objective of properly handling basepath during prerendering. The implementation:

  • Correctly retrieves the router instance asynchronously within the loader
  • Constructs the API path using router.options.basepath and router.origin
  • Maintains proper type safety with the User type parameter
  • Preserves existing error handling behavior

This pattern is consistent with the same changes in users.tsx and the corresponding solid-start and vue-start implementations shown in the relevant code snippets.

const url = new URL(request.url)
const basePath = joinPaths(['/', ROUTER_BASEPATH])

if (!url.pathname.startsWith(basePath)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when would the url not start with the basepath? only during prerendering?
if yes, then i dont like the idea of mixing this into each request handling :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when running in vite preview for example the request url is not prepended with the custom basepath while SERVER_FN_BASE is. This is true in both prerender and non-prerender cases as long as the basepath is used.

Note using the express server this is not a problem since it is served correctly hence the check first to see if the the update needs to be applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the looks of the issue description this might be similar cause to #6152

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to assume the users app route will never match the base path. Would it be risky?

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.

Prerender fails when using custom base path - attempts to fetch / instead of configured base path

4 participants