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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support for postcss 8 plugins #200

Conversation

m4thieulavoie
Copy link

@m4thieulavoie m4thieulavoie commented Sep 16, 2020

Notable Changes

This PR is aiming to support postcss 8 plugins. Not sure if I cover everything in there, or if I should be adding any additional tests

Commit Message Summary (CHANGELOG)

Support for postcss 8 plugins

feat: support for postcss 8 plugins

Type

  • Feature

SemVer

  • Feature (:label: Minor)

Issues

Checklist

  • Lint and unit tests pass with my changes
  • I have added tests that prove my fix is effective/works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes are merged and published in downstream modules

@coveralls
Copy link

coveralls commented Sep 16, 2020

Coverage Status

Coverage remained the same at 98.058% when pulling efc9ebb on m4thieulavoie:feat/support-postcss-8-plugins into fda5f4a on michael-ciniawsky:master.

@ai
Copy link
Member

ai commented Sep 16, 2020

@michael-ciniawsky can you give me access to the repo and npm package?

Copy link

@RyanZim RyanZim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do Not Merge; this breaks linting, and I am 99% sure this does not function correctly.

@m4thieulavoie
Copy link
Author

Do Not Merge; this breaks linting, and I am 99% sure this does not function correctly.

Can you expand on that? Happy to adjust

@RyanZim
Copy link

RyanZim commented Sep 17, 2020

Refs postcss/postcss#1410

Can you expand on that? Happy to adjust

Specifically, I'm not sure what this logic is doing, but I'm pretty sure it's not doing its intended purpose: https://github.com/michael-ciniawsky/postcss-load-config/blob/b03d1950e243f16cd3feb6d9948d35de37cefe37/src/plugins.js#L59-L61 Also, not sure what this plugin.plugins check is even there for. I tested last night, it wasn't broken as I expected, but I'm still not sure this logic is correct; would need to sit down and look closer to be sure.

@m4thieulavoie
Copy link
Author

Refs postcss/postcss#1410

Can you expand on that? Happy to adjust

Specifically, I'm not sure what this logic is doing, but I'm pretty sure it's not doing its intended purpose:

https://github.com/michael-ciniawsky/postcss-load-config/blob/b03d1950e243f16cd3feb6d9948d35de37cefe37/src/plugins.js#L59-L61

Also, not sure what this plugin.plugins check is even there for. I tested last night, it wasn't broken as I expected, but I'm still not sure this logic is correct; would need to sit down and look closer to be sure.

Alright, sounds like I'll need to wait for you to come back to me with your conclusion. Please keep me posted, or feel free to scrap that PR with a proper fix if that's easier for you 👍

@ai
Copy link
Member

ai commented Sep 17, 2020

We need a different way to fix #202

@ai ai closed this Sep 17, 2020
@m4thieulavoie m4thieulavoie deleted the feat/support-postcss-8-plugins branch September 18, 2020 11:51
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.

PostCSS 8 support
4 participants