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 to enforce consistent order of composite values #23

Open
MorevM opened this issue Apr 1, 2024 · 6 comments
Open

Rule to enforce consistent order of composite values #23

MorevM opened this issue Apr 1, 2024 · 6 comments

Comments

@MorevM
Copy link
Contributor

MorevM commented Apr 1, 2024

What is the problem you're trying to solve?

As a continuation of the idea raised here in the point 3, I suggest we discuss this in more detail here - naming and options.

I propose the following implementation plan:

  1. Choose a name for the rule. I propose composite-values-order or shorthand-values-order.

  2. Make a list of properties to which the rule needs to be applied. First of all these are shorthand properties like animation, transition, background and so on, but we need to decide about other values as well - for example, we can control the order of some keywords like auto in aspect-ratio property (auto 1/2 and 1/2 auto both valid). I'm sure there are others like that.

  3. Make a list of tokens defining the constituents of each rule. I suggest using token names defined in MDN, for example for animation it would be ['time', 'easing-option', 'single-animation-iteration-count', '...so on'].

  4. Define what default order we wish to provide without the need for additional configuration. I guess we could also focus on MDN, although for some properties personally I prefer a slightly different order... But in terms of standards, I think it's better to stick to open authoritative sources.

In the end, I envision a configuration like this:

{
  'composite-values-order': [true, {
    // only for overwriting defaults, if needed
    'aspect-ratio': ['auto', 'ratio'],
    'animation': ['time', 'easing-function', 'single-animation-iteration-count', '...']
  }]
}

Potential problems:

  1. Properties that have some conditions (for example, in the font property, the line-height part can only come after font-size, there is a similar thing for background property, and so on).
    I don't think they can be thrown out from the configuration - we should probably come up with own compound tokens for these properties (e.g. font-size-and-line-height), but it's not very clear what to do in case of misconfiguration (lack of mandatory part) - Stylelint, as far as I know, doesn't provide a way to notify the user about invalid configuration. We'll probably have to leave that up to the users.

  2. Other user misconfiguration - for example, listing not all required tokens. What to do with the remaining ones? It looks like the rule should be turned off in such cases, but how the user should know about - it's still a question.

  3. Some of compound parts may have an optional order within it. Referring to MDN, linear-stop part may have a conditional order of compound parts.
    We'll probably have to add tokens that don't match CSS properties to the configuration for such scenarios.


So, what do you think about the name, non-shorthand properties handling, about defaults and so on?
If there are any other clarifications/opinions, I'd love to hear them. For now, I plan to go with this plan, recording progress here.
It's a long and rather difficult task, but I believe this can be done.

What solution would you like to see?

I will start working on the implementation according to the plan above after receiving feedback :)

@MorevM
Copy link
Contributor Author

MorevM commented Apr 6, 2024

Here's a list of CSS properties I've found and their tokens, which can be in any order.
I might have missed something, but it's enough to get started anyway.

animation
  - [time, easing-function, iteration-count, direction, fill-mode, play-state, name]
  - [linear-stop = linear-stop-value && linear-stop-length]

aspect-ratio
  - [auto, ratio]

background
  - [attachment, clip, origin, color, image, position-and-size, repeat]

border
  - [width, style, color]

border-image
  - [source, slice-width-outset, repeat]

column-rule
  - [width, style, color]

column
  - [width, count]

flex
  - [grow-shrink, basis]

flex-flow:
  - [direction, wrap]

font:
  - [style, variant-css2, weight, width-css3, stretch, size-and-line-height, family]

font-synthesis:
  - [weight, style, small-caps, position]

font-variant:
  - ???

grid-auto-flow:
  - [row-or-column, dense]

list-style:
  - [position, image, type]

mask:
  - [reference, position-and-size, style, composition-operanor, mode, origin, clip]

mask-border:
  - [source, slice-width-offset, repeat, mode]

offset:
  - [source, slice-width-offset, repeat, mode]

outline:
  - [width, style, color]

text-decoration:
  - [line, style, color]

text-emphasis:
  - [line, style, color]

text-wrap:
  - [mode, style]

transition:
  - [property, duration, function, delay]

@MorevM
Copy link
Contributor Author

MorevM commented Apr 12, 2024

  • Just keeping up to date with progress :)

I started writing code and realized that implementing a shorthand parser within this repository is somewhat redundant.
I made the decision to create a separate package for parsing shorthands, and use its public API here.
This allows to separate entities better, and also better test the API.

Something like this:
https://github.com/xNoRain001/css-shorthand-parser
https://github.com/mahirshah/css-property-parser

But these repositories are abandoned, don't work with some features, and generally work a bit differently than what we need for sorting already written properties.

@firefoxic
Copy link
Collaborator

@MorevM Hi!
I apologize, I've been really busy. I'll try to answer everything.

  1. Choose a name for the rule. I propose composite-values-order or shorthand-values-order.

The first variant is more suitable, because for example the aspect-ratio property is not a shorthand.

  • Properties that have some conditions (for example, in the font property, the line-height part can only come after font-size, there is a similar thing for background property, and so on).
    I don't think they can be thrown out from the configuration - we should probably come up with own compound tokens for these properties (e.g. font-size-and-line-height), but it's not very clear what to do in case of misconfiguration (lack of mandatory part) - Stylelint, as far as I know, doesn't provide a way to notify the user about invalid configuration. We'll probably have to leave that up to the users.

  • Other user misconfiguration - for example, listing not all required tokens. What to do with the remaining ones? It looks like the rule should be turned off in such cases, but how the user should know about - it's still a question.

Both points seem to be solved by combining two tokens into one, as in the list below for the font property of the size-and-line-height token. Implying of course that the merged token may contain optional parts, like the same line-height token.

All in all, everything looks not bad.

@firefoxic
Copy link
Collaborator

I made the decision to create a separate package for parsing shorthands, and use its public API here.

I haven't had time to figure out the necessity of it yet. But...

I've thought about inviting you into the organization before. And creating separate tools seems like a good reason to do so. The shorthand parser could be placed in the organization.

@MorevM Have a desire to be part of the team?

@MorevM
Copy link
Contributor Author

MorevM commented Apr 14, 2024

@firefoxic Hi,

It will definitely be required.
I just started yesterday, but the structure is already quite extensive - and that's not even 20% of all the work required.

image

Have a desire to be part of the team?

In general, I wanted to make a universal shorthand parser like the links above... Although, on second thought, I have nowhere to use it except here :D
And keeping only the use case within Stylelint in mind - it makes it easier to design an API.

So I might consider creating a specialized package with stylelint-stylistic utilities within the organization.
As part of that, we can plan to migrate and refactor the current utilities (from the /utils/ folder).
I think this could be a good separation and make further migrations somewhat easier.

I'll message you in Discord, discuss details there if necessary :)

@firefoxic
Copy link
Collaborator

I'll message you in Discord

Uh, do we have Discord? 👀

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

No branches or pull requests

2 participants