SVG elements ignored with tagname-lowercase rule
#1795
Conversation
The following SVG elements are ignored: animateMotion animateTransform clipPath feBlend feColorMatrix feComponentTransfer feComposite feConvolveMatrix feDiffuseLighting feDisplacementMap feDistantLight feDropShadow feFlood feFuncA feFuncB feFuncG feFuncR feGaussianBlur feImage feMerge feMergeNode feMorphology feOffset fePointLight feSpecularLighting feSpotLight feTile feTurbulence foreignObject linearGradient radialGradient textPath
coliff
changed the title
tagname-lowercase rule
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1795 +/- ##
===========================================
+ Coverage 96.99% 100.00% +3.00%
===========================================
Files 2 1 -1
Lines 1628 1 -1627
Branches 335 0 -335
===========================================
- Hits 1579 1 -1578
+ Misses 49 0 -49 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of the tagname-lowercase rule flagging valid camelCase SVG element names. The changes introduce a list of SVG tags to ignore by default and enhance the exception mechanism to support regular expressions, which is a good improvement for flexibility. The accompanying updates to tests and documentation are thorough. I have one minor suggestion in src/core/rules/tagname-lowercase.ts to simplify a new helper function, which will improve code consistency.
coliff
deleted the
1794-svg-elements-like-lineargradient-are-case-sensitive-and-should-be-ignored-in-tagname-lowercase-rule
branch
There was a problem hiding this comment.
Pull request overview
This PR fixes the tagname-lowercase rule to properly handle SVG elements with camelCase names (e.g., linearGradient, foreignObject, clipPath). Previously, these valid SVG elements would incorrectly trigger the lowercase rule. The implementation also adds RegExp support to the exceptions array, allowing users to define custom patterns for tag names that should be excluded from the lowercase requirement.
Changes:
- Added comprehensive list of 33 SVG camelCase elements to the default exceptions
- Implemented RegExp support for custom exception patterns (both RegExp objects and string-based patterns)
- Updated TypeScript types to reflect the new array configuration option
- Added basic test coverage for SVG elements and array-based configuration
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/core/rules/tagname-lowercase.ts | Added SVG camelCase element list and testAgainstStringOrRegExp helper function to support RegExp patterns in exceptions |
| src/core/types.ts | Updated type definition for tagname-lowercase rule option to accept boolean | Array<string | RegExp> |
| test/rules/tagname-lowercase.spec.js | Added tests for SVG elements being ignored and array-based configuration |
| website/src/content/docs/rules/tagname-lowercase.mdx | Updated documentation to explain SVG elements are automatically included and provided examples |
| dist/core/rules/tagname-lowercase.js | Compiled JavaScript output from the TypeScript source changes |
| - `true`: enable rule | ||
| - `false`: disable rule | ||
| - `['clipPath', 'data-Test']`: Ignore some tagname name | ||
| - `['clipPath', 'data-Test']`: enable rule except for the given tag names. All SVG camelCase elements are included, for example `linearGradient`, `foreignObject` |
There was a problem hiding this comment.
The documentation does not mention that RegExp patterns are supported in the array configuration. While the implementation supports both RegExp objects and string-based regex patterns (e.g., '/pattern/' or '/pattern/i'), this capability is not documented. Consider adding an example showing RegExp support, similar to the attr-lowercase rule documentation, to help users understand this feature.
| - `['clipPath', 'data-Test']`: enable rule except for the given tag names. All SVG camelCase elements are included, for example `linearGradient`, `foreignObject` | |
| - `['clipPath', 'data-Test']`: enable rule except for the given tag names. All SVG camelCase elements are included, for example `linearGradient`, `foreignObject`. The array can include literal tag names as strings, `RegExp` objects, or string-based regex patterns such as `'/^svg[A-Z]/'` or `'/^data-/i'`, for example: | |
| - `['clipPath', /data-[A-Z]\w+/, '/^my-/i']` |
| * testAgainstStringOrRegExp | ||
| * | ||
| * @param value string to test | ||
| * @param comparison raw string or regex string | ||
| */ | ||
| function testAgainstStringOrRegExp(value: string, comparison: string | RegExp) { | ||
| // If it's a RegExp, test directly | ||
| if (comparison instanceof RegExp) { | ||
| return comparison.test(value) | ||
| ? { match: value, pattern: comparison } | ||
| : false | ||
| } | ||
|
|
||
| // Check if it's RegExp in a string | ||
| const firstComparisonChar = comparison[0] | ||
| const lastComparisonChar = comparison[comparison.length - 1] | ||
| const secondToLastComparisonChar = comparison[comparison.length - 2] | ||
|
|
||
| const comparisonIsRegex = | ||
| firstComparisonChar === '/' && | ||
| (lastComparisonChar === '/' || | ||
| (secondToLastComparisonChar === '/' && lastComparisonChar === 'i')) | ||
|
|
||
| const hasCaseInsensitiveFlag = comparisonIsRegex && lastComparisonChar === 'i' | ||
|
|
||
| // If so, create a new RegExp from it | ||
| if (comparisonIsRegex) { | ||
| const valueMatches = hasCaseInsensitiveFlag | ||
| ? new RegExp(comparison.slice(1, -2), 'i').test(value) | ||
| : new RegExp(comparison.slice(1, -1)).test(value) | ||
|
|
||
| return valueMatches | ||
| } | ||
|
|
||
| // Otherwise, it's a string. Do a strict comparison | ||
| return value === comparison |
There was a problem hiding this comment.
The testAgainstStringOrRegExp function is duplicated from attr-lowercase.ts (lines 77-114). This creates code duplication and increases maintenance burden. Consider extracting this function to a shared utility module to avoid duplication and ensure consistency across both rules.
| * testAgainstStringOrRegExp | |
| * | |
| * @param value string to test | |
| * @param comparison raw string or regex string | |
| */ | |
| function testAgainstStringOrRegExp(value: string, comparison: string | RegExp) { | |
| // If it's a RegExp, test directly | |
| if (comparison instanceof RegExp) { | |
| return comparison.test(value) | |
| ? { match: value, pattern: comparison } | |
| : false | |
| } | |
| // Check if it's RegExp in a string | |
| const firstComparisonChar = comparison[0] | |
| const lastComparisonChar = comparison[comparison.length - 1] | |
| const secondToLastComparisonChar = comparison[comparison.length - 2] | |
| const comparisonIsRegex = | |
| firstComparisonChar === '/' && | |
| (lastComparisonChar === '/' || | |
| (secondToLastComparisonChar === '/' && lastComparisonChar === 'i')) | |
| const hasCaseInsensitiveFlag = comparisonIsRegex && lastComparisonChar === 'i' | |
| // If so, create a new RegExp from it | |
| if (comparisonIsRegex) { | |
| const valueMatches = hasCaseInsensitiveFlag | |
| ? new RegExp(comparison.slice(1, -2), 'i').test(value) | |
| : new RegExp(comparison.slice(1, -1)).test(value) | |
| return valueMatches | |
| } | |
| // Otherwise, it's a string. Do a strict comparison | |
| return value === comparison | |
| * Normalize a string or RegExp into a usable matcher. | |
| * | |
| * Supports: | |
| * - RegExp instances | |
| * - Regex-like strings such as "/pattern/" or "/pattern/i" | |
| * - Plain strings (falls back to strict equality) | |
| */ | |
| function normalizeStringOrRegExp( | |
| comparison: string | RegExp | |
| ): RegExp | string { | |
| if (comparison instanceof RegExp) { | |
| return comparison | |
| } | |
| // Attempt to interpret strings like "/pattern/flags" as regular expressions. | |
| if (comparison.length >= 2 && comparison[0] === '/') { | |
| const lastSlashIndex = comparison.lastIndexOf('/') | |
| if (lastSlashIndex > 0) { | |
| const pattern = comparison.slice(1, lastSlashIndex) | |
| const flags = comparison.slice(lastSlashIndex + 1) | |
| // Only accept valid JS RegExp flags; otherwise, treat as a plain string. | |
| if (/^[gimsuy]*$/.test(flags)) { | |
| try { | |
| return new RegExp(pattern, flags) | |
| } catch { | |
| // If the pattern is invalid, fall through and treat as a plain string. | |
| } | |
| } | |
| } | |
| } | |
| return comparison | |
| } | |
| /** | |
| * Test a value against a string, regex-like string, or RegExp. | |
| * | |
| * @param value string to test | |
| * @param comparison raw string, regex-like string, or RegExp | |
| */ | |
| function testAgainstStringOrRegExp( | |
| value: string, | |
| comparison: string | RegExp | |
| ): boolean { | |
| const matcher = normalizeStringOrRegExp(comparison) | |
| if (matcher instanceof RegExp) { | |
| return matcher.test(value) | |
| } | |
| return value === matcher |
| it('Set to array list should not result in an error', () => { | ||
| const code = '<CustomComponent /><AnotherCamelCase />' | ||
| ruleOptions[ruleId] = ['CustomComponent', 'AnotherCamelCase'] | ||
| const messages = HTMLHint.verify(code, ruleOptions) | ||
| expect(messages.length).toBe(0) | ||
| ruleOptions[ruleId] = true | ||
| }) |
There was a problem hiding this comment.
The test coverage for RegExp support is incomplete. While the implementation supports RegExp patterns (both as RegExp objects and as string patterns like '/pattern/'), there are no tests for this functionality. The attr-lowercase rule has comprehensive RegExp tests (see test/rules/attr-lowercase.spec.js lines 48-60) including tests for RegExp objects and string-based regex patterns. Add similar test cases to ensure the RegExp functionality works correctly.
The following SVG elements are ignored: