Skip to content

fix: Improve attr-value-no-duplication logic #1664

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 17, 2025
+37 −24
Merged

fix: Improve attr-value-no-duplication logic #1664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 23 additions & 21 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions src/core/rules/attr-value-no-duplication.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ export default {
for (let i = 0, l = attrs.length; i < l; i++) {
attr = attrs[i]

// Skip style and media attributes entirely
// Skip content, media, and style attributes entirely
if (
attr.name.toLowerCase() === 'style' ||
attr.name.toLowerCase() === 'media'
attr.name.toLowerCase() === 'content' ||
attr.name.toLowerCase() === 'media' ||
attr.name.toLowerCase() === 'style'
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While the current conditional logic works, using an array with the includes() method for checking skipped attributes can enhance readability and maintainability, especially if the list of attributes to skip grows in the future.

Consider defining the list of attributes to skip (e.g., ['content', 'media', 'style']) as a constant at a higher scope (like the module level or within the exported object) for better organization and reusability.

          // Consider defining this array as a constant at a higher scope for better maintainability.
          if (['content', 'media', 'style'].includes(attr.name.toLowerCase()))

) {
continue
}
Expand Down
7 changes: 7 additions & 0 deletions test/rules/attr-value-no-duplication.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,11 @@ describe(`Rules: ${ruleId}`, () => {
const messages = HTMLHint.verify(code, ruleOptions)
expect(messages.length).toBe(0)
})

it('Content attribute should be skipped entirely', () => {
const code =
'<meta name="keywords" content="HTML, CSS, JavaScript, HTML, responsive design">'
const messages = HTMLHint.verify(code, ruleOptions)
expect(messages.length).toBe(0)
})
})
2 changes: 2 additions & 0 deletions website/src/content/docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ An example configuration file (with all rules disabled):
"attr-sorted": false,
"attr-unsafe-chars": false,
"attr-value-double-quotes": false,
"attr-value-no-duplication": false,
"attr-value-not-empty": false,
"attr-value-single-quotes": false,
"attr-whitespace": false,
Expand Down Expand Up @@ -74,6 +75,7 @@ An example configuration file (with all rules disabled):
"spec-char-escape": false,
"src-not-empty": false,
"style-disabled": false,
"tag-no-obsolete": false,
"tag-pair": false,
"tag-self-close": false,
"tagname-lowercase": false,
Expand Down
1 change: 1 addition & 0 deletions website/src/content/docs/getting-started.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { Tabs, TabItem } from '@astrojs/starlight/components';
"meta-viewport-require": true,
"spec-char-escape": true,
"src-not-empty": true,
"tag-no-obsolete": true,
"tag-pair": true,
"tagname-lowercase": true,
"title-require": true
Expand Down