Skip to content

It is impossible to lazy load libraries correctly #14830

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

Closed
RobM-ADP opened this issue Jun 19, 2019 · 11 comments
Closed

It is impossible to lazy load libraries correctly #14830

RobM-ADP opened this issue Jun 19, 2019 · 11 comments

Comments

@RobM-ADP
Copy link

RobM-ADP commented Jun 19, 2019

🐞 Bug report

Command (mark with an x)

- [ ] new
- [x] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Is this a regression?

Not that I know of

Description

When lazy loading bundles via a library project as described here: #6373 (comment) the bundling of the routes is incorrect. There does not seem to be a way to correctly lazy-load mutliple routes from the same library and have the code chunked correctly. See the repo below

🔬 Minimal Reproduction

Clone and run the following repo and you will notice that while the "wrapper modules" are correctly lazy loaded, the actual modules and components from the library are all bundled into the common.js bundle which completely defeats the purpose of lazy loading. It seems that when lazy loading routes from a library, each route would need to be its own library or secondary entry point in order for the bundling to work correctly which doesn't make sense when using the libraries for feature development as a feature could consist of multiple lazy-routes.

Minimal repro: https://github.com/RobM-ADP/lazy-load-bug

🌍 Your Environment


$ ng version

Angular CLI: 8.0.3
Node: 10.15.3
OS: darwin x64
Angular: 8.0.1
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.800.3
@angular-devkit/build-angular      0.800.3
@angular-devkit/build-ng-packagr   0.800.3
@angular-devkit/build-optimizer    0.800.3
@angular-devkit/build-webpack      0.800.3
@angular-devkit/core               8.0.3
@angular-devkit/schematics         8.0.3
@angular/cli                       8.0.3
@ngtools/json-schema               1.1.0
@ngtools/webpack                   8.0.3
@schematics/angular                8.0.3
@schematics/update                 0.800.3
ng-packagr                         5.3.0
rxjs                               6.5.2
typescript                         3.4.5
webpack                            4.30.0

common.js bundle

Notice that components from both Module AAA and Module BBB are in here.

common js

aaa-wrapper-module.js bundle

Notice that the only thing in here is the wrapper itself. So lazy-loading has really added no benefit.

Module AAA wrapper bundle

@statop
Copy link

statop commented Jun 19, 2019

To add to this, here is a second demo app -
https://github.com/statop/lazy-load-bug

It has specific examples

@RobM-ADP RobM-ADP changed the title Lazy loaded routes from library not bundled as expected It is impossible to lazy load libraries correctly Jun 19, 2019
@alan-agius4
Copy link
Collaborator

Hi all, this issue is pretty much the same as #13635 and there is a pretty good explanation on why this happens in ng-bootstrap/ng-bootstrap#2883 (comment)

In the case of @RobM-ADP, if you want to achieve better tree-shaking when having lazy loaded modules you should create separate modules using secondary entry-points, in the above repo by @statop, he is making use of this https://github.com/statop/lazy-load-bug.

More information about secondary entry points can be found here: https://github.com/ng-packagr/ng-packagr/blob/master/docs/secondary-entrypoints.md

Let's keep tracking this in #13635, thanks.

@RobM-ADP
Copy link
Author

@alan-agius4 I don't think anyone would agree that the right solution to this is to force the developer to make every lazily-loadable item be its own secondary entry point. Every example repo showing that solution is extremely simplistic and not representative of the impact that that structure would have in a real application. In a large enterprise application where you have nested routes, that structure would not be acceptable.

While there is some similarity between this and the issue you reference, I don't think they are the same from the perspective of a developer who has run into this issue; even if the root cause is related. And certainly, seeing this issue closed and following the link to the one you reference will not clarify the issue nor provide any hint at how to solve it.

@statop
Copy link

statop commented Jun 20, 2019

@alan-agius4 I agree with @RobM-ADP .

In addition, it seems that tutorials and advice all over the internet recommends putting stuff in libraries, when in fact doing that will affect your applications performance unless you are very careful.

Nowhere have I found any advice about angular cli libraries that cautions people about what libraries mean from a lazy loading perspective.

I think it is very important to either stop making that recommendation or find a way to fix this issue.

Our team has had to basically reverse course on making libraries because of this issue.

Not using libraries and having as simple rule for our devs to never use any barrel files is much easier for junior devs, and management, to understand.

@alan-agius4
Copy link
Collaborator

@RobM-ADP, I appreciate your feedback but I don’t really agree with your comment that every example of using secondary entry points is basic.

Most of the Angular framework packages have secondary entry points and Angular Material itself has a large number of these. They have a secondary entrypoint for each and every component and the main reason for that was achieve better tree shaking and faster applications build times. I would strongly recommend that at least each feature is a secondary entrypoint.

The problem complicated, In a nutshell the tools that we use webpack and terser works best with different kind of modules. Webpack prefers small files, while terser larger files. Unfortunately these is no silver bullet for this.

At the end it is also up to you as a developer to see which library architecture/structure works best for your use-case and the consumers of your library.

@RobM-ADP
Copy link
Author

RobM-ADP commented Jun 20, 2019

@alan-agius4 What I meant was that the examples showing lazy loading from libraries are typically very simplistic. We have several internal component libraries for which secondary entry points work great, but I am talking about a very different use case for which there appears to be no solution. For example, we may have a feature module that not every user has access to so we would want to lazily load it. Inside of it, we could have multiple child routes that are large and we want to lazily load them too.

secondary-entry-point-for-feature
    main-feature-route <-- This can be lazily loaded based on permissions
         child-route-1 <-- We want to laziliy load this and anything it pulls in
         child-route-2 <-- We want to laziliy load this and anything it pulls in
         child-route-3 <-- We want to laziliy load this and anything it pulls in

Right now, you can't effectively lazily load the children because all of the code will be pulled in to the bundle for the feature instead of the children.

Given the number of "defects" logged about being able to do this, I believe this to be a pretty important thing. You can certainly argue that it isn't a defect in that it was not designed to do this, but I think most devs would agree that this is a requirement that should be possible.

@alan-agius4
Copy link
Collaborator

@RobM-ADP, I am slightly confused as in your reproduction you are not using secondary entrypoints. Secondary entry-points are not defined at application level, but rather at a library level. Here's a definition of what's an entry-point https://angular.io/guide/glossary#entry-point

In your example you have a library foo which is used by aaa and bbb with FESM the contents of the library will be a single JS module for Webpack. When Webpack sees that this module is shared among various lazy chunks it will bailout optimisations (See: https://webpack.js.org/plugins/module-concatenation-plugin/#optimization-bailouts) and will extract this into a common chunk.

While with ESM modules, this issue will be solved, at this point as mentioned in the linked previous issue, ESM actually end up causes more unneeded things to be retained, because of the differences there is between tools like Webpack and Terser.

The optimal solution for better code splitting when using libraries and lazy loading, such libraries need to provide additional entry-points, Example:

mylib/feature-1
mylib/feature-2

If you take a look at the APF specs https://docs.google.com/document/d/1CZC2rcpxffTDfRDs6p1cfbmKNLA6x5O-NtkJglDaBVs/preview#heading=h.3kkc9d322g9f there is the below note.

As of webpack v4 the flattening of ES modules optimization should not be necessary for webpack users, and in fact theoretically we should be able to get better code-splitting without flattening of modules in webpack, but in practice we still see size regressions when using unflattened modules as input for webpack v4. This is why "module" and "es2015" package.json entries still point to fesm files. We are investigating this issue and expect that we'll switch the "module" and "es2015" package.json entry-points to unflattened files when the size regression issue is resolved.

@RobM-ADP
Copy link
Author

@alan-agius4 The sample repo was intended to show that two routes from the same library cannot effectively be lazy loaded as expected. Having a secondary entry point just moves the problem down one level. So in my example if AAA and BBB were secondary entry points, they could, as you say, be chunked correctly. However, and this is why I said that all the examples are too simplistic, if you have child routes that you want to also lazy load, you would essentially have the exact same issue because both child routes would be under the same secondary entry point which result in the same situation I have in my repo.

mylib/feature-1
mylib/feature-1/child-route-1
mylib/feature-1/child-route-2
mylib/feature-2

In my opinion the feature entry points should be used to organize code not for instructing webpack on how to chunk it as you are now taking the responsibility that should be webpack's and putting it on the developer.

For example of the issue even with secondary entry points, look at the repo from @statop (https://github.com/statop/lazy-load-bug)

@statop
Copy link

statop commented Jun 21, 2019

@alan-agius4 - Our example repos are simply to make our point.

Yes, you can put everything in secondary entry points.

But what we have found is that that quickly escalates into almost every single file being an entry point.

And you are basically taking the responsibility for code splitting away from the build tools and placing it in the hands of the dev.

There is no longer anything automatic about code splitting.

@statop
Copy link

statop commented Jun 21, 2019

Also, again, please take a closer look at my repo - https://github.com/statop/lazy-load-bug

One of the things it demonstrates is how smart lazy loading can be. And how using a library removes that smarts.

And if you think about it, how difficult it would be to structure the library so that the code splitting from the library code looks like the splitting from the non-library code. (hint - you would need an entry point for each file in the common library)

@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 9, 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

No branches or pull requests

3 participants