Skip to content
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

Fix selector-max-compound-selectors with ignoreSelectors for class selectors #7559

Merged
merged 3 commits into from
Mar 19, 2024

Conversation

ybiquitous
Copy link
Member

@ybiquitous ybiquitous commented Mar 14, 2024

Which issue, if any, is this issue related to?

Follow-up to PR #7544
See #7544 (comment)

Is there anything in the PR that needs further explanation?

Note that this change doesn't need any changelog item since the ignoreSelectors option is unreleased.


To fix the bug mentioned on #7544 (comment), this PR slightly changes the algorithm.

New algorithm:

  1. Filter out ignoreSelectors in a selector at once, e.g.
    • .foo .bar > .ignored ➡️ .foo .bar >
    • .foo .bar > .ignored .baz ➡️ .foo .bar > .baz
    • .ignored > .foo .bar ➡️ > .foo .bar
  2. Normalize combinators in the filtered nodes, e.g.
    • .foo .bar
    • .foo .bar > .baz
    • .foo .bar
  3. Count compound selectors in the normalized selector, i.e. {compound_count} = {combinator_count} + 1
    • .foo .bar ➡️ 2
    • .foo .bar > .baz ➡️ 3
    • .foo .bar ➡️ 2

…s selectors

Follow-up to PR #7544
See #7544 (comment)

Note that this change doesn't need any changelog item
since the `ignoreSelectors` option is unreleased.
Copy link

changeset-bot bot commented Mar 14, 2024

⚠️ No Changeset found

Latest commit: debfdec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

lib/rules/selector-max-compound-selectors/index.mjs Outdated Show resolved Hide resolved
lib/rules/selector-max-compound-selectors/index.mjs Outdated Show resolved Hide resolved
@Mouvedia
Copy link
Member

You can ignore my review if you are in a hurry.
What matters is that the behaviour is now consistent.
Hence this can be merged as is.

@Mouvedia
Copy link
Member

Note that this change doesn't need any changelog item since the ignoreSelectors option is unreleased.

You could add & ([@ybiquitous](https://github.com/ybiquitous)) to

- Added: `ignoreSelectors: []` to `selector-max-compound-selectors` ([#7544](https://github.com/stylelint/stylelint/pull/7544)) ([@FloEdelmann](https://github.com/FloEdelmann)).

@Mouvedia Mouvedia dismissed their stale review March 18, 2024 17:11

non-blocking

@ybiquitous
Copy link
Member Author

You could add & ([@ybiquitous](https://github.com/ybiquitous)) to

Don't mind the mention in the changelog since I'll release it. 😅

@ybiquitous ybiquitous merged commit d758d63 into main Mar 19, 2024
14 checks passed
@ybiquitous ybiquitous deleted the follow-up-pr-7544 branch March 19, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants