Skip to content
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(@angular/cli): add --compress option #4618

Closed
wants to merge 1 commit into from
Closed

feat(@angular/cli): add --compress option #4618

wants to merge 1 commit into from

Conversation

cherryland
Copy link

@cherryland cherryland commented Feb 11, 2017

By default, gzip compression is enabled in production mode. There are several use cases, where gzip compression might not be supported and/or required

Usage

$ ng build --prod --compress=false

or

$ ng serve --prod --compress=false

Example Output

$ ng --help
ng build <options...>
  Builds your app and places it into the output path (dist/ by default).
  aliases: b
  --target (String) (Default: development)
    aliases: -t <value>, -dev (--target=development), -prod (--target=production)
  ...
  --compress (Boolean) (Default: true) use gzip compression in production mode
  ...

Basic Implementation

We simply wrap the plugin with a conditional operator and void the output, if the expression evaluates to false

  plugins: [
    ...
    buildOptions.compress ? new CompressionPlugin({...}) : (): void => null
  ]

This is perfectly fine, since Webpack installs a plugin by calling its apply method, passing a reference to the Webpack compiler object.

Related Issues

Related Pull Requests

Add compress option to toggle gzip compression in production mode
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@cherryland
Copy link
Author

Awaiting #4596 to add e2e end-to-end tests

@googlebot
Copy link

CLAs look good, thanks!

test: /\.js$|\.html$|\.css$/,
threshold: 10240
})
buildOptions.compress ?
Copy link
Member

@clydin clydin Feb 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be cleaner if a check was done above for buildOptions.compress and then the plugin was added onto extraPlugins. This should probably be moved into common instead if it's now configurable or a warning issued if it is only supported for production.

@filipesilva thoughts on this?

@@ -108,7 +108,7 @@ export default Task.extend({
stats: statsConfig,
inline: true,
proxy: proxyConfig,
compress: serveTaskOptions.target === 'production',
compress: serveTaskOptions.target === 'production' && serveTaskOptions.compress,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just be serveTaskOptions.compress once the option default is based on build target.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make sure, that the compress option will only be available in combination with -t production, but this behavior can be changed in general

@filipesilva

@@ -25,6 +25,12 @@ export const baseBuildCommandOptions: any = [
{ name: 'locale', type: String },
{ name: 'extract-css', type: Boolean, aliases: ['ec'] },
{
name: 'compress',
type: Boolean,
default: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default here can be removed as it can be set based on build target. This can be done by adding the option values here: https://github.com/angular/angular-cli/blob/master/packages/%40angular/cli/models/webpack-config.ts#L79

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done by adding the option values here: https://github.com/angular/angular-cli/blob/master/packages/%40angular/cli/models/webpack-config.ts#L79

It already is

The default here can be removed

I disagree, the default value may be redundant here, but is also included in the CLI help output

Copy link
Member

@clydin clydin Feb 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except the default is false for the development target which makes it misleading in its current form. I know it says production in the description but even a small amount of ambiguity can lead to confusion.

Either way, help should eventually support displaying target specific defaults.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hansl
Copy link
Contributor

hansl commented Feb 11, 2017

Hi @0xcherry,

After an offline discussion here with the team we decided to remove the compression plugin entirely. The reasoning is that there is an easy workaround, it's noise and the service worker is buggy with it.

Could you transform this PR with:

  1. remove compression plugin on production builds all the time,
  2. add a documentation to /doc/documentation explaining how to compress the files, in case users miss the functionality, and
  3. add a BREAKING CHANGE section to your commit message explaining that those files are not outputted anymore.

If you need any help with the above steps let me know.

Cheers!

Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Feb 14, 2017
@filipesilva
Copy link
Contributor

Superseded by #4702

filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Feb 14, 2017
filipesilva added a commit that referenced this pull request Feb 14, 2017
asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017
@iamdenny
Copy link

I can't find compress doc in https://github.com/angular/angular-cli/tree/master/docs/documentation
please let me where it is.

@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 12, 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.

None yet

6 participants