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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to postcss 8, update dependencies #15

Merged
merged 2 commits into from
Feb 10, 2022

Conversation

bjentsch
Copy link
Contributor

@bjentsch bjentsch commented Feb 1, 2022

Hi folks

As mentioned in #13 and in postcss/postcss-nested#139 (comment), this library should be upgraded to postcss@8.

I tried to keep the invasiveness of the changes at a minimum to make sure to port the exact functionality of the plugin. The only real changes are the ones suggested in the PostCSS 8 upgrade guide, such as using the updated basic API (OnceExit) and declaring module.exports.postcss = true;.

For code style's sake, I tested prettier on the project and thought it would make a nice addition, hence the .prettierrc file. I didn't add a hard dependency here though, and I kept the original formatting for the project for this PR. I don't have a strong opinion on this though, and if you don't like prettier, I'm open to removing the .prettierrc file again.

Apart from that, I bumped all the other dependencies to their respective latest versions and updated the README.

Let me know if this PR is any good 馃

@Antonio-Laguna
Copy link
Member

@bjentsch thanks a lot for taking the time to do this. I'll take a closer look, looks great at first sight! Nice work!

It might take some time as I'm not familiar with this plugin (yet)

Will keep you updated!

@Antonio-Laguna Antonio-Laguna self-assigned this Feb 2, 2022
@Antonio-Laguna
Copy link
Member

@bjentsch thanks for your patience with this! I'm keen to merge this and also to release it. That said I'd love to keep parity on what we've done for the rest of the plugins as much as possible.

That means, we'd be better without prettier (as you kind of hinted) but eslint enforcing it. I'll set up GitHub actions for this plugin once it's merged. We will also remove source maps as we haven't seen them useful in any reports.

Would you want to do this as part of the PR or would you like us to do that after merging it?

Thanks a lot again!

@bjentsch
Copy link
Contributor Author

Hey @Antonio-Laguna, never mind, I'll remove the prettier contents and update the PR. 馃槂

As for the other changes (eslint, source maps) I'd be glad if you could do it afterwards.

@Antonio-Laguna
Copy link
Member

Sounds like a plan!

@bjentsch
Copy link
Contributor Author

Removed the prettier related stuff 馃槂

@Antonio-Laguna
Copy link
Member

Nice, thanks a lot!

@Antonio-Laguna Antonio-Laguna merged commit 3c770b6 into csstools:master Feb 10, 2022
@Antonio-Laguna
Copy link
Member

@bjentsch this got published as 4.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants