Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

☂️ CSS support #984

Closed
sebmck opened this issue Aug 8, 2020 · 33 comments
Closed

☂️ CSS support #984

sebmck opened this issue Aug 8, 2020 · 33 comments
Assignees
Labels
Help wanted Help would be really appreciated L-CSS Language: CSS umbrella Issue to track a collection of other issues

Comments

@sebmck
Copy link
Contributor

sebmck commented Aug 8, 2020

We already have the basis of a CSS formatter that @bitpshr has built out. Not sure how much more work needs to go into supporting additional syntax and testing.

Linting

What sort of rules do we want? https://stylelint.io/ and https://postcss.org/ would be good references.

@sebmck sebmck added umbrella Issue to track a collection of other issues L-CSS Language: CSS labels Aug 8, 2020
@tomhodgins
Copy link

One thing that I haven't seen that would be nice is if there are explicitly author-defined things in CSS, like a registered custom property, would be for tooling to support/lint it according to its own definition in CSS/JS.

It's possible create custom properties in CSS like this:

a {
  --demo: blue;
}

And there's also the ability for you to register them from CSS with @property or from JavaScript with registerProperty():

@property --demo {
  syntax: '<color>';
  inherits: true;
  initial-value: blue;
}

or in JS

CSS.registerProperty({
  name: '--demo',
  syntax: '<color>',
  inherits: true,
  initialValue: 'blue'
})

Given one of these definitions, it would be nice if there was tooling that could look at code like the following example and produce error messages that instruct what it needs in order to be valid according to your own definitions of what that custom property is:

example {
  --demo: 1px;
}

Error: --demo property accepts a <color> type as its value

@alexander-akait
Copy link

@sebmck @ematipico Need help? I have long dreamed of rewriting postcss/stylelint/cssnano (over the years, many problems have accumulated, especially related postcss - lack of opportunities of parsing). Many stylelint rules was written/rewritten by me, cssnano (minificator for CSS) lately supported by me, have worked on CSS formatting in prettier. But I would like to discuss some aspects a little, especial non standard syntax, like less/scss/sass/etc, improvements, fixes (small/not small roadmap).

@alexander-akait
Copy link

cc @sebmck @ematipico friendly ping

@ematipico
Copy link
Contributor

ematipico commented Oct 31, 2020

Hi @evilebottnawi , at the moment @bitpshr has made a stable base for the CSS parser although there are still some aspects missing (I can't remember the details but there some sub grammars that are not supported yet).

Although we could potentially start writing something around linting, which is the first step I think (together with the formatter).

About minification/purging, there's nothing yet and having a good input on how to start is always welcome.

About non-standard syntax, I don't know if we are ready yet. SASS and LESS are supersets of CSS so if we don't have a stable CSS parser, I don't think we can implement much around SASS and LESS. Theoretically we could start the foundations but I would wait and focus on what's more important.

@ematipico
Copy link
Contributor

We need to implement the following selector syntax: https://www.w3.org/TR/selectors-4/#syntax

@ematipico ematipico added the Help wanted Help would be really appreciated label Nov 16, 2020
@alexander-akait
Copy link

@ematipico Thanks for answer, I'll look at the code and get back to it in near future, maybe we should start with CSS formatter (without scss/less/etc), it should be easy and will allow finish parser, good parser = good linter

@tomhodgins
Copy link

@ematipico Let me know if there's anything I can do to help, either by providing samples of valid CSS syntax, or hunting for things in specs (tokens, productions, etc). I've already got a bunch of tools I use in my own work for parsing or validating pieces of CSS syntax according to the spec :D

And I've got plenty more samples and examples too, here's a list of known-valid CSS custom property values gleaned from the Web Platform Tests:

legal css custom property values {
  --percentage: 25%;
  --number: 37;
  --length: 12em;
  --time: 75ms;
  --function: foo();
  --nested-function: foo(bar());
  --parentheses: ( );
  --braces: { };
  --brackets: [ ];
  --at-keyword-unknown: @foobar;
  --at-keyword-known: @media;
  --at-keyword-unknown-block: @foobar {};
  --at-keyword-known-block: @media {};
  --cdo-at-top-level: <!--;
  --cdc-at-top-level: -->;
  --semicolon-not-top-level: (;);
  --cdo-not-top-level: (<!--);
  --cdc-not-top-level: (-->);
}

@ematipico
Copy link
Contributor

ematipico commented Jan 10, 2021

@ematipico
Copy link
Contributor

Currently working on CSS Variables and var()

@ematipico
Copy link
Contributor

Currently working on Keyframes

@ematipico
Copy link
Contributor

Thank you @tomhodgins , FYI I am going to update our test-fixtures with the example that you pasted here.

@jeddy3
Copy link

jeddy3 commented Feb 13, 2021

It's fantastic to see so much progress on the CSS parser and formatter!

What sort of rules do we want? https://stylelint.io/ and https://postcss.org/ would be good references.

I can break down the stylelint rules for you. I helped create stylelint, and I'm more than happy to share what we learnt.

We split the rules in stylelint into three categories:

  • possible error
  • limit language feature
  • stylistic issue

The stylistic issue rules check whitespace, case and quotes. (The rules are configurable, but an opinionated formatter would avoid that complexity.) We didn't develop a good solution for automatically fixing long line lengths. Prettier does this well, though.

stylelint-order is the most popular stylistic plugin and can order the content of blocks, i.e. properties, nested rules, etc.

The limit language feature rules let you enforce non-stylistic code consistency. For example:

These tend to be specific to a team or project, which is why the rules are configurable and not found in the official configs.

The possible error rules check for duplicates, overrides and (easy to find) invalid syntax. Full validation is the most requested feature in stylelint and is not something we've been able to do in stylelint itself.

For example, flagging that 10px isn't a colour:

a { color: 10px; }

This ties into @tomhodgins suggestion above of validating registered custom properties.

CSSTree does validation well, and you can validate property-value pairs in stylelint using the CSSTree stylelint validator plugin. Here is CSSTree's reference for the <color> type, which it pulls from the MDN data set.

I hope this quick breakdown of the rules was helpful. I'm happy to fresh out any bits if that'll be useful to you.

I'm excited for Rome. As a fan of CSS, it'd be amazing to write linting rules while not also having to maintain a CLI and editor extension just for CSS!


They may already be on your radar, but I didn't see the following syntaxes mentioned above:

@ematipico
Copy link
Contributor

Starting working on media queries!

@jeddy3
Copy link

jeddy3 commented Feb 19, 2021

Starting working on media queries

Fantastic!

They may already be on your radar, but I often see the range context from the level 4 specification, and custom media queries and the user preference media features from the level 5 specification in the wild.

/* range context */
@media (480px <= width < 768px) {}

/* custom media queries */
@custom-media --narrow-window (max-width: 30em);
@media (--narrow-window) {}

/* user preference media features */
@media (prefers-reduced-motion: reduce) {}
@media (prefers-color-scheme: dark) {}

The accessibility community is driving the adoption of user preference media queries.

The range context and custom media queries are typically transpiled using postcss-preset-env.

@ematipico
Copy link
Contributor

ematipico commented Feb 19, 2021

Thank you for the pointers! This opens up a lot of things, especially what should we support in Rome. If it was for me I would start writing AST nodes and logic without breaks; probably we would need to come up with something.

@jeddy3 BTW, do you know how/where to check the actual stage of a specification? Unfortunately I don't know much about how's the bureaucracy around CSS.

@jeddy3
Copy link

jeddy3 commented Feb 19, 2021

I didn't offhand, but I found this list of current work. The first specification listed is CSS Snapshot 2020, which introduces the process and stages.

I tend to dig into a specification when someone requests a new rule in stylelint for a CSS feature I haven't seen before. Even after a few years of building stylelint, I'm still learning about all the things that CSS can do. It's an amazing language.

@tomhodgins
Copy link

what should we support in Rome

All features supported in the CSS language will abide by CSS's syntax, specified here: https://drafts.csswg.org/css-syntax-3/ so if CSS is parsed accurately according to its own syntax, all features should be something we can work with (for example @media (min-width: 500px) versus @media (width >= 500) are both super easy to parse because you don't need to know anything about media queries to know width is an <ident>, etc, parsing future features or author-defined features should be possible even if we never know what they are, as long as they are parsable according to the syntax defined in the syntax spec.)

BTW, do you know how/where to check the actual stage of a specification?

There are collections of the current drafts being worked on in a couple places like:

There are statuses like 'Candidate Recommendation' and 'Draft' etc but they don't work like 'stages' for JS proposals. If you want an unofficial peek at what 'stages' things might be, Jon Neal has a site here with upcoming drafts and roughly (in his estimation) where they're at in a stage-like process: https://github.com/csstools/cssdb

@jeddy3
Copy link

jeddy3 commented Mar 3, 2021

All features supported in the CSS language will abide by CSS's syntax ... parsing future features or author-defined features should be possible even if we never know what they are

That's really cool. I assume that's what allowed the explosion of PostCSS plugins that extended CSS.

I'm not very familiar with parsers, so I don't know if this is possible, but a granular parser would make it easier to write lint rules. For example, the media-feature-name-disallowed-list rule from stylelint.

Given:

/* user preference media features */
@media (prefers-reduced-motion: reduce) {}

/* range context */
@media (480px <= width < 768px) {}

/* custom media queries */
@custom-media --narrow-window (max-width: 30em);

Is it possible to have prefers-reduced-motion, width, and max-width as both idents and, more specifically, as feature names?

@VictorHom
Copy link
Contributor

I'm going to find time to work on the @import rule if that's available

@ematipico
Copy link
Contributor

ematipico commented Mar 29, 2021

I will work on implementing few nodes around @font-family.

@ematipico
Copy link
Contributor

CSS lint is now enabled on master!! I added a first rule so we have diagnostic categories, highlighting, harness test, etc. well prepared.

Probably the highlighting part needs to be reviewed and we would need to add few stuff to make the coloring better.

We should start discussing which rules we want to implement. We could focus, first, on rules that prevent errors and don't require some configuration. After that we could start working on other rules.

This section of the stylelint website is a good starting point.

@jeddy3
Copy link

jeddy3 commented Apr 8, 2021

CSS lint is now enabled on master!!

That's fantastic!

This section of the stylelint website is a good starting point.

The rules in this section can be roughly split into two groups:

  • invalid CSS that's definitely a mistake
  • valid CSS that's probably a mistake

The first group is made up of:

I believe Rome already handles the function-calc-no-invalid, function-calc-no-unspaced-operator and, possibly, the string-no-newline rules at the parser level?

Rather than add piecemeal no-valid, no-nonstandard and no-unknown rules like we did in stylelint, it may be better to investigate how MDN data could be used to build a comprehensive validator like CSSTree and CSSType do.

The second group is made up of:

There is also the no-descending-specificity and no-extra-semicolons rules. The latter rule is likely covered by Rome's AST-based formatter?

All of the patterns caught by these rules are valid CSS but are likely mistakes that won't be caught by a validator.

Out of these rules, you may want to start with the no-duplicates and no-*-overrides ones as these are frustrating issues for developers to debug. For example, the longhand margin-top is overridden by the shorthand margin:

a {
  margin-top: 5px; /* this is overridden */
  margin: 10px;
}

Not all the no-duplicates rules are as equally clear-cut as each other. For example, duplicated custom properties are likely to be mistakes but duplicated properties could be intentional fallbacks:

a {
  --my-prop: 2px;
  --my-prop: 4px; /* likely a mistake */
  font-size: 16px;
  font-size: 1rem; /* likely intentional */
}

As such, the declaration-block-no-duplicate-properties rule has secondary options, whereas the declaration-block-no-duplicate-custom-properties) rule has none.

It may be best to start with the clearer-cut of these no-duplicates rules first.

I think users have a love-hate relationship with the no-empty rules; they are useful for spotting accidentally left-in code but can be annoying when you're in the process of writing code.

I'd place the following rules at the bottom of the list:


Although not in this section, I think the no-unknown-animations rule is particularly interesting and worth an early look because it is so limited in stylelint; only animations (i.e. custom-idents) defined in the same source are considered known in stylelint as linting happens on each source in isolation.

The 3rd party stylelint-value-no-unknown-custom-properties plugin works around this limitation with an importFrom option, but it would have been better if we had made something like this available in stylelint itself.

Bringing this and validation together, I'd be fantastic if a user could define their custom properties in one place:

/* colors.css */
@property --primary,
@property --accent {
  syntax: '<color>';
  inherits: true;
}

@property --primary {
  initial-value: red;
}

@property --accent {
  initial-value: blue;
}

And reference them in another:

/* component.css */
a {
  margin: var(--acent); /* violation - unknown custom property */
  margin: var(--accent); /* violation - unexpected `<color>` value for `margin` property */
} 

I don't know if Rome can lint a source in the broader context of all the sources in the graph, but if it can then it'll leapfrog over what stylelint is currently capable of doing and will open up lots of exciting possibilities!

@alexander-akait
Copy link

stylelint-value-no-unknown-custom-properties plugin

This rule will have a lot of limitations, because custom properties hoist, so you can use variable from other files or context, for example style tag of HTML

@ematipico
Copy link
Contributor

Lot of amazing feedback! Thank you! I will probably start creating an umbrella issue where I will list the rules that actually makes sense having.

I believe Rome already handles the function-calc-no-invalid, function-calc-no-unspaced-operator and, possibly, the string-no-newline rules at the parser level?

Yes, many rules won't be needed because they are errors in the spec, so we throw diagnostics during the parsing phase of the file.

e.g.

.style { 
	width calc(2rem * 2rem / 1rem)
}

this syntax will throw a diagnostic error during parsing, because by spec after / we can only have numbers.

There is also the no-descending-specificity and no-extra-semicolons rules. The latter rule is likely covered by Rome's AST-based formatter?

The formatter already takes care of adding leading semicolons, but double semicolons are peculiar. We might need to check if it's an error in the spec or not, and if not, we might need a rule.

Out of these rules, you may want to start with the no-duplicates and no-*-overrides ones as these are frustrating issues for developers to debug.

This is a good case! We should add them to the list! And we could decide to add an auto fix. Thank you!

I don't know if Rome can lint a source in the broader context of all the sources in the graph, but if it can then it'll leapfrog over what stylelint is currently capable of doing and will open up lots of exciting possibilities!

At the moment the linter works on single files, but Rome has a solid infrastructure with a dependency graph out of the box. We might be able to achieve this, we just need to think how.

stylelint-value-no-unknown-custom-properties plugin

This rule will have a lot of limitations, because custom properties hoist, so you can use variable from other files or context, for example style tag of HTML

I believe in Rome we could eventually achieve this. Not at rule level (lint) but probably at compilation level. Because of the fact that Rome can parse JS, CSS and HTML, and the fact that we have already a dependency graph that works, we might be able to build the correct scopes for CSS and understand when a property is used (or not), where, and if it makes sense having it. It would require lot of work, but I think it's achievable.

@alexander-akait
Copy link

@ematipico just interesting is Media Queries Level 4 (https://www.w3.org/TR/mediaqueries-4/) supported?

@ematipico
Copy link
Contributor

ematipico commented Apr 8, 2021

Yes they are! Although range context is not yet. The PR was becoming too big so I decided to cut it short and implementing it later

@alexander-akait
Copy link

Great, as I understand it, in the future we will have minimizer/uglifier too?

I understand what rome is self-sufficient, but maybe we can publish some parts, for example current CSS parsers are very bad, if we could get the parts like parsers/minimizers/etc we could start moving community, and in the end it would allow everyone to migrate painlessly. What do you think? Can we discussion about it?

@ematipico
Copy link
Contributor

Releasing internal packages is part of the roadmap!

Not sure how and when, but I know that Sebastian wants to do it :)

Probably after we craft a decent plugin system. It's one of the main concerns.

@alexander-akait
Copy link

alexander-akait commented Apr 9, 2021

@ematipico Sounds good, hopefully it will come sooner so that we can start this migration as it will take time.

Getting more feedback we will fix more bugs faster and implement more features.

@ematipico
Copy link
Contributor

Closing in favour of #2350

@ematipico ematipico mentioned this issue May 9, 2023
8 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Help wanted Help would be really appreciated L-CSS Language: CSS umbrella Issue to track a collection of other issues
Projects
None yet
Development

No branches or pull requests

7 participants