-
Notifications
You must be signed in to change notification settings - Fork 12k
Increased main.js bundle size but less lazy chunks, another case with Angular v9-rc.11 in a monorepo structure #16799
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
@angular/material is the most curious piece - it looks like with Ivy all of it is being retained in these apps. That's highly unusual. Is there any chance we could follow up offline and get a debug version of the bundle for us to analyze? Is there anything else atypical about your setup? |
@dogmatico I'll take you up on that. Will look into it as soon as you make it available to us. |
Great, thank you. I added you as a collaborator of https://github.com/dogmatico/bmat-front-end-monorepo-angular. |
Awesome, taking a look now. |
Can reproduce. Mostly looks like angualr material/cdk/animations were brought into main. Also luxon. |
@dogmatico when you say:
What lib is this? I'm looking further into this bit. |
@filipesilva the library is inside the folder I have attached zooms of the lib section of source-map-explorer. Without Ivy: Thanks, |
Heya, just want to give you updates as I go. It helps me think and keeps me honest. So the first thing I wanted to do was compare the bundle sizes on disk and sourcemaps side-by-side. I know you gave me samples already here in the issue, but I like to get my own and iterate. To do this I changed the Now I can run I found luxon to be kinda random to also change. That's not a angular lib and there shouldn't be anything special wrt the library and AOT compilation/optimizations. I explored the codebase a bit to see if anything stood out as a promising investigation approach. Didn't find anything. I looked into the I thought there was no way I could cover this much source code just be reading stuff. When I don't really have any strong intuition to follow on these problems I just start cutting down the problem, and hopefully I can get to something small but still representative that's easier to analyse. Since both apps exhibited roughly the same problem I decided to focus on one, and I compared the built file sizes and names for a bit and it looked like each had a main bundle, 6 named chunks, and then the number of common chunks differed. VE had 5 common chunks and Ivy had 4. But Ivy was larger both on main and on total size, which I found very surprising. I started cutting down the code further. A good way to reduce build sizes for investigation is to remove lazy routes. They are already independent entry points that should only be connected by the dynamic I cut off all lazy routes for At this point I'm thinking it must be something like #16146 (comment). But just for kicks I try removing the last lazy route to see what the sizes are without multiple chunks. On all other projects I found Ivy performed better on this case. But I was very surprised to see that in this one Ivy made a larger main bundle without lazy routes. Sourcemaps show the same story as before: extra angular material/cdk/animations, and also luxon. I don't know what's up with luxon but I'm pretty sure it's important, because it's surprising (to me). At this point I think I need to dig in in either the remaining source code, and then on the generated bundles. |
Tried to find imports to Which is really odd because that's inside one of the lazy modules I removed. The VE log doesn't contain any references to it, and the Ivy log has these two:
This is a warning we recently added in #15061. Odd that it comes up twice. Even more odd that the component isn't actually referenced in the webpack list of modules. I also tried to see if the logs contain any other reference starting with
This one is imported in
So this goes back to your comment about the libs: #16799 (comment).
In VE this imports only In Ivy this import results in a lot more things from
The The ivy log says it's included because it's being imported by the index.ts on that library:
But again nowhere to be found in the VE log. So it seems that the problematic thing seems to be that |
I also don't understand why I see so many of these warnings in the ivy compilation:
This shouldn't happen because the tsconfig for this build contains explicit entry points:
Ah but this one extends another one that actually has a broad include:
Removing that include removes the warnings, but doesn't affect file size. Double checked my assertion that |
Tried completely removing all imports from Although this was just a 1kb difference, there's something odd here. Importing a re-export from a index file shouldn't cause some unrelated files to stay in the compilation. Knowing this, I went back to Sure enough, it points me towards I comment out everything in So there's definitely something going on with re-exports. In some causes they get retained in Ivy, but not in VE. |
Ok, I was able to destil this down to a minimal repro: BarrelSideEffectsThis repository reproduces the problem in #16799.
What's happening?This app contains the following files:
export * from './another.module';
export * from './side-effects';
import { NgModule } from '@angular/core';
import { CommonModule } from '@angular/common';
@NgModule({
declarations: [],
imports: [
CommonModule
]
})
export class AnotherModule { }
import { DateTime } from 'luxon';
export const local = () => DateTime.local();
export const map = new Map([['key', 'value']]);
import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { AppComponent } from './app.component';
import { AnotherModule } from './another/index';
@NgModule({
declarations: [
AppComponent
],
imports: [
BrowserModule,
AnotherModule,
],
providers: [],
bootstrap: [AppComponent]
})
export class AppModule { } These files are setup so that:
The Ivy build will contain the contents of |
Great, thanks for your deep analysis and minimal repo. I'll take a look; I'm not used to ngcc and build tool internals but anyway will try to see if I can help with this issue. |
I'm not really sure what can be done about this, as it was a design choice. VE used deep imports because of the whole separate factory file business, and we also ended up removing unused imports after the decorators were stripped out of the original file, so index file were bypasses. But Ivy just uses the same file instead, so whatever imports are there, stay there. It's not strictly incorrect mind you. VE was doing something incorrect, insofar as any side effects or imports you had referenced in files re-exported from index files could be dropped. But your app might rely on them, so that wasn't right. Ivy does the right thing and doesn't mess with your import structure. But in cases like this, where there's a lot of shared code and the shared libraries are not free from side effects, extra code ends up being retained. You could hypothetically go around and use deep imports everywhere, but that's not great. There's something else you can do though. If your shared libraries are meant to be free from side effects, add The more I think about it, the more I that's the right thing to do in this case. My reasoning is this: VE erroneously caused some index files to be treated as though they had no side effects and left you no recourse, but Ivy doesn't do this and instead requires you to mark code free from side effects as such. |
For most of our applications is not a big issue to have larger main bundles; those are b2b web application with service workers, hence it's fine to once in while fetch more data. Also, I noticed that patching the package.json of Luxon, or the Webpack configuration Maybe we can add somewhere a mention to this change and why it's the correct behaviour compared to VE. It will save other users from opening issues like this one and may help them into finding ways to improve their bundle sizes. Again, thanks a lot for your deep review and comments. |
Just to be clear, when I said
The shared libraries I meant where the ones in your It's not safe to patch the |
Yes, all was understood. The patch was an experiment to set the ESM entry point instead of CJS one, not an actual code change nor sideEffects = false declaration, and see what happened. For my part, and considering that the Ivy behaviour is more reasonable, this issue can be closed. My only suggestion is, perhaps, explain this new treatment of the side effects in the https://next.angular.io/guide/ivy page. Thanks, |
so does this mean that angular/forms should be marked as |
In my particular case, We didn't patch any third party library, nor |
The |
We've added documentation about this class of problem in angular/angular#35178. It should be available https://next.angular.io/guide/ivy-compatibility#payload-size-debugging now, and shortly in https://angular.io/guide/ivy-compatibility#payload-size-debugging |
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?
Yes, with Angular v8, or Ivy disabled, the main bundle size issue did not happen
Description
Like in issue #16146, Ivy enabled builds result in bigger main bundles.
We use an app-shell plus lazy load of components- with Angular Elements like in aio, and routes. With View Engine (VE), both in Angular 8 and 9, the main bundle effectively only included what was required to render the shell:
Once Ivy is enabled, it includes additional code. E.g., in one application, we get those two source-map-explorer results:
In other app, the result is also bigger main bundles. In particular, it adds all the
@Injectable({providedIn: 'root})
services that are provided in a data-access internal library:🔬 Minimal Reproduction
The issue is in a proprietary mono-repo of front-ends. We can provide access in Bitbucket or Github private repositories if necessary. To generate the bundles, we used
And only changed the tsconfig.json to set
angularCompilerOptions.enableIvy = false
🌍 Your Environment
The text was updated successfully, but these errors were encountered: