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

Refactor *-specificity tests and documentation to align with standard CSS #7688

Conversation

romainmenke
Copy link
Member

@romainmenke romainmenke commented May 11, 2024

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

Closes #7495

Is there anything in the PR that needs further explanation?

As explained in the linked issue :global() and :local() are non-standard functional pseudo classes that have these properties:

  • have no specificity of their own
  • do not support selector lists, only a single selector at a time (i.e. :local(a), :local(b) and not :local(a, b))

But they were used in tests to verify the ignoreSelectors feature.
This feature acts like a hole punch. Only the ignored node is skipped when calculating specificity. The child nodes are still part of the equation.

Given that :global and :local have no specificity of their own and do not allow lists, it isn't ideal that we are using these to test ignoreSelectors. All of the complexity for ignoreSelectors is in calculating the specificity of lists of child selectors.

This change replaces :global and :local with :is and :has because these do support lists of selectors (e.g. :is(a, b)).

It also adds tests with :host because :host does have its own specificity.

This gives us meaningful coverage for ignoreSelectors in a way that aligns with standard CSS.


The goal of this change is to have coverage in a way that makes it easier to change the implementation of this rule.

The goal is not to make breaking changes that are noticeable to end users, only to make sure that we are testing the right things in the right way.


Side note:

Calculating correct specificity for :global, :local or any other selector is purely an upstream concern in @csstools/selector-specificity. So I think it is fine if we don't have explicit coverage for these (non-standard) selectors.

We need coverage for the rule and for ignoreSelectors here, but not for the specificity of any individual selector.

Copy link

changeset-bot bot commented May 11, 2024

⚠️ No Changeset found

Latest commit: 4d38248

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

@Mouvedia
Copy link
Member

Can you update

code: ':global(.buildResultsSummaryTable) span {} :global(.buildResultsSummaryTable) span:nth-child(2) {}',

so that it uses either :host(), :is() or :has() instead?

@romainmenke romainmenke changed the title Refactor selector-max-specificity tests to align with standard CSS Refactor *-specificity tests and documentation to align with standard CSS May 11, 2024
…atch-reality--frank-pygmy-marmoset-f3bfe759f6
Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

Thanks, I like this change. Can you fix the test failures?

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM 👍🏼

@stylelint stylelint deleted a comment from romainmenke May 11, 2024
@romainmenke
Copy link
Member Author

Thank you for the reviews, in particular from @Mouvedia 🙇

@romainmenke romainmenke merged commit 5cfceac into main May 11, 2024
17 checks passed
@romainmenke romainmenke deleted the refactor-selector-max-specificity-tests-to-match-reality--frank-pygmy-marmoset-f3bfe759f6 branch May 11, 2024 12:49
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.

Fix selector-max-specificity README examples of the ignoreSelectors secondary option
3 participants