search

shellscape / postcss-less

PostCSS Syntax for parsing LESS
MIT License
122 stars 39 forks source link

refactor: Update to PostCSS 8. #151

Closed ludofischer closed 3 years ago

ludofischer commented 3 years ago

Fixes: #150

BREAKING CHANGE: drop Node 8 support, as PostCSS 8 requires Node >= 10.

Which issue # if any, does this resolve?

150

Please check one:

This PR:

I’ve looked at this this commit to get an idea of the required changes. In the end only the tests needed to change imports. In theory you could release a version that’s compatible with both PostCSS 7 and 8, if you kept a separate test suite.

ludofischer commented 3 years ago

It looks like the CircleCI tests fails at the security checks because npm audit reports some vulnerabilities in the dev dependencies.

shellscape commented 3 years ago

Thanks for putting this together. I'll get that audit fixed up for you here shortly.

shellscape commented 3 years ago

Updated. Please merge from upstream/master.

ludofischer commented 3 years ago

Merged upstream, Node 8 tests fail as expected (latest ava drops Node 8 support).

shellscape commented 3 years ago

Gah. That's right. I'll clean up the CI config for you here.

ludofischer commented 3 years ago

@shellscape One PostCSS maintainer (@ai) suggested on Gitter to copy the postcss/lib/tokenize.js into the parser’s own tree instead of importing it, as it’s not meant to be a public API and relying on it has caused breakage in the past. What do you think about this?

shellscape commented 3 years ago

@ludofischer I have a great deal of respect for Andrey, and he's helped me and other postcss-related projects out more times than I can count. But on this particular issue, he and I disagree. I adopted use of the internal classes to ease the burden of maintaining a separate parser and tokenizer. Even if the base classes in PostCSS change, it's still a trivial update in this package. Whereas maintaining both myself was a massive burden rife with edge cases that had already been accounted for in the PostCSS base classes. Copy/pasting theirs into this project also seemed futile to me and I ruled that out.

All in all, I'm happy to update on my end when the base classes change. And as this is mean to be a development-time-only package, I don't see an issue with two separate copies of PostCSS, should a parent package/project use a newer version that's out of range with the one that postcss-less depends upon.

shellscape commented 3 years ago

thanks!