Skip to content

Create group-attrs rule #383

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

reteps
Copy link

@reteps reteps commented Jul 2, 2025

Description

Creates a rule for grouping related attributes together. Essentially a more relaxed version of @html-eslint/sort-attrs, that gives more flexibility for semantic grouping.

Disclaimer:

  • The test suite and rule was written by claude-4-sonnet, I am in the process of reviewing the code as well as the attribute set.

@yeonjuan
Copy link
Owner

yeonjuan commented Jul 3, 2025

Hi @reteps thanks for the suggestion, how about providing this feature as an option for sort-attrs? I think it would be nice to avoid conflicting rules as much as possible.

@reteps
Copy link
Author

reteps commented Jul 3, 2025

@yeonjuan that sounds great! I'm happy to make that change if you suggest how you think the two should work together.

@reteps
Copy link
Author

reteps commented Jul 4, 2025

I'm thinking that the default sort-attrs rule essentially corresponds to one group in the group-attrs rule containing all attributes, but I'm not best sure how to combine the two.

@yeonjuan
Copy link
Owner

yeonjuan commented Jul 6, 2025

@reteps

that sounds great! I'm happy to make that change if you suggest how you think the two should work together.

Awesome!

I think the rule should take grouping into account when the group option is provided.If the group option is not specified, the rule should behave as it currently does.
We might be able to use the oneOf schema for the rule options:

schema: [
  {
       oneOf: [
            { /* priority option */},
            { /* group option */ }
       ]
   }
]

And I think regex support might be necessary for attributes patterns like data-*.

"sort-attrs": ["error": {
   "group": [
       ["id", "type"],
       ["^aria-", ... ]
       ["^data-", ...]
    ]
  }
}]

think the DEFAULT_GROUPS defined in this PR is very useful.
What do you think about allowing users to apply it like this?
If the group option is set to a string (e.g., related), we can apply DEFAULT_GROUPS accordingly.

"sort-attrs" ["error", { group: "related" }]

@reteps
Copy link
Author

reteps commented Jul 7, 2025

I want sort-attrs to correspond to this rule:

groups: [
[...priority, '/.*/']
]

One concern I have is that the sorting in sort-attrs is best effort: if it is separated by a template parameter, each 'subgroup' split by a template param is sorted. This conflicts with my rule, which is a violation if separated.

<foo min="1" {{#thing1}} max="2" {{/thing1}}
<foo max="2" {{#thing1}} min="1" {{/thing1}}

We can't automatically fix this behavior, and shouldn't omit a wrongOrder violation since it can't be fixed.

One issue I have is that groups should not be best effort for templating:

<foo min="1" {{#thing1}} max="2" class="foo" {{/thing1}} {{^thing2}} max="3" class="foo" {{/thing2}}
<foo min="1" {{#thing1}} max="2" class="foo" {{/thing1}} {{^thing2}} class="foo" max="3" {{/thing2}}

I can't think of how to handle this case when min and max should be grouped. I think that some reasonable behavior is that grouping violations won't be omitted if they are separated by a template parameter.

I'm not sure how best to handle this:

  • Error codes that differentiate between e.g. wrongOrder and wrongOrderTemplate (if it contains a template param)
  • Not omitting grouping violations and falling back to best-effort sorting for templates
  • Configurable behavior

I'm thinking that the sorting should be best effort, and grouping violations are ignored if moving the attribute could have different semantic meaning (e.g. the field is separated by a templated parameter).

@yeonjuan any thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants