43081j / postcss-lit

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

fix: account for afters of non-root nodes #19

Closed 43081j closed 2 years ago

43081j commented 2 years ago

Fixes #18

cc @abdonrd

I've published postcss-lit@next if you want to try it

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 1579970728


Files with Coverage Reduction New Missed Lines %
lib/locationCorrection.js 4 89.42%
<!-- Total: 4 -->
Totals Coverage Status
Change from base Build 1553599349: -1.3%
Covered Lines: 591
Relevant Lines: 599

💛 - Coveralls
abdonrd commented 2 years ago

I just tried it! It seems to solve the problem from before, but a new one arises. See the multiline main:

Screenshot 2021-12-11 at 13 50 06
43081j commented 2 years ago

@abdonrd thanks for giving it ago, i've published a new @next, could you try it out?

this was caused by the fact postcss stores the entire selector in a selector property, containing the whitespace too. so i had to mutate that when stringifying (and have opened postcss/postcss#1691).

abdonrd commented 2 years ago

Thanks @43081j! Now it works well in the IBM/pwa-lit-template! 👏🏻

But trying in another project (with the same config) I see more changes:

Screenshot 2021-12-13 at 12 11 41 Screenshot 2021-12-13 at 12 14 00

Also leaving an entire file empty:

Screenshot 2021-12-13 at 12 12 51
43081j commented 2 years ago

😞 its the same problem in a different place.

i think i need to solve it globally somehow.

basically, various AST nodes have string values in them (e.g. rule.selector, decl.value, etc). they contain the surrounding whitespace, rather than using the before/after strings we correct.

in this case, the declaration's value needs correcting just the same way we corrected the rule's selector.

the empty file thing is another issue but im sure that'll be an easy fix.

thanks so much for testing, ill see if i can sort them tonight

43081j commented 2 years ago

just an update...

by some miracle i think i may have fixed it for all node types. tomorrow i'll add more tests to confirm this

there's a lot of places indentation can exist so i just need to make sure we have tests for enough of them

edit: i've published a new @next for you to try out @abdonrd

abdonrd commented 2 years ago

Thanks @43081j!

Now just this auto fix:

Screenshot 2021-12-14 at 01 05 00

But I don't know if it is a postcss-lit thing or it's because Stylelint v14.

abdonrd commented 2 years ago

Well, and still happens that component files are empty.

Screenshot 2021-12-13 at 12 12 51
43081j commented 2 years ago

@abdonrd the first one is stylelint 14.x, they introduced a new recommended rule which migrates all rgb values to space-separated. rgba(r, g, b, a) becomes rgb(r g b / a)

we had the same huge change in our repos :D was a surprise to me too

i'll try fix the empty file thing tonight

43081j commented 2 years ago

@abdonrd i've fixed the empty file problem now. published a new @next

abdonrd commented 2 years ago

Yay! It works now, thanks @43081j ! 👏

43081j commented 2 years ago

awesome, ill get it published soon as i can.

thanks so much for testing