43081j / postcss-lit

PostCSS syntax for extracting CSS from template literals inside JS/TS files
84 stars 6 forks source link

Base indentation detection #16

Closed 43081j closed 2 years ago

43081j commented 2 years ago

This detects the base indentation (that of the last backtick of the template) and removes it from the CSS source before parsing it.

We need this so we don't trigger stylelint's indentation rules incorrectly.

Fixes #14

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 1542386357

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Files with Coverage Reduction New Missed Lines %
lib/locationCorrection.js 1 91.06%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 1508993096: -1.3%
Covered Lines: 444
Relevant Lines: 449

💛 - Coveralls
43081j commented 2 years ago

turns out that implementation was nonsense. have updated to sort it out, should be fine now.

also added a test which simply enables stylelint's indentation rule in the hope that it'd fail if we did something wrong here

43081j commented 2 years ago

@abdonrd @web-padawan could you possibly give postcss-lit@next a try? and let me know if you run into any problems

web-padawan commented 2 years ago

Thanks, I will try it tomorrow 👍

43081j commented 2 years ago

i gave it a go, i think we still have one last problem: accounting for the first newline.

css`
  .foo {}
`;

The first new line after css will cause stylelint to complain about no empty lines being allowed at the start. so we should just ignore/strip it i think

web-padawan commented 2 years ago

@43081j Tried to use it with our web components monorepo and faced an error:

$ stylelint packages/**/theme/**/*-styles.js
SyntaxError: Unexpected token (1:0)
    at Object._raise (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:541:17)
    at Object.raiseWithData (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:534:17)
    at Object.raise (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:495:17)
    at Object.unexpected (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:3580:16)
    at Object.parseExprAtom (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:12026:22)
    at Object.parseExprSubscripts (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:11584:23)
    at Object.parseUpdate (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:11564:21)
    at Object.parseMaybeUnary (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:11539:23)
    at Object.parseMaybeUnary (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:9918:20)
    at Object.parseMaybeUnaryOrPrivate (/Users/serhii/vaadin/web-components/node_modules/postcss-lit/node_modules/@babel/parser/lib/index.js:11353:61)

Here is the branch: https://github.com/vaadin/web-components/tree/stylelint-14 In order to try it out, run yarn and then yarn lint:css.

43081j commented 2 years ago

strange, that sounds like there's syntax in your code that the babel parser didn't understand. ill see if i can reproduce it locally tonight

43081j commented 2 years ago

@web-padawan this is because stylelint-config-vaadin is still loading the styled-components preprocessor. that turns your stylesheet into some strange nested stylesheet, which the stylelint parser doesn't understand (since nesting isn't a thing yet).

i removed it locally and it then lints correctly, but has the same "rule-empty-line-before" problem i mentioned above, ill fix that tonight

web-padawan commented 2 years ago

Thanks for looking into it. I will put together a new version of the Stylelint config tomorrow.

43081j commented 2 years ago

@web-padawan i published a new @next build and ran it against your repo with styled-components disabled, it all passes! it finds some legitimate warnings, some spacing between keyframes which i guess is an improvement or change in stylelint's rules since 14.x.

I had to put a bit of a hack in my PR which has been written up here: stylelint/stylelint#5767 for anyone curious.

43081j commented 2 years ago

i've tested this against our repos too and it seems to work fine, so im gonna go ahead and merge.

if you run into any problems, can you please open another issue?

web-padawan commented 2 years ago

@43081j Thanks! Could you please also publish 0.1.3 so we can use it in the project?

43081j commented 2 years ago

i've just published 0.2.0 @web-padawan

abdonrd commented 2 years ago

@43081j it doesn't work well for me 😕

With the same branch: https://github.com/IBM/pwa-lit-template/compare/stylelint-14 If I run npm run lint:stylelint -- --fix, it moves the indentation:

Screenshot 2021-12-09 at 00 33 55
43081j commented 2 years ago

@abdonrd can you possibly raise a new issue and ill try look into it tomorrow

will run it against your branch and see where i get to

abdonrd commented 2 years ago

Done! https://github.com/43081j/postcss-lit/issues/18

web-padawan commented 2 years ago

@43081j I got it working in our web components monorepo, many thanks for putting this together! ❤️ Here is a PR in case you want to take a look: https://github.com/vaadin/web-components/pull/3166.

Some interpolations were causing "comment-whitespace-inside" errors so I removed them, for example:

const richTextEditorStyles = css`
  ${contentStyles}
  ${toolbarStyles}

:host([readonly]) [part='toolbar'] {
    display: none;
  }
`;

The errors are caused by ${contentStyles} and ${toolbarStyles} lines apparently treated as comments 🤷‍♂️

43081j commented 2 years ago

ah yes, this is because we replace expressions with comments. but i opened #6 for this, as we may need to be smarter about what we use as a placeholder (e.g. values in value positions, comments elsewhere).

i'll add a test like that at some point and see what i can do to make it better