fix(vite): respect tanstack start route manifest in dev#4274
Draft
pi0 wants to merge 2 commits into
Draft
Conversation
Consult `globalThis.TSS_ROUTES_MANIFEST` (published by `@tanstack/start-plugin-core`) during dev so URLs matching a Start route count as explicit and flow through nitro's middleware (where Start's devApp handles them) instead of the asset/page heuristic.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughDependency versions for TanStack packages are updated in documentation and examples. The dev middleware's explicit-route detection is extended to recognize TanStack Start routes from a manifest, bypassing the asset/route ambiguity heuristic for paths matching compiled Start route patterns. ChangesTanStack Start Routes Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/build/vite/dev.ts (1)
310-314: ⚡ Quick winRemove narrative inline comments in this helper block.
Line 310–314 adds explanatory comments that conflict with the repo rule for TS/JS files.
Proposed cleanup
-// ---- TanStack Start route manifest probe ---- -// Start's router plugin publishes a filesystem-routes manifest on globalThis for sibling Vite -// plugins to consume (see `@tanstack/start-plugin-core` `routes-manifest-plugin`). When present, -// any URL matching one of Start's routes is treated as explicit so it flows through Nitro's -// middleware (where Start's devApp gets first shot) instead of the asset/page heuristic. +// ---- TanStack Start route manifest probe ----As per coding guidelines: "Do not add comments explaining what the line does unless prompted".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/build/vite/dev.ts` around lines 310 - 314, The added narrative comment block beginning with "---- TanStack Start route manifest probe ----" should be removed to comply with the TS/JS commenting guideline; locate the helper block in src/build/vite/dev.ts that publishes the Start router manifest to globalThis (the surrounding comments referencing `@tanstack/start-plugin-core` and the filesystem-routes manifest) and delete those explanatory lines, leaving only minimal, if any, concise tags or none at all so the code itself remains self-descriptive.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/build/vite/dev.ts`:
- Around line 310-314: The added narrative comment block beginning with "----
TanStack Start route manifest probe ----" should be removed to comply with the
TS/JS commenting guideline; locate the helper block in src/build/vite/dev.ts
that publishes the Start router manifest to globalThis (the surrounding comments
referencing `@tanstack/start-plugin-core` and the filesystem-routes manifest)
and delete those explanatory lines, leaving only minimal, if any, concise tags
or none at all so the code itself remains self-descriptive.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa5b5816-bc71-4d45-9cd6-6631eead5c0c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
docs/4.examples/vite-ssr-tss-react.mdexamples/vite-ssr-tss-react/package.jsonsrc/build/vite/dev.ts
commit: |
pi0
changed the title
pi0
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the TanStack Start regression reported in TanStack/router#7403 (specifically this comment):
<img src="/api/.../thumbnail">returns404 text/htmlin dev, even though hitting the URL directly works.Why #4272 was not enough
#4272 made the dev middleware consult
nitro.routing.routesto decide whether a request is an explicit user route. But TanStack Start API routes live atsrc/routes/api/**.tsand register viaviteDevServer.middlewares.use(...), so they never appear innitro.routing.routes— that table only contains the SSR catch-all/**. As a result,isExplicitRouteis alwaysfalsefor Start API routes, and any image-context request (Sec-Fetch-Dest: image) falls into the asset branch and is claimed by Vite.Fix
globalThis.TSS_ROUTES_MANIFEST(published by@tanstack/start-plugin-core'sroutes-manifest-pluginfor sibling Vite plugins) so URLs matching a TanStack Start filesystem route are treated as explicit.devAppgets first shot) instead of being classified by the asset/page heuristic.@tanstack/react-router,@tanstack/react-router-devtools, and@tanstack/react-startin thevite-ssr-tss-reactexample to versions that publish the manifest (separate commit).Verification
Verified locally against
examples/vite-ssr-tss-react(Start app whose API routes are not visible to nitro), with the fix toggled on/off:GET /api/test(plain)GET /api/test+Sec-Fetch-Dest: imageGET /api/files/42+Sec-Fetch-Dest: image(parametrized\$id)GET /api/files/foo.png+Sec-Fetch-Dest: image(.pngin path param)GET /api/nope/x+ image (unmatched)GET /favicon.ico+ image (real asset)Caveat
The probe relies on the undocumented
globalThis.TSS_ROUTES_MANIFESTcontract. If Start renames/removes it, the fallback silently disables and we regress to the #4272 behavior. Worth coordinating with the Start team so the contract is intentional.