Open sthzg opened 7 years ago
Ahh, right - this property... I am on it.
@stylesuxx let me know if I can be of any assistance with testing/fixing this bug.
@stylesuxx / @sthzg: Maybe we should add the default key and remove the question on installing if this is possible? Users would then only have to install the postcss node modules per hand and the config would not have to be altered as it is today. What do you think about this?
@weblogixx just a short follow-up: @stylesuxx came up w/ a PR that fully integrates the PostCSS functionality again https://github.com/react-webpack-generators/generator-react-webpack/pull/313. The only thing left to merge this PR in is a final, manual check.
Hi @sthzg,
nice, will also have a look at it. I really dislike the way we are reparsing the config with esprima in the current version, just wanted to remove this if it is possible.
Imho it would have no negative effect to add it by default. If the user chooses not to use it, nothing changes, if he wants to use it, he simply adds the plugins.
We could then handle it via non ast parsing. Let me know how I should handle it, should not be too much of a hassle.
Am 25.11.2016 17:26 schrieb "Chris" notifications@github.com:
Hi @sthzg https://github.com/sthzg,
nice, will also have a look at it. I really dislike the way we are reparsing the config with esprima in the current version, just wanted to remove this if it is possible.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/react-webpack-generators/generator-react-webpack/issues/307#issuecomment-262991455, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7tELjR_V-gdKQwBLTL4HoISReUYi-Mks5rBwxSgaJpZM4K5UG_ .
@stylesuxx, yes, that would be the way to go for me. In this case, we just would have to remove the question (add postcss support) when using v4 and up, so v3 would still implement it.
Cool, then I defer the manual testing until after these new changes.
@stylesuxx I started to prepare the gen-side in above's commit (cece8d0).
The error on Travis is on purpose, as the updated tests require removing the postcss
and postcss-loader
deps on the template repo (I've commented on the PR there).
Maybe you could also look over this PR. I am not sure if we could get rid of some of the deps that were necessary for AST processing in the postcss.js
install step (now removed, some of the AST deps are required by the setup-env
subgen though).
IMO we can now finish testing with the combination of
and then maybe finally ship this thing 🙌 ...
app.css --> @imports --> one.css --> @imports --> two.css
imports, font import
imports, font import
nesting, vars through import, mixin through import, font import
nesting, vars through import, mixin through import, font import
nesting, vars through import, mixin through import, font import
nesting, vars through import, mixin through import, font import
nesting, vars through import, mixin through import, font import
nesting, vars through import, mixin through import, font import
Thank you so much for all your testing, I adapted the deps in the template. Should I to the 3 last checks or are you on it?
@stylesuxx only the stylus tests are left. one thing that puzzles me is that I wasn't able to recreate the problems I had w/ scss in a real-world project. my take on that is that I consider it working until I can distill the error out of that other project and am able to get a test-case up. I'll have enough time this evening to do both *.styl
tests. when that works I think we can ping weblogixx to get a final verdict. :)
Awesome work! @weblogixx could you take look over both PRs?
Once 1) is merged and published I would trigger Travis on 2) again so that it (hopefully) turns green.
Hi @sthzg and @stylesuxx,
just had a look at both prs. Seems really nice! Thank you for your continued work for this project. Keep up the good work! 👍
When installing with PostCSS support, the
postcss
config property seems to be inserted at the wrong place (the config fails webpack configuration)./cc @stylesuxx