linaria icon indicating copy to clipboard operation
linaria copied to clipboard

postcss-linaria (custom syntax for stylelint v14)

Open kutnickclose opened this issue 3 years ago • 8 comments

Motivation

Begin addressing https://github.com/callstack/linaria/issues/930 (@linaria/stylelint does not work with stylelint ^14.0.0) by creating the postcss-linaria package and accompanying stylelint-config-standard-linaria package.

Summary

For stylelint 13, @linaria/stylelint uses a preprocessor to create a css-ish file before running linting tests on it.

For stylelint 14, stylelint discourages using a (pre)processor in favor of a custom syntax that essentially tells stylelint how to work with a non-css file.

Overview of postcss-lit custom syntax

postcss-lit is a custom syntax for lit templates and one of the few current examples of css-in-js custom syntaxes. For simple use cases of linaria, it works. However, it doesn't quite work for linaria out of the box because its placeholder strategy for expressions only uses comments.

For example, postcss lit will take source code like

const expr = "color: black";
css`
  ${expr}
`

and parse into an ast that includes css like

/* POSTCSS_LIT:0 */

where the ${expr} was replaced by a comment. The 0 in the comment helps the comment get turned back into an expression during the stringification process. This passes lint as there is nothing wrong with the comment. Other non-expression css is linted as usual.

However, once you get more use cases of expressions, the linter throws errors. For example,

const expr = 'color';
const expr2 = 'black';
css`
  ${expr}: ${expr2}
`

and parse into an ast that includes css like

/* POSTCSS_LIT:0 */: /* POSTCSS_LIT:1 */

which will throw a syntax error due to the colon without properties or values before or after it.

Overview of postcss-linaria

postcss-linaria (or @linaria/postcss-linaria if there's a preference) in this PR borrows heavily from postcss-lit but uses a slightly more advanced placeholder strategy. Instead of just using comments, we can add other placeholders that will avoid syntax errors.

For example, we can parse

css\`
      \${expr0}
      .foo {
        \${expr1}: \${expr2};
      }

      \${expr3} {
        .bar {
          color: black;
        }
      }
      \${expr4}
    \`;
  `,

into

   /* pcss-lin:0 */
   .foo {
     --pcss-lin1: pcss-lin2;
   }

   .pcss-lin3 {
     .bar {
       color: black;
     }
   }
   /* pcss-lin:4 */

The number next to each linaria placeholder helps map this value back to an expression during stringification.

ruleset placeholder =  /* pcss-lin:# */
atRule/Selector placeholder = .pcss-lin#
property placeholder = --pcss-lin#
value placeholder = pcss-lin#

One downside is that we'll need to include one modified linting rule to ignore comments with postcss-linaria in them since this rule is added by stylelint-config-standard. The other downside is that we're linting placeholders rather than the fully generated code.

Test plan

There are a bunch of tests in the test folder. That will give us a way to keep accounting for more and more use cases. I will be working on testing this in my company's large linaria code base. The stylelint config would need to look like

module.exports = {
  extends: ['stylelint-config-standard-linaria'],
}

TODO

  • [x] test on actual code in company codebase (100K+ test suites)
  • [x] tests for locationCorrection
  • [x] tests for utils file
  • [x] add docs

kutnickclose avatar Sep 09 '22 22:09 kutnickclose

@Anber, I was hoping to get alignment on the general approach here. Would you have time to review? (in priority order)

  1. Would you be open to accepting two new packages (postcss-linaria and stylelint-config-standard-linaria) in the linaria monorepo or should they live outside the repo? (the names are based on stylelint conventions)
  2. Any general feedback or code review appreciated!

kutnickclose avatar Sep 15 '22 23:09 kutnickclose

@Anber any chance review would be possible?

kutnickclose avatar Sep 21 '22 17:09 kutnickclose

Hi @kutnickclose! Thank you for such a great job!

  1. Would you be open to accepting two new packages (postcss-linaria and stylelint-config-standard-linaria) in the linaria monorepo or should they live outside the repo? (the names are based on stylelint conventions)

Yes, sure.

  1. Any general feedback or code review appreciated!

I'll take a look in a couple of days. Sorry for the delay.

Anber avatar Sep 21 '22 18:09 Anber

Test an example:

styled.h1.goo { width: ${p => p.size}px; }``

usmanyunusov avatar Sep 21 '22 22:09 usmanyunusov

Test an example:

styled.h1.goo { width: ${p => p.size}px; }``

@usmanyunusov I have one test here: https://github.com/callstack/linaria/pull/1065/files#diff-daad0281133eae62fd49cb8f51d688b3d1fbafe22db18e04da1cc815616310a8R246

Is there a particular reason you're requesting another example?

kutnickclose avatar Sep 21 '22 23:09 kutnickclose

Auto fix replaces function with this pcss-lin0px

usmanyunusov avatar Sep 22 '22 06:09 usmanyunusov

@usmanyunusov I believe I fixed the issue you brought up and added a test for it here: https://github.com/callstack/linaria/pull/1065/files#diff-ceb627f577c7f458f7ad9f8bb0d5fe262c6ead0d285f4fba35bb86631063f9abR59

kutnickclose avatar Sep 22 '22 23:09 kutnickclose

Hi @Anber, thank you for the review! I updated the namespace of the packages and ran pnpm changeset. I think there may be updates to master that I don't have here so I was little confused about what to do with package versions. Please let me know if I should change from patch to minor since it's a new feature and if I need to update the package.json to start at 4.1.3 (currently 4.1.2) or something else.

kutnickclose avatar Sep 23 '22 17:09 kutnickclose