-
Notifications
You must be signed in to change notification settings - Fork 12k
feat(build): add --extract-css flag #3943
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(build): add --extract-css flag #3943
Conversation
bfd51ef
to
bc6b28d
Compare
Great improvement :) b24: this PR: For a bit of context as to how large my styles.scss is, I'm using FA + bootstrap 4: /* You can add global styles to this file, and also import other style files */
@import 'scss/variables.scss';
@import 'scss/mixins/range-background-variant.scss';
@import 'node_modules/font-awesome/scss/font-awesome';
@import "node_modules/bootstrap/scss/bootstrap";
@import 'scss/base';
@import 'scss/loader';
@import 'scss/forms';
@import 'scss/range';
@import 'scss/tables';
@import 'scss/buttons';
@import 'scss/navs';
@import 'scss/modals';
@import 'scss/cards';
@import 'scss/dropdown';
@import 'scss/app-theme-dark'; |
bc6b28d
to
8cd2a3d
Compare
8cd2a3d
to
3f2919a
Compare
@@ -20,6 +20,9 @@ export default function serveRun(commandOptions: ServeTaskOptions) { | |||
} | |||
} | |||
|
|||
// default to extractCss to true on prod target | |||
commandOptions.extractCss = commandOptions.extractCss || commandOptions.target === 'production'; |
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.
Won't this always be true for production?
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.
Right, bad logic there. Will fix.
@filipesilva hello, does it mean, that any change in global CSS file (or scss file) WILL NOT proc entire rebuild process? It will be just recompiled by something like node-sass and the page will be refreshed? (would it be possible to hot-reload/live reload styles?) |
Having css extraction on all the time makes dev rebuild times very slow for global styles. This flag defaults to false on dev targets and true on prod targets.
3f2919a
to
7a7733e
Compare
@montella1507 for global CSS/etc, yes. Component CSS/etc still will need reloads. |
@filipesilva oh jesus, this would be the best commit ever :D if it works, it would be absolutely perfect :-) My HTML/CSS designer will love you for that, because he wont need to rebuild entire page with every little change :-) |
LGTM. |
Does this mean no global CSS watch in dev? |
@Meligy I don't quite understand your question. Can you rephrase? |
Does this PR affect the default behaviour of watching global CSS files and rebuilding (re-bundling) them on change when running |
@Meligy nope. But the css will be served in js bundles instead of css files in |
Aha. Makes sense. Good stuff! |
i have done some benchmarks: gulp-sass (build and copy generated .css to src folder)+ angular-cli (watch detech + rebuild): angular-cli only (styles:["style.scss"]) + --extract-css: |
@montella1507 extracting CSS in webpack is always slow I believe :/ |
Having css extraction on all the time makes dev rebuild times very slow for global styles. This flag defaults to false on dev targets and true on prod targets.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Having css extraction on all the time makes serve rebuild times very slow for global styles.
This flag defaults to false on dev targets and true on prod targets for
ng serve
, and defaults to false inng build
.@intellix can you verify this solves your long reloads for scss?
Partially address #1980