-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refactor(router-core): server-only fast path optimization for interpolatePath #6505
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
refactor(router-core): server-only fast path optimization for interpolatePath #6505
Conversation
📝 WalkthroughWalkthroughAdds a server-aware fast-path to interpolatePath for simple dollar-based templates, threads a Changes
Sequence Diagram(s)(omitted — changes are internal and do not introduce a multi-component sequential flow warranting a diagram) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
View your CI Pipeline Execution ↗ for commit 129d310
☁️ Nx Cloud last updated this comment at |
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: 1
🤖 Fix all issues with AI agents
In `@packages/router-core/src/path.ts`:
- Line 279: The condition currently uses "isServer ?? server" which ignores the
test parameter "server" because "isServer" is always defined; change the
evaluation to prefer the function parameter first (e.g., use "server ??
isServer" or an explicit ternary like "server !== undefined ? server :
isServer") so tests can override the imported "isServer" constant; update the
condition where "isServer ?? server" appears in path.ts accordingly.
interpolatePathis in the hot path of almost everything, so anything we can squeeze out of it might be worth it. This PR proposes a code-path that doesn't requireparseSegment, but only works for 2 kinds of segments:/$paramand/$The lines-of-code to performance ratio isn't good enough for client-side code, so this hot path is gated behind an
isServercheck, and should be fully removed by dead-cod elimination for client-side builds.before

after

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