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
declaration-block-no-redundant-longhand-properties
is not forwards compatible
#7630
Comments
Extra disclosure : I don't use this rule and actually actively push team members to use longhands over shorthands. I want this rule to work well for those who want to use it, but I am not attached to or invested in it. |
@romainmenke Thanks for opening the discussion. Your thought is very interesting for me, especially:
Currently, we have turned on this rule in our standard config: But, considering recent bugfix reports, it seems that we should remove the rule from the shared config at least. For this point, @Mouvedia may have any thoughts. |
Concerning #7232, I have introduced an option with the fix so that the impact can be mitigated.
I think I did my due diligence in that issue to explain the impact in minute details and introducing a secondary option with the fix.
Should this be moved to discussions?
I think the policy should be to introduce changes if the support reaches a certain threshold. From a maintainer stand point, it's a matter of being careful about that type of changes. |
This has already the |
I was talking about https://docs.github.com/en/discussions |
We don't run GitHub Discussions since we've managed discussions by the label so far. This will not change in the future. |
The problem here is not only specific properties (e.g. If we can't easily resolve the problem, we should remove the rule from our shared config and even leave a warning in the rule doc, at least until the rule would become enough mature. |
When seeing the rule's implementation, custom logic for some properties have increased. Such a logic will be more, as new properties are added to CSS.
EDIT: If we had enough human resources, maturing the rule would be possible. However, as you know, there are a few people who can often work on this product. 😢 |
This is indeed very useful as an escape hatch when user notice that their CSS is altered in incorrect ways. But it does expect users to notice. I fear that most will assume that Stylelint knows best and will get frustrated when they eventually find that Stylelint is breaking their code. We could have automation/signals to help us update this rule more quickly ( Ultimately this is also an upstream issue :) |
At the minimum add |
Ah, this seems likely to me... We may need to even suspend the rule's autofix. |
I think this will not resolve the fundamental problem with this rule. First of all, we should discuss if this rule should be present the shared config. |
Aside: Wanted to say that none of this is feedback on the hard work that so many people have been putting into this rule :) It is a really complicated subject that is non-trivial to get right and maintain. This is mostly an illustration of how complexity cascades. Shorthands are significant complexity in CSS itself and any tooling that touches on shorthands will also encounter complexity. |
Forwards compatibility means that future CSS can be used in a (slightly) older version of Stylelint.
I don't expect this to be true for large syntax changes.
But I expect Stylelint to not break my code each time a new constituent is added to a shorthand.
A concrete example is the introduction of
text-decoration-thickness
intext-decoration
.The syntax was updated:
<'text-decoration-line'> || <'text-decoration-style'> || <'text-decoration-color'>
<'text-decoration-line'> || <'text-decoration-thickness'> || <'text-decoration-style'> || <'text-decoration-color'>
See :
https://stylelint.io/demo/#N4Igxg9gJgpiBcICGACYAdAdugLjmAHjgLSyQBOSOAlhJsTgBbVgDWmMAzp-CgEwAHAgG4sufEVIwKVWvU44AngBsYvKBDwwoo7FslkIlGnWLLqHXgFdMscuY67xhEoeNzikZUd7ltugF8QABoQADNqVQA5JABbOERCOIFVADowbhDwOgiAcwQQDEwUFHQQFxhbTjLeAG0sEpKyhRUYBxJITDziBSRbJHIoMoaUAF0sINDOvIAxI1iqAoArTjos2AFOAqKmkBbVdprSkGUqLhwy4JHmpQOLDpzqXJ6cPqgBoYRj0-wFYcwggEgA
Is changed to
Which erases
text-decoration-thickness
.Updates can be made to teach Stylelint about the updated syntax but this implies a significant maintenance burden.
This is different from other kinds of unknown syntax or CSS aspects as many CSS authors will not have memorized exactly which longhands are constituents of which shorthands and might not even notice the destructive changes Stylelint makes in these cases.
The issue here is not
text-decoration
itself, but that changing longhands to shorthands is a very impactful source code change.Longhands and shorthands are simply not the same in CSS.
This issue intended for discussion around this topic and to make sure that this aspect of this rule is in line with the overal goals of Stylelint core rules.
The text was updated successfully, but these errors were encountered: