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

Add CSS-in-JS support #3264

Closed
ai opened this issue Apr 17, 2018 · 21 comments
Closed

Add CSS-in-JS support #3264

ai opened this issue Apr 17, 2018 · 21 comments
Labels
status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules

Comments

@ai
Copy link
Member

ai commented Apr 17, 2018

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

feature request

@gucong3000 announced amazing postcss-styled parser for all Button = styled() CSS-in-JS solutions (styled-components, Emotion.js, etc). Let’s add it to default pack as other parsers (or to the docs).

CSS-in-JS become more and more popular. To bring new user and keep “trendy” feeling (it is very important to find new contributors) we should have official CSS-in-JS support.

Right now we can do it by stylelint-processor-styled-components. But, as I understand, processor doesn’t work with --fix. Also, it doesn’t work well with all rules.

postcss-styled could be used easy as any other PostCSS syntaxes like postcss-scss.

@jeddy3 what do you think?

@alexander-akait
Copy link
Member

@ai 👍

@hudochenkov hudochenkov added the status: needs discussion triage needs further discussion label Apr 23, 2018
@ai
Copy link
Member Author

ai commented Apr 27, 2018

@jeddy3 any thoughts or questions?

@ntwb ntwb added this to the future-major milestone Apr 28, 2018
@ntwb
Copy link
Member

ntwb commented Apr 28, 2018

Thanks for creating the issue @ai 👍

This is part of a refactoring of processors that stylelint uses with the goal of reducing the overall optimizing the package size, see:

#3261 (comment)
#3248 (comment)

We will for sure work toward adding postcss-styled as part of all this 😄

@jeddy3 jeddy3 changed the title Add build-in CSS-in-JS support What languages should be supported out-of-the-box? May 17, 2018
@jeddy3
Copy link
Member

jeddy3 commented May 17, 2018

There were two amazing things I ran into while reviewing #3261:

  1. CSS being autofixed within markdown fences, <style> tags and style="" attributes! This is due to @gucong3000's hard work on the new PostCSS syntaxes.
  2. The advanced features of processors! The fact that stylelint can, through the hard work of processor authors, lint the following blows my mind:
import styled from 'styled-component';
const Button = styled.div`
  ${/* sc-selector */ Button} {
    ${/* sc-prop */ something}: papayawhip;
    margin-${/* sc-custom 'left' */ rtlSwitch}: 12.5px;
  }
`

Where /* sc-selector */ etc are interpolation tags included within the styled-components processor.

Or:

import glamorous from 'glamorous';
const Component = glamorous.div({ margin: 0 });
render(
  <Div css={{ margin: 0 }}/>
)
export const styles = 
// @css
{
  margin: 0
}

Where all three types of style definitions are extracted by the glamorous processor.

I've highlighted the two amazing things above because I wanted to show that, if I'm correct, the existing processors and the new syntaxes offer different features. There is definitely an overlap, but @gucong3000 am I right in saying that postcss-styled does not support interpolation tagging and can't parse all three of the glamorous style definitions?

So I believe the question are:

  • can autofixing only be achieved through syntaxes?
  • If so, can syntaxes include features like interpolation tagging?
  • If not, can syntaxes and processors be made to work together e.g. the syntax parses and the processor handles things like interpolation tagging?

The other question at hand is, what should be bundled in stylelint itself?

We originally added support for processors because we realised that there was a never-ending list of things that CSS can be extracted out of and trying to keep-up with them all wouldn't be sustainable. We added the --custom-syntax flag for the same reasons. It feels like the CSS-in-JS landscape is shifting even quicker. We're struggling to support the existing syntaxes (CSS, SCSS, SASS, Less and SugarSS) and I'm unsure if we'd cope with more. We should consider this when weighing up the pros and cons of adding built-in support for more syntaxes.

We also recently realised stylelint has a weight problem (#3248).

I laid out my reasons in #3252 (comment) for why I think we should:

  • continue to support CSS, SCSS, Sass, Less and SugarSS within their own files (and continue to infer the syntax from the filename extension e.g. .scss, .sass, .less and .sss)
  • continue to support CSS, SCSS, Sass, Less and SugarSS within <style> tags (and continue to allow the syntax to be specify in the lang attribute e.g. <style lang="scss">)
  • continue to support CSS within style="" attributes

And drop built-in support for markdown fences.

We can, instead, improve the documentation around custom syntaxes (and processors if they stick around). We can better emphasise that, as well as the built-in support for the existing syntaxes and style tags/attributes, stylelint can be used with custom syntaxes (and processors).


So, I believe there are two parts to this issue:

  • what is the role of syntaxes and how do processors fit into the picture?
  • what should stylelint support out-of-the-box?

It feels like we should resolve the former before moving onto the latter.

@gucong3000
Copy link
Member

gucong3000 commented May 19, 2018

but @gucong3000 am I right in saying that postcss-styled does not support interpolation tagging and can't parse all three of the glamorous style definitions?

interpolation tagging was supported in new version of postcss-styled.

I created a new package postcss-jsx to suppurt glamorous and styled-components, but it use babel@7.x.x-beta as parser, it is not a safe version.

@gucong3000
Copy link
Member

gucong3000 commented Jun 12, 2018

There is some information for postcss syntaxes.

packages dependencies languages
postcss-html htmlparser2 html, html-like
postcss-markdown remark, remarkunist-util-find-all-after markdown
postcss-styled null template literals in js/ts
postcss-jsx @babel/generator, @babel/parser, @babel/traverse, @babel/types, postcss-styled object/template literals in js/ts

@gucong3000
Copy link
Member

How about removing markdown support and adding styled support?

@ai
Copy link
Member Author

ai commented Jun 12, 2018

CSS-in-JS got 10% market just for last year. styled support (with --fix) is very important.

@gucong3000
Copy link
Member

gucong3000 commented Jun 12, 2018

Current support for frameworks:

@jeddy3
Copy link
Member

jeddy3 commented Jun 21, 2018

interpolation tagging was supported in new version of postcss-styled.

Thanks. I've created a separate issue to follow up on deprecating the processor system in favour of using syntaxes.


styled support (with --fix) is very important.

Users can already use the --custom-syntax option to support any language they want e.g. they can already use --fix in styled components using the postcss-styled syntax.

I think we should ask ourselves what are the pros and cons of bundling syntaxes within stylelint itself?

In #3264 (comment), I suggested we keep bundled support for:

  • CSS preprocessors (e.g. Scss, Less, Sass and SugarSS)
  • extracting from <style>/style=""

And remove bundled support for extracting from markdown.

I no longer think that's a good approach as the line that divides what is bundled and what isn't feels arbitrary This in turn makes it difficult to for users to understand how to use stylelint.

I can think of two less arbitrary alternatives... stylelint bundles what's necessary to support:

  • only standards (i.e. CSS and CSS within <style>/style="")
  • all non-standard CSS-like languages (i.e. Scss, Less, Sass and SugarSS) and all non-standard containers (i.e. all CSS-in-JS flavours and markdown)

Whatever approach we decided on, the built-in rules will continue to focus on only standard syntax and will attempt to ignore any non-standard constucts.

Anyone familiar with the history of stylelint knows that I've historically been in favour of the former. David came around to this thinking too, and there are a lot of good reasons laid out in #2259. However, it looks like prettier adopts the latter i.e. it bundles every parser within its package. I enjoy the ease-of-use that comes with prettier i.e. it works out-of-the-box with most things I throw at it.

Offhand, I can think of a few pros and cons to each approach...

If we support only standards, then the pace of stylelint's development will slow down to match the pace of change in the standards world i.e. we get stylelint itself off the hamster-wheel that comes with the preprocessor and CSS-in-JS worlds. Users of CSS and HTML standards (i.e. <style> and style="") get support out-of-the-box. Users of non-standard things will need to install the extra bits they need, and then use the --custom-syntax option. This is a breaking change, however it will arguably make the core more stable.

If we bundle everything, then more users are supported out-of-the-box and there are no breaking changes. However, I can foresee a couple of disadvantages:

  • the backlog of issues will get more littered with bugs around non-standard things
  • we'll be on the hamster-wheel, watching flavours of preprocessors and CSS-in-JS come and go
  • we'll need to add a build step, similar to prettier's, to roll-up the syntaxes into their own files so that stylelint is performant

@stylelint/core I don't think there's an obvious right answer, and there are (dis)advantages to each. What does everyone want to do?

@jeddy3 jeddy3 added status: needs discussion triage needs further discussion and removed status: wip is being worked on by someone labels Jun 21, 2018
@ai
Copy link
Member Author

ai commented Jun 21, 2018

I think we should be focused on giving the best experience to users, instead of thinking about standards and rational categories.

Most of Stylelint users will be junior developers (since junior developers are the biggest part of development community). They will not understand what is standard syntax and what is not standard syntax.

It was a key philosophy behind Autoprefixer and it helps a lot. So instead of thinking of syntax types (standard/non-standard), I think it will be much better if we will choose the most popular use cases (Sass, React + CSS Modules, React + CSS-in-JS, Vue.js, CSS) and then give them the moth easy way to use Stylelint.

It doesn’t mean, that we must support all these popular languages out-of-box. My suggestions:

  • It means that instead of syntax error message we should show simple step-by-step instruction what to do to support their syntax.
  • Or what to do if they didn’t have config files (link to step-by-step wizard).

Example:

> npx stylelint components/**/*.js
< You don’t have Stylelint config in your project root. Here is a wizard to generate config:
   https://maximgatilin.github.io/stylelint-config/
> fix config
> npx stylelint components/**/*.js
< To support CSS-in-JS you need:
   1. Run npm install postcss-styled
   2. Add "syntax": "postcss-styled" to .stylelintrc
> npx stylelint components/**/*.js
< errors
< Call npx stylelint ---fix components/**/*.js to fix some of errors automatically
> npx stylelint components/**/*.js
< no errors

@ntwb
Copy link
Member

ntwb commented Jun 21, 2018

Thanks for writing this up @jeddy3 and your reply already @ai 💯

I'm leaning towards the bundle everything option (aka the popular use cases @ai suggested above) primarily due to adding new members to the @stylelint/contributors team I'm hoping the workload can be lightened by spreading over a larger team, "many hands make light work".

@alexander-akait
Copy link
Member

Somebody can say me what is wrong with support markdown? All projects who use css/sass/less/etc and using documentation - use markdown for this. I don't see any problem with markdown (he is very popular for documentation). Linting documentation it great feature out of box. Right now we first linter who support most of popular syntaxes out of box and it is very great.

@hudochenkov
Copy link
Member

I would keep everything we have today. Except for markdown, maybe. It's nice to have, but I wouldn't consider it as necessary.

Instead of including more syntaxes follow @ai advice:

It means that instead of syntax error message we should show simple step-by-step instruction what to do to support their syntax.

As contributors, we know all (almost all) use cases. Putting instructions into CLI just a matter of documentation location and user convenience. Also, it'll onboard users, who wouldn't use stylelint, only because they didn't know about some syntax support or didn't know how to make work.

Benefits:

  1. User-friendly. Not user-friendly as an all-in-one, but provide a positive experience. Good examples with similar behavior are Jest and React. In case of an error or a warning, they provide a good explanation of a problem and a solution in a console.
  2. Users don't download more things they wouldn't use.
  3. Easily extendable.
  4. We don't introduce more complexity in our code.
  5. Support for custom syntaxes is a responsibility of the community.

In addition, I would add common use cases on a website (CSS-in-JS with template literals, CSS-in-JS with styles as objects, etc...). Our website has tons of information. It's a good thing. But on the other hand for users to achieve the desired result they need to read a lot and solve quite a puzzle.

@ntwb
Copy link
Member

ntwb commented Jun 22, 2018

3 of the 5 projects largeish projects I use stylelint with use markdown to ensure the documentation is correct.

As such if we are going to include Sass, React + CSS Modules, React + CSS-in-JS, Vue.js, CSS then I'm with @evilebottnawi and including markdown in that list would be my preference.

@jeddy3
Copy link
Member

jeddy3 commented Jun 29, 2018

I think we should be focused on giving the best experience to users, instead of thinking about standards and rational categories.

This is a valid point.

It also seems that the general consensus is to take stylelint in this direction.

Let's:

  • add postcss-styled and postcss-jsx to our bundle
  • continue to bundle postcss-less, postcss-scss, postcss-sass, sugarssandpostcss-markdown`

I believe that will provide out-of-the-box support for the most common use cases i.e. styles in:

  • separate files (in various syntaxes)
  • in style tags (in various syntaxes)
  • in style attributes
  • template literals
  • in objects
  • in markdown fences (in various syntaxes)

We can, at a later date, address weight and performance concerns by following prettier's approach of rolling-up the syntaxes into separate files.

For any future syntax, we can discuss on a per case basis whether to bundle it or provide step-by-step instructions via the command.


There is already a PR to bundle postcss-styled. @gucong3000 Would you mind updating that PR to bundlepostcss-jsx too?

@jeddy3 jeddy3 changed the title What languages should be supported out-of-the-box? Add CSS-in-JS support Jun 29, 2018
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules and removed status: needs discussion triage needs further discussion labels Jun 29, 2018
@jeddy3 jeddy3 removed this from the future-major milestone Jun 29, 2018
@jeddy3
Copy link
Member

jeddy3 commented Jul 21, 2018

Bundled support for template literals was added in #3405

Next up is bundled support for JSX.

@hudochenkov
Copy link
Member

JSX was added in #3506 and released in 9.5.0.

Thank you, @gucong3000, for making this possible. And thank you, @ai, for bringing this topic up.

@WebTravel
Copy link

Stylelint need to add --fix option for css-in-js. I would want to reopen this issue.

@hudochenkov
Copy link
Member

@WebTravel --fix works for CSS-in-JS.

@trainiac
Copy link

Linking this issue in case it helps anyone identify whey their CSS-in-JS stylelint configuration isnt working. #4018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules
Development

No branches or pull requests

8 participants