perf(server): avoid URL construction for pathname/query extraction#91561
Open
benfavre wants to merge 2 commits intovercel:canaryfrom
Open
perf(server): avoid URL construction for pathname/query extraction#91561benfavre wants to merge 2 commits intovercel:canaryfrom
benfavre wants to merge 2 commits intovercel:canaryfrom
Conversation
Collaborator
|
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
Replace `new URL(req.url, 'http://n')` with cheap string operations in hot server paths where only the pathname or a single query parameter is needed. `new URL()` must parse a full spec-compliant URL even when the input is just a relative path like `/foo?bar=1`. Adds `getPathnameFromUrl()` and `getQueryParamFromUrl()` utilities that use `indexOf`/`substring` instead. These are 5-10x cheaper for the common case of relative URLs in Node.js `req.url`. Changed call sites: - `app-render.tsx`: 2 ISR status calls (dev-only, but still worth it) - `base-server.ts`: invokePath pathname extraction (every request with invokePath) - `base-server.ts`: `_rsc` cache-busting param lookup (every RSC request) Inspired by: https://tanstack.com/blog/tanstack-start-5x-faster-ssr Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
d4de73a to
7ff62bd
Compare
When _rsc is sent without a value (e.g. ?_rsc instead of ?_rsc=hash), getQueryParamFromUrl returned null instead of '' (empty string). This caused the RSC hash validation to see expectedHash='' !== actualHash=null, triggering an infinite 307 redirect loop. Now matches URLSearchParams behavior: value-less params return ''.
Contributor
Author
Test Verification
All tests run on the |
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
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.
Summary
new URL(req.url, 'http://n')with lightweight string operations in hot server paths where only the pathname or a single query parameter is neededgetPathnameFromUrl()andgetQueryParamFromUrl()utilities usingindexOf/substringinstead of full URL parsingapp-render.tsxandbase-server.tsBackground
The TanStack Start team identified
new URL()as a significant contributor to SSR self-time when profiling their 5x throughput improvement (blog post). In Node.js,req.urlis always a relative URL like/path?query=1, so constructing a fullURLobject just to read.pathnameor a single.searchParamsentry is unnecessarily expensive.Changed call sites
app-render.tsx(L2121)app-render.tsx(L2400)base-server.ts(L1518)base-server.ts(L2094)_rsccache-busting param lookupThe
get-layer-assets.tsxnew URL(ctx.assetPrefix)call was intentionally left unchanged — it parses a fully qualified URL to extract.origin, which genuinely needs URL construction, and it's already inside a try/catch for the fallback case.Test plan
new URL().pathnameandnew URL().searchParams.get()for relative URLs🤖 Generated with Claude Code