-
Notifications
You must be signed in to change notification settings - Fork 155
added support postcss #195
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
Conversation
lib/compilers/postcss-compiler.js
Outdated
module.exports = (content) => { | ||
let css = null | ||
|
||
postcss(plugins) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can run postcss synchronously: https://github.com/vuejs/component-compiler-utils/blob/37b4a6a99ecaa8bf27c777191ac17e9168c70cd1/lib/compileStyle.ts#L115
It means asynchronous plugins won't be applied, but I think that's is fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added check for async plugin, i think it is more correct way, because we have async plugin and processing postcss does not end properly
@eddyerburgh pls take a look again (: |
package.json
Outdated
"extract-from-css": "^0.4.4", | ||
"find-babel-config": "^1.1.0", | ||
"js-beautify": "^1.6.14", | ||
"node-cache": "^4.1.1", | ||
"object-assign": "^4.1.1", | ||
"postcss": "^7.0.17", | ||
"postcss-load-config": "^2.1.0", | ||
"postcss-nested": "^4.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be in devDependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
}) | ||
|
||
if (result.processing) { | ||
prevCheckIsAsync = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we could return early at this point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Looks good, I think there's a dependency that should be a devDep, and there's a minor nit |
Since postcss is now not a dependency, we should only require |
@eddyerburgh Any chance you could have a look at it, as it would be great to have proper support for PostCSS rather sooner than later. 😉 |
Hello @eddyerburgh! What about merge pull request ? I see, mr is completed, or have some problem ? I think it is a good, solution, and we await this feature in vue jest a lot of time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d love to see the readme.md
updated to reflect these changes, to make sure we provide examples for using vue-jest
with PostCSS. Could you please update the relevant section?
Sorry this repo has been neglected for so long! I will try and breathe some more life into it. Let's merge this. I don't know anything about PostCSS, but tests all seem to be passing. Is there some PostCSS specific options? It looks like this PR will load them automatically using the idiomatic postcss config file, so I'll just add a note that this is supported for now.
Edit: I just realized this was targeting v3. I can do a release for v3. I tried merging this into v4 (current master) but the conflicts are pretty significant. I wonder what it would take to port this to v4. |
No description provided.