Skip to content

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

Merged
merged 1 commit into from
Jan 12, 2017

Conversation

filipesilva
Copy link
Contributor

@filipesilva filipesilva commented Jan 10, 2017

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 in ng build.

@intellix can you verify this solves your long reloads for scss?

Partially address #1980

@filipesilva filipesilva force-pushed the fix-css-build--speed-regression branch from bfd51ef to bc6b28d Compare January 11, 2017 00:03
@intellix
Copy link
Contributor

intellix commented Jan 11, 2017

Great improvement :)

b24:
styles.scss: ~12s
my-cmp.scss: ~3.5 - 5s

this PR:
styles: ~5-6s
some-cmp: ~3s

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';

@filipesilva filipesilva force-pushed the fix-css-build--speed-regression branch from bc6b28d to 8cd2a3d Compare January 11, 2017 00:39
@filipesilva filipesilva force-pushed the fix-css-build--speed-regression branch from 8cd2a3d to 3f2919a Compare January 11, 2017 13:59
@@ -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';
Copy link
Member

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?

Copy link
Contributor Author

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.

@montella1507
Copy link

@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.
@filipesilva filipesilva force-pushed the fix-css-build--speed-regression branch from 3f2919a to 7a7733e Compare January 12, 2017 14:28
@filipesilva
Copy link
Contributor Author

filipesilva commented Jan 12, 2017

@montella1507 for global CSS/etc, yes. Component CSS/etc still will need reloads.

@montella1507
Copy link

@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 :-)

@hansl
Copy link
Contributor

hansl commented Jan 12, 2017

LGTM.

@hansl hansl merged commit 87536c8 into angular:master Jan 12, 2017
@filipesilva filipesilva mentioned this pull request Jan 15, 2017
@Meligy
Copy link
Contributor

Meligy commented Jan 15, 2017

Does this mean no global CSS watch in dev?

@filipesilva filipesilva deleted the fix-css-build--speed-regression branch January 16, 2017 12:26
@filipesilva
Copy link
Contributor Author

@Meligy I don't quite understand your question. Can you rephrase?

@Meligy
Copy link
Contributor

Meligy commented Jan 16, 2017

Does this PR affect the default behaviour of watching global CSS files and rebuilding (re-bundling) them on change when running ng build / ng serve in dev target/env?

@filipesilva
Copy link
Contributor Author

@Meligy nope. But the css will be served in js bundles instead of css files in ng serve, which was how it worked since beta 15 or something. If you absolutely need the css files in dev rebuilds, do ng serve --extract-css. It's gonna be slower though.

@Meligy
Copy link
Contributor

Meligy commented Jan 16, 2017

Aha. Makes sense. Good stuff!

@montella1507
Copy link

i have done some benchmarks:

gulp-sass (build and copy generated .css to src folder)+ angular-cli (watch detech + rebuild):
150ms + 18sec

angular-cli only (styles:["style.scss"]) + --extract-css:
35sec
¨:-/

@filipesilva
Copy link
Contributor Author

@montella1507 extracting CSS in webpack is always slow I believe :/

MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
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.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants