-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
fix: postinstall of css artifact #149
Conversation
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
That sounds reasonable to me, thanks @derberg! |
"generate:atomcss": "cross-env NODE_ENV=production postcss template/css/atom-one-dark.min.css -o template/css/main.min.css", | ||
"generate:maincss": "cross-env NODE_ENV=production postcss template/css/custom.css -o template/css/main.min.css", | ||
"generate:tailwind.css": "cross-env NODE_ENV=production postcss template/css/tailwind.min.css -o template/css/main.min.css", | ||
"postinstall": "rimraf \"template/css/main.min.css\" && npm run generate:atomcss && npm run generate:maincss && npm run generate:tailwind.css", |
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 have one question. Does postcss merge everything to one main.min.css
, yes?
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.
define everything
😄 there is no way to merge multiple files into one at once, nothing in the CLI that I've seen 😞
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.
So, postinstall
script overrides main.min.css
3 times, or what? 😄 EDIT: Or only appends new content?
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.
appends/merges
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 mean not a simple append but smart merge 😄
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.
LGTM
🎉 This PR is included in version 0.17.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sorry for being late. I had a look on the changes and I want to leave my opinion:
As far as i can see, that changes has already been reverted in #153
Merging all css files into one is a good idea. However you do not need to do it this way. There are easier options like @import. Postcss can handle this and its working fine with tailwind. We just needs to use We just need two files: @import "./atom-one-dark.css";
@tailwind base;
@tailwind components;
@tailwind utilities;
/* our custom css */
html,
body { ... }
.... I've tested it locally and will open a PR.
Thats fine!
What about separation of development files and generated assets / build files? Would it be possible to place our "output" files like |
@Theiaz hey man, no worries, thanks for getting back with valuable input and a PR!
|
generator support hooks into the generation process, we can have a hook that after generation removes not needed files from the output. We could actually move postcss processing to hooks too if this is needed and if possible to use postcss as a library. Here you have an example of the hook https://github.com/asyncapi/html-template/blob/master/hooks/02_renameOutFile.js |
Description
Atm artifacts are not generated during release and not pushed to npm. This PR fixes it and introduces a separate css file that is considered to be
generated-only
@chatne thanks for spotting the issue
main.min.css
that will always be.gitignore
and will be a generated artifact.postinstall
script that always generates freshmain.min.css
filemain.min.css
in the HTML, no matter the mode (singleFile or not)main.min.css
will not be pushed to npm as it will be anyway always generated after installation + it will never be updated in the repository@Theiaz @marcortw what do you think about this change?
Related issue(s)
See also #142 (comment)