feat(eslint-plugin-router): route param name rule #6473
Merged
+734
−0
Conversation
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
|
View your CI Pipeline Execution ↗ for commit ed276e6
☁️ Nx Cloud last updated this comment at |
Contributor
📝 WalkthroughWalkthroughAdds a new ESLint rule "route-param-names" that validates route parameter identifiers in TanStack Router path strings, along with constants, utilities for extracting/validating params, and comprehensive tests covering valid/invalid cases and multiple invocation shapes. Changes
Sequence Diagram(s)sequenceDiagram
participant Source as "Source File / AST"
participant ESLint as "ESLint Rule\nroute-param-names"
participant Utils as "Param Utils"
participant Reporter as "ESLint Reporter"
Note over Source,ESLint: Linter visits project files
Source->>ESLint: Detect router import/use and calls
ESLint->>ESLint: Determine call shape (path prop, first-arg, file-based, curried)
ESLint->>Utils: Extract path string(s) from literal/template/filepath
Utils->>Utils: extractParamsFromPath / isValidParamName / getInvalidParams
Utils-->>ESLint: Return invalid params list
ESLint->>Reporter: Report `invalidParamName` for each invalid param (with paramName)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/eslint-plugin-router/src/rules/route-param-names/route-param-names.rule.ts`:
- Around line 46-60: The getStringLiteralValue function treats empty template
literals inconsistently because it checks node.quasis[0]?.value.cooked by
truthiness; change that check to explicitly test for null/undefined (e.g.,
cooked != null) so an empty string ("") is returned rather than null. Update the
TemplateLiteral branch (AST_NODE_TYPES.TemplateLiteral and
node.quasis[0].value.cooked) to capture the cooked value into a variable and
return it when it's not null/undefined, preserving existing behavior for
non-string Literal nodes.
- Around line 74-89: The rule currently only recognizes Identifier keys named
'path' in object arguments; update the property-key check in the createRoute
branch (where pathAsPropertySet is used and
reportInvalidParams/getStringLiteralValue are called) to also accept
string-literal keys. Replace the single check "prop.key.type ===
AST_NODE_TYPES.Identifier && prop.key.name === 'path'" with a condition that
allows either an Identifier named 'path' or a Literal whose value === 'path'
(i.e., check prop.key.type === AST_NODE_TYPES.Literal && prop.key.value ===
'path') before calling getStringLiteralValue and reportInvalidParams.
packages/eslint-plugin-router/src/rules/route-param-names/route-param-names.rule.ts
Show resolved
Hide resolved
packages/eslint-plugin-router/src/rules/route-param-names/route-param-names.rule.ts
Outdated
Show resolved
Hide resolved
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.
Problem
We used to extract all route params with regular expressions. This would intrinsically enforce a very strict format for the parameter names:
[a-zA-Z$][a-zA-Z0-9$]*, i.e. a valid javascript variable name.When we rewrote the matcher (#5722), regular params were not matched by a regex anymore (
$param) but more complex param syntaxes were still (e.g.prefix{-$optional}suffix). So we were only half enforcing the parameter name, which is not ideal.Now we would like to move completely off of regexes (#6470) and we should probably enforce some limitations on parameter names, or we risk preventing future evolutions of the syntax if parameter names are too lax, and users start using characters we would eventually like to assign special meaning to.
Solution
This PR proposes we add an ESLint rule so that such invalid parameter names can be detected in IDE and in CI.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.