common chunk optional 1.2 update

[coryrylan]

Bug Report or Feature Request (mark with an x)

• bug report -> please search issues before submitting

Versions.

@angular/cli: 1.2.1
node: 8.1.4
os: win32 x64
@angular/animations: 4.1.3
@angular/common: 4.1.3
@angular/compiler: 4.1.3
@angular/core: 4.1.3
@angular/forms: 4.1.3
@angular/http: 4.1.3
@angular/platform-browser: 4.1.3
@angular/platform-browser-dynamic: 4.1.3
@angular/platform-server: 4.1.3
@angular/router: 4.1.3
@angular/cli: 1.2.1
@angular/compiler-cli: 4.1.3
Windows 10

Repro steps.

The commons chunk update that occurred here causes a performance regression 2090f64

The use case, we have three NgModules. Module A, B and C. All three are lazy loaded separately quite large at 50kb each. Module C is lazy loaded but depends on Module A and B. Because Module C depends on A and B the modules are moved into a common chunk and leaves the lazy loaded modules empty with just the module definition code. Now module A, B and C are now all bundled into a common bundle and no longer lazy loaded. Now when user navigates to Feature A all 150kb of feature A, B and C are loaded.

Not sure if there is a fix to this or our best option is to eject. Maybe proposal to make the common chunk optional behind a config? The common bundle removed duplicated code but essentially removed the benefit of lazy loaded modules if one of those happens to depend on each other.

In certain apps with certain use cases having common chunks configurable would be necessary. It would seem that larger apps would likely run into our similar use case.

The log given by the failure.

Desired functionality.

Would be glad to get a pull request to make this optional in the CLI.

Mention any other details that might be useful.

[kevinbuhmann]

An easy fix for this would be to make it possible to disable the common chunk (i.e. use the previous behavior) via a build command arg (alongside --vendor-chunk).

[coryrylan]

This pull request gives the most flexibility I think. It allows the dev to determine the best trade off with their app. More code up front and less duplication vs more duplication and less code up front.

The CommonsChunkPlugin is going to be at odds with the NgModules. I think this could happen with third party code NgModules as well. A third party module is loaded for two of 30 other feature modules. Now all 30 features must aggressively load the two common chunks that only two features need.

[coryrylan]

Additional notes to this case it looks like our project is now running into this issue (6196) after upgrading from cli 1.1 to 1.2 and continues to be broken in both situations on 1.3 beta

It looks like it could be related to the common chunk plugin. Which would make sense because we ran into this issue and the one above at the same time when upgrading to 1.2. If we can get this pull request in this will be able to help with anyone on #6196 as well.

Here is the source issue in webpack
webpack/webpack#959

[coryrylan]

@filipesilva Here is a demo repo you can clone and see the issue. There are three lazy loaded feature modules A, B and C. C relies on A and B so the common chunks removes the A and B components moving them into the common chunk and leaving empty lazy loaded NgModule definitions.

https://github.com/coryrylan/test-proj

[filipesilva]

Thank you for taking the time to do this repro.

I agree that there should be an option to turn it off, but expect that it is overall better to have the common-chunk.

It is true that the first lazy-loaded chunk can be larger than necessary due to dependencies of other lazy-loaded chunks, but it is also true that any chunk but the first will be significantly smaller. Being able to chose is important though.

I don't see the hard connection between this issue and #6196 though. I understand that they might be related from the webpack thread, but it doesn't seem like that's the whole story.

[coryrylan]

Yeah not quite sure how they are related either but just noticed we ran into that same issue at the same time.

On this situation I'm not quite sure what the best thing to do would be for the overall performance health of the app. In our situation having the first load and js smaller is more important as a public facing app. We have the service worker lazy loading those additional modules in the background so having those be a little bigger is not an issue for us. I think it really depends on the app and its goals and what performance trade offs to make.

Ideally being able to adjust the min chunk value would give us the most flexibility because we would like the usefulness of common chunks but not such an aggressive setting value of 2. But I also understand being careful about not exposing to many webpack settings in the public api.