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.
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.