-
Notifications
You must be signed in to change notification settings - Fork 12k
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
Comments
To add to this, here is a second demo app - It has specific examples |
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. |
@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. |
@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. |
@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 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. |
@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.
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. |
@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 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 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.
|
@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.
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) |
@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. |
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) |
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. |
🐞 Bug report
Command (mark with an
x
)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
common.js bundle
Notice that components from both Module AAA and Module BBB are in here.
aaa-wrapper-module.js bundle
Notice that the only thing in here is the wrapper itself. So lazy-loading has really added no benefit.
The text was updated successfully, but these errors were encountered: