feat(matcher): hint at params: {} workaround in discarded params warning#2689
Conversation
…rning Closes #1617 Co-authored-by: shanliuling <1362913430@qq.com>
✅ Deploy Preview for vue-router canceled.
|
📝 WalkthroughWalkthroughExtended warning validation for catch-all routes with named redirects. Added test cases verifying that invalid params trigger appropriate hints when inherited during matching, and refined the warning message to suggest using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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 |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2689 +/- ##
=======================================
Coverage 85.91% 85.92%
=======================================
Files 88 88
Lines 10071 10078 +7
Branches 2311 2316 +5
=======================================
+ Hits 8653 8660 +7
Misses 1407 1407
Partials 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router/src/matcher/index.ts`:
- Around line 271-279: The heuristic that sets isInherited is too loose; replace
the current some(...) check with a stricter predicate that returns true only
when every discarded param key came from the previous location and (ideally) its
value matches the previous value. Concretely, update the isInherited computation
(the variable used before the warn call) to use invalidParams.every(name => name
in currentLocation.params && currentLocation.params[name] === /* the value being
discarded */) so that the catch-all redirect hint is only appended when all
discarded params were truly inherited from currentLocation.params (matching keys
and values).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3149cb76-381b-46e6-b2c3-703469611ca7
📒 Files selected for processing (2)
packages/router/__tests__/warnings.spec.tspackages/router/src/matcher/index.ts
| const isInherited = | ||
| !matcher!.keys.length && | ||
| invalidParams.some(name => name in currentLocation.params) | ||
| warn( | ||
| `Discarded invalid param(s) "${invalidParams.join( | ||
| '", "' | ||
| )}" when navigating. See https://github.com/vuejs/router/blob/main/packages/router/CHANGELOG.md#414-2022-08-22 for more details.` | ||
| )}" when navigating.` + | ||
| (isInherited | ||
| ? ` If you are using a catch-all route with a named redirect, pass an empty \`params\` object: \`redirect: { name: '...', params: {} }\`.` |
There was a problem hiding this comment.
Tighten the inherited-param heuristic before showing the workaround hint.
some(name => name in currentLocation.params) also matches explicitly supplied invalid params whenever the current route happens to have the same param key, e.g. navigating from /:id to a static named route with { params: { id: 'next' } }. That would show the catch-all redirect hint even though the params were not inherited.
Consider requiring all discarded params to come from the current location and, ideally, matching their values before appending the hint.
Possible narrowing
- const isInherited =
- !matcher!.keys.length &&
- invalidParams.some(name => name in currentLocation.params)
+ const locationParams = location.params || {}
+ const isInherited =
+ !matcher!.keys.length &&
+ invalidParams.every(
+ name =>
+ name in currentLocation.params &&
+ locationParams[name] === currentLocation.params[name]
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router/src/matcher/index.ts` around lines 271 - 279, The heuristic
that sets isInherited is too loose; replace the current some(...) check with a
stricter predicate that returns true only when every discarded param key came
from the previous location and (ideally) its value matches the previous value.
Concretely, update the isInherited computation (the variable used before the
warn call) to use invalidParams.every(name => name in currentLocation.params &&
currentLocation.params[name] === /* the value being discarded */) so that the
catch-all redirect hint is only appended when all discarded params were truly
inherited from currentLocation.params (matching keys and values).
Closes #1617. Supersedes #2662 (credits original author via
Co-authored-by).Appends a hint to the dev-only "Discarded invalid param(s)" warning when the params were inherited from the current location and the target matcher has no keys — the classic catch-all → named-redirect case.
Tests: added a matcher-level test for the hint, and a router-level test asserting the
params: {}workaround silences the warning entirely.Summary by CodeRabbit
Tests
Bug Fixes