-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(start-server-core): return 404 for API routes without GET handler #5758
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughNormalizes HTTP handler keys, maps HEAD to GET when appropriate, adds ANY-handler awareness, and returns 404 responses when no matching handler exists. Adds unit tests and test mocks, and wires mock modules into the package's Vite test config. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as createStartHandler
participant Lookup as Handler Lookup (normalized)
participant HandlerFn as Selected Handler
participant Response
Client->>Server: HTTP Request (method, path)
Server->>Lookup: Normalize handlers keys → UPPERCASE
Server->>Lookup: Lookup by method
alt method is HEAD and no HEAD but GET exists
Lookup-->>Server: select GET handler
else no method -> try ANY
Lookup-->>Server: select ANY handler (if present)
end
alt Handler found
Server->>HandlerFn: invoke handler
HandlerFn-->>Response: handler response
Response-->>Client: 200/handler-defined
else No handler
alt request method is HEAD
Server->>Response: 404 with empty JSON content-type
else
Server->>Response: 404 with { error: "Not Found" } JSON body
end
Response-->>Client: 404
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/{*-start,start-*}/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (9)📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-11-02T16:16:24.898ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-10-01T18:31:35.420ZApplied to files:
🧬 Code graph analysis (1)packages/start-server-core/tests/createStartHandler.test.ts (2)
🔇 Additional comments (3)
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 |
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/start-server-core/src/createStartHandler.ts(1 hunks)packages/start-server-core/tests/createStartHandler.test.ts(1 hunks)packages/start-server-core/tests/mocks/injected-head-scripts.ts(1 hunks)packages/start-server-core/tests/mocks/router-entry.ts(1 hunks)packages/start-server-core/tests/mocks/start-entry.ts(1 hunks)packages/start-server-core/tests/mocks/start-manifest.ts(1 hunks)packages/start-server-core/vite.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/start-server-core/tests/mocks/injected-head-scripts.tspackages/start-server-core/tests/mocks/start-entry.tspackages/start-server-core/tests/mocks/router-entry.tspackages/start-server-core/tests/createStartHandler.test.tspackages/start-server-core/tests/mocks/start-manifest.tspackages/start-server-core/src/createStartHandler.tspackages/start-server-core/vite.config.ts
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-server-core/tests/mocks/injected-head-scripts.tspackages/start-server-core/tests/mocks/start-entry.tspackages/start-server-core/tests/mocks/router-entry.tspackages/start-server-core/tests/createStartHandler.test.tspackages/start-server-core/tests/mocks/start-manifest.tspackages/start-server-core/src/createStartHandler.tspackages/start-server-core/vite.config.ts
🧠 Learnings (4)
📚 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:
packages/start-server-core/tests/mocks/router-entry.tspackages/start-server-core/tests/createStartHandler.test.tspackages/start-server-core/tests/mocks/start-manifest.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
packages/start-server-core/tests/mocks/router-entry.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:
packages/start-server-core/tests/mocks/router-entry.tspackages/start-server-core/tests/mocks/start-manifest.tspackages/start-server-core/vite.config.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
Applied to files:
packages/start-server-core/vite.config.ts
🧬 Code graph analysis (2)
packages/start-server-core/tests/createStartHandler.test.ts (2)
packages/start-server-core/src/createStartHandler.ts (1)
createStartHandler(54-341)packages/start-server-core/tests/mocks/router-entry.ts (1)
currentHandlers(3-3)
packages/start-server-core/src/createStartHandler.ts (1)
packages/start-client-core/src/serverRoute.ts (1)
RouteMethod(427-435)
🪛 ESLint
packages/start-server-core/tests/mocks/router-entry.ts
[error] 1-1: All imports in the declaration are only used as types. Use import type.
(@typescript-eslint/consistent-type-imports)
[error] 3-3: 'currentHandlers' is never reassigned. Use 'const' instead.
(prefer-const)
packages/start-server-core/tests/createStartHandler.test.ts
[error] 1-1: Member 'expect' of the import declaration should be sorted alphabetically.
(sort-imports)
packages/start-server-core/vite.config.ts
[error] 7-7: Expected 1 empty line after import statement not followed by another import.
(import/newline-after-import)
[error] 7-7: path import should occur before import of vitest/config
(import/order)
[error] 7-7: Prefer node:path over path.
(node/prefer-node-protocol)
🔇 Additional comments (9)
packages/start-server-core/tests/mocks/start-manifest.ts (1)
1-10: LGTM!The mock manifest structure is minimal and appropriate for testing the server handler behavior.
packages/start-server-core/tests/mocks/start-entry.ts (1)
1-7: LGTM!The mock start entry provides minimal defaults suitable for testing the start handler.
packages/start-server-core/tests/mocks/injected-head-scripts.ts (1)
1-1: LGTM!The empty string mock for injected head scripts is appropriate for isolated testing.
packages/start-server-core/src/createStartHandler.ts (2)
383-392: LGTM! Correct HTTP method normalization and HEAD mapping.The normalization ensures case-insensitive method matching, and the HEAD→GET mapping follows HTTP semantics correctly.
395-408: LGTM! Proper 404 handling for routes without matching handlers.The logic correctly returns 404 JSON responses when no handler exists for the requested method and no ANY fallback is present. HEAD requests appropriately return an empty body with 404 status per HTTP standards.
packages/start-server-core/vite.config.ts (1)
15-23: LGTM! Test mock aliases properly configured.The path aliases enable isolated testing of the start handler by redirecting module imports to test mocks.
packages/start-server-core/tests/createStartHandler.test.ts (2)
19-33: LGTM! Core test validates 404 JSON response.This test correctly validates the main fix: routes without a GET handler now return 404 JSON instead of 200 OK with the HTML shell.
35-80: LGTM! Comprehensive method resolution coverage.The test suite thoroughly validates POST handling, HEAD→GET mapping, HEAD 404 behavior, and ANY handler fallback scenarios.
packages/start-server-core/tests/mocks/router-entry.ts (1)
5-30: LGTM! Minimal router mock for testing.The fake router provides all necessary properties to exercise the handler resolution logic without requiring the full router implementation.
Fixes #5473:
API routes without a GET handler returned 200 + SPA shell instead of 404 JSON.
Previously:
/api/xyzwhere only aPOSThandler existed returned 200 OK with the HTML shell.Now:
ANYfallback) correctly return 404 JSON ({ "error": "Not Found" }).HEADrequests behave correctly (empty 404 if noGEThandler).Summary by CodeRabbit
New Features
Bug Fixes
Tests