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

Rule options where users set selectors #7557

Open
romainmenke opened this issue Mar 13, 2024 · 11 comments
Open

Rule options where users set selectors #7557

romainmenke opened this issue Mar 13, 2024 · 11 comments
Labels
status: needs discussion triage needs further discussion

Comments

@romainmenke
Copy link
Member

Opening a new issue to discuss this separate from any individual bug report or refactor because I am first looking for some guidance on which direction is more favorable.


Examples :

  • rule-selector-property-disallowed-list: disallow specific properties when a selector matches a pattern
  • selector-disallowed-list: disallow a list of selectors

It used to be relatively fine to treat selectors as strings and to use tools like regex but I don't think this works well anymore.

With nesting it is ambiguous if users want to match the resolved or unresolved string.
Even when they agree they want to match the resolved string they might not agree on the method (sass like or following the spec).

But that is not the only bit of complexity.

:not() is a negation, so should :not(input) { width: 100% } error when disallowing width for /input/ ?

:has() matches elements when other elements match a selector, not when it itself is something. Same as before, should this error :has(input) { width: 100% }?


The underlying issue is that string matching can only go so far and that the outcome will always be surprising to some users.

I personally think that there should be an effort to use a more semantic approach.
Alternative we could embrace that these rules operate on strings and that they are agnostic of parent rules, nesting, selector matching mechanics,...

Any rule that enforces something onto tags, classnames, types, attributes will be unaffected by this. They already target specific selector parts.

A concrete example is : #7488

This rule treats selectors as strings and doesn't consider their semantic meaning or which elements they would match.

Updating that rule so that nested selectors are resolved would be surprising as it can radically change what does or doesn't give errors.

Note

Keep in mind that nesting has an implicit &
so .foo { input {} } would resolve to .foo input and would no longer match "input" as a plugin option.
That alone would be a breaking change.

But the same rule would also be really helpful if it was implemented differently.
It could look at selectors in the same way browsers do and only disallow properties when the right most compound selector would be a semantic match.


I think that handling this case by case always favors the current bug reporter as we all want to help people :) This might mean that we accidentally end up somewhere that is worse instead of better.

So I am looking for input here as I don't know what others think is the better solution here.

@romainmenke romainmenke added the status: needs discussion triage needs further discussion label Mar 13, 2024
@ybiquitous
Copy link
Member

@romainmenke Thank you for opening the discussion. I think your concern absolutely makes sense. With the modern syntaxes or components (e.g. nested selectors, :not(), etc.), I agree that string-based or regex-based filtering has been gradually confusing.

At this point, the ignoring behavior of each rule depends on the selector parser mostly.

@Mouvedia Mouvedia mentioned this issue Mar 14, 2024
5 tasks
@Mouvedia
Copy link
Contributor

Mouvedia commented Mar 14, 2024

With nesting it is ambiguous if users want to match the resolved or unresolved string.

To make an informed choice we need a list of all the rules which are affected by the & selector that have at least one option that accept a regex. Once that's compiled we can either:

  1. choose one way over the other and act accordingly (major change)
  2. document the statu quo using > [!NOTE] and eventually fix rules that support both (e.g. 2 options)
  3. pick the rules that warrant to be changed, create issues

Without that list I can't tell which route is better, sorry.
1 would be a headache unless only one rule is affected
2 is lazier but faster to implement (common sense will dictate whether a fix is necessary or not)
3 is a pragmatic approach but require argumentation

I have added this issue to the Nesting sublist of #7396.

@ybiquitous
Copy link
Member

Depending on the rules, resolving nested selectors is valid or not.

I personally think that there should be an effort to use a more semantic approach.

I'm curious, how do you think specifically?

@romainmenke
Copy link
Member Author

The rule-selector-property-disallowed-list rule makes it possible to disallow specific properties when a selector matches user config.

But what users maybe want is to disallow specific properties on certain DOM elements.
(Or as close as we can get to that with static analysis.)

When disallowing width on p:

These would error

p { width: 100px; }

p.foo { width: 100px; }

.foo p { width: 100px; }

:is(h5, p) { width: 100px; }

p:has(a) { width: 100px; }

These would not error

p .foo { width: 100px; }

p ~ h2 { width: 100px; }

:not(p) { width: 100px; }

div:has(p) { width: 100px; }

The proposed mechanic:

  • only consider the right most compound when matching selectors
  • fully resolve nested selectors
  • interpret the meaning of functional pseudo's like :is(), :not(), :has(), ...

The edge case that won't be caught:

<p class="foo"></p>
.foo { width: 100px; }

I am ignoring how complex this would be to create.
If this is something that is vastly better for users then we should consider it.

@romainmenke
Copy link
Member Author

I personally think that there should be an effort to use a more semantic approach.

A completely different strategy (and simpler to implement) is to have a well known way to normalize selectors.

  • handle nesting
  • handle :scope ?
  • handle whitespace
  • handle ordering within compounds (.foo[bar="1"] vs [bar="1"].foo)
  • handle selectors that can be written in multiple different ways ([bar=1] vs [bar="1"])

leaving it vague what handle implies, the exact definition is less important than always using the same system

Setting the correct options would be easier for a wider range of selectors.

@Mouvedia
Copy link
Contributor

Mouvedia commented Mar 14, 2024

The proposed mechanic:

  • only consider the right most compound when matching selectors
  • fully resolve nested selectors
  • interpret the meaning of functional pseudo's like :is(), :not(), :has(), ...

That sounds ideal.

I am ignoring how complex this would be to create.

Incremental improvement is also a possibility.
i.e. start with the simplest to implement or the most useful-for/requested-by users

A completely different strategy (and simpler to implement) is to have a well known way to normalize selectors.

If the way contradicts what our rules are doing currently, it would be a breaking change.
If not, it's just documenting what we have done/decided so far.
The former corresponds to 1 and later to 2 in my comment above.

@ybiquitous
Copy link
Member

@romainmenke @Mouvedia Thank you for your feedback. I also believe the suggestions for rule-selector-property-disallowed-list would be useful for many. But at the same time, I recommend gradually implementing the features, keeping the compatibility as much as possible.

During the process of such steps in implementation, we could discover many ideas or improve the current implementation or documentation.

I basically welcome such attempts toward modern CSS, although we have to care about compatibility!

@romainmenke
Copy link
Member Author

I am purposefully ignoring compatibility for now in this issue, not because I do not think it is important, but because I first wanted to see if this direction is something we want to pursue at all :)

For me compatibility is mostly about communication and planning.

If we don't want it anyway (not all ideas are good ideas), then we don't even need to discuss how to handle potential breaking changes.

If it isn't feasible for technical reasons, then we also don't need to think about the compat issues :)


I have a couple of technical concerns with this idea:

  1. This implies a lot more parsing than before.
    We would need to parse all selectors to be able to check config against them.

This would dramatically slow down these rules.

  1. This isn't forwards compatible. If new CSS features are introduced that affect the meaning of a selector we would always need to push out an update before authors can have working config.

  2. Most other config uses simpler string/regex matching. It would be harder to explain to users that some rules have a completely different way of working.

  3. String/regex based matching is messy and inaccurate but it is also very easy to understand. It matches the text users can see, copy/paste and debug. It doesn't match an unknowable parsed interpretation of their selector.

@ybiquitous
Copy link
Member

ybiquitous commented Mar 19, 2024

Since new features have entered the CSS world today, I predict people will want more accurate parsing.

This would dramatically slow down these rules.

For performance, we need to actually measure new implementations, right? We might be able to provide opt-in and experimental options to see actual performance or users' evaluation, for example.

@romainmenke
Copy link
Member Author

This would dramatically slow down these rules.

Since the parsed result is never mutated and only used for analysis we could also cache this. Then each unique selector string would only need to be parsed once, instead of every selector needing to be parsed for every rule.

So even if measurements indicate that is is really slow, we might still have options :)

@Mouvedia
Copy link
Contributor

I have a couple of technical concerns with this idea:

  1. This implies a lot more parsing than before.
    We would need to parse all selectors to be able to check config against them.

This would dramatically slow down these rules.

  1. This isn't forwards compatible. If new CSS features are introduced that affect the meaning of a selector we would always need to push out an update before authors can have working config.
  2. Most other config uses simpler string/regex matching. It would be harder to explain to users that some rules have a completely different way of working.
  3. String/regex based matching is messy and inaccurate but it is also very easy to understand. It matches the text users can see, copy/paste and debug. It doesn't match an unknowable parsed interpretation of their selector.

It seems that you are slowly coming to a resolution yourself.
I think a simpler method would be to reply to users requests/issues.
i.e. pull strategy instead of push
We will assess their demand and see if it's worth supporting on a case by case basis.
e.g. the consistency argument—compared to existing rules—can be moot if common sense tells us that resolved string is desirable/expected/better for that option
My only doubt would be in the unlikely case that the default resolve strategy differs between 2 options of the same rule; that would be too confusing IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion
Development

No branches or pull requests

3 participants