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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increased main.js bundle size but less lazy chunks, another case with Angular v9-rc.11 in a monorepo structure #16799

Closed
1 of 15 tasks
dogmatico opened this issue Jan 30, 2020 · 23 comments
Labels
area: devkit/build-angular freq1: low Only reported by a handful of users who observe it rarely severity5: regression type: bug/fix
Milestone

Comments

@dogmatico
Copy link

dogmatico commented Jan 30, 2020

馃悶 Bug report

Command (mark with an x)

  • new
  • serve
  • build
  • test
  • e2e
  • generate
  • add
  • update
  • lint
  • xi18n
  • run
  • config
  • help
  • version
  • doc

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:

  • Core framework,
  • Auth back-end,
  • Angular Material sidenav and toolbar,
  • Shell components.

Once Ivy is enabled, it includes additional code. E.g., in one application, we get those two source-map-explorer results:

Captura de pantalla de 2020-01-30 09-41-47
Captura de pantalla de 2020-01-30 09-41-49

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:

Captura de pantalla de 2020-01-30 10-45-15
Captura de pantalla de 2020-01-30 10-51-58

馃敩 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

ng build <project-name> --prod --source-map --localize=false

And only changed the tsconfig.json to set angularCompilerOptions.enableIvy = false

馃實 Your Environment




Angular CLI: 9.0.0-rc.11
Node: 12.14.1
OS: linux x64

Angular: 9.0.0-rc.11
... animations, cli, common, compiler, compiler-cli, core
... elements, forms, language-service, localize
... platform-browser, platform-browser-dynamic, platform-server
... router, service-worker
Ivy Workspace: No

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.900.0-rc.11
@angular-devkit/build-angular      0.900.0-rc.11
@angular-devkit/build-ng-packagr   0.900.0-rc.11
@angular-devkit/build-optimizer    0.900.0-rc.11
@angular-devkit/build-webpack      0.900.0-rc.11
@angular-devkit/core               9.0.0-rc.11
@angular-devkit/schematics         9.0.0-rc.11
@angular/cdk                       9.0.0-rc.8
@angular/material                  9.0.0-rc.8
@angular/pwa                       0.803.23
@ngtools/webpack                   9.0.0-rc.11
@schematics/angular                8.3.23
@schematics/update                 0.900.0-rc.11
rxjs                               6.5.4
typescript                         3.7.5
webpack                            4.41.2

@IgorMinar
Copy link
Contributor

@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?

@IgorMinar IgorMinar added this to the v9-candidates milestone Jan 30, 2020
@ngbot ngbot bot removed this from the v9-candidates milestone Jan 30, 2020
@IgorMinar IgorMinar added this to the v9-candidates milestone Jan 30, 2020
@ngbot ngbot bot removed this from the v9-candidates milestone Jan 30, 2020
@filipesilva filipesilva added area: devkit/build-angular freq1: low Only reported by a handful of users who observe it rarely severity5: regression labels Jan 30, 2020
@ngbot ngbot bot modified the milestone: needsTriage Jan 30, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jan 30, 2020
@filipesilva filipesilva modified the milestones: Backlog, v9-candidates Jan 30, 2020
@filipesilva
Copy link
Contributor

filipesilva commented Jan 30, 2020

We can provide access in Bitbucket or Github private repositories if necessary.

@dogmatico I'll take you up on that. Will look into it as soon as you make it available to us.

@dogmatico
Copy link
Author

Great, thank you. I added you as a collaborator of https://github.com/dogmatico/bmat-front-end-monorepo-angular.

@filipesilva
Copy link
Contributor

Awesome, taking a look now.

@filipesilva
Copy link
Contributor

filipesilva commented Jan 31, 2020

Can reproduce. Mostly looks like angualr material/cdk/animations were brought into main. Also luxon.

@filipesilva
Copy link
Contributor

@dogmatico when you say:

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:

What lib is this? I'm looking further into this bit.

@dogmatico
Copy link
Author

@filipesilva the library is inside the folder libs/reporting-tool/data-access-reporting-tool-api and mapped in Typescript as @bmat/reporting-tool/data-access-reporting-tool-api. Those are small services that facade HttpClient calls and part of the reporting-tool project.

I have attached zooms of the lib section of source-map-explorer. Without Ivy:

no_ivy_01
no_ivy_02
no_ivy_03

With Ivy,
ivy_01
ivy_02

Thanks,
Cristian

@filipesilva
Copy link
Contributor

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 angular.json to make it easier to run a single command and compare builds. I changed the prod config for reporting-tool and media-ads to always output sourcemaps, never add hashes, not use localize, and always name chunks. That makes the filenames better. I also added another config called ivy with a different outputPath and tsConfig, this way I don't need to manually change the tsconfig to get different builds.

Now I can run ng build reporting-tool --configuration production,ivy && ng build reporting-tool --prod and the same for media-ads to get a VE/Ivy comparison. I did that and saw the same sourcemaps you pasted here. They looked the same: main bundle now included angular material/cdk/animations, and also luxon.

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 libs/reporting-tool/data-access-reporting-tool-api services and thought it couldn't be the whole picture, because those don't seem to (directly) include material, but material is a big part of the size change. They did use luxon though.

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 reporting-tool was the smallest so I picked it.

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 import(). So we comment that off and it's completely removed from the compilation.

I cut off all lazy routes for reporting-tool except for @bmat/reporting-tool/feature-reports. Ivy still showed a larger main and larger total size. Looks representative of whatever is happening.

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.

@filipesilva
Copy link
Contributor

Decided to look at the sourcemaps instead. Maybe it would give me a clue about where I should focus on. I saw I had made an error: the difference wasn't actually extra angular material/cdk/animations/luxon. I saw those boxes in source-map-explorer and didn't pay much more attention before, but now I see that was a mistake.

This is the node_modules sourcemap for the Ivy build without any lazy routes:
image

And here's the same for VE:
image

Breaking the differences down in a table

package/size (kb) Ivy VE
angular/core 124 99
angular/animations 65 64
angular/router 62 61
angular/cdk 58 56
angular/material 49 28
angular/forms 44 -
angular/common 42 25
angular/platform-browser 17 18
angular/elements 5 5
angular/service-worker 5 5
luxon 68 -
auth0 44 44
rxjs 43 42

angular/elements and service-worker can't be seen in the screenshots but clicking on only the angular folder I can see them in both source maps.

So there's a more of angular/material, and of angular/core as well, but what's really interesting is that angular/forms isn't even on the VE source-map. And then there's luxon again.

So those two give me something to trackdown. Why are they in the final Ivy bundle? I can get detailed information about the modules in a build and optimization by doing ng build reporting-tool --configuration production,ivy --output-path dist/sandbox-ivy --verbose &> log-ivy.txt. I need to pipe it to a file because it's a lot of output. I put it in another folder because I'm expecting to do more experiments and don't want to override what I already have. I also do the same for the VE build to compare those logs.

With this output at hand I search for luxon on the Ivy log and find a couple of interesting things. It's a CJS module and thus subject to some deoptimizations as far as bundling goes. @bmat/shared/luxon-tz-handler/feature-luxon is in the bundle and seems to be a angular module for luxon. @bmat/shared/luxon-tz-handler/utils-tz-mappings contains luxon related utilities. The terser output also mentions dropping a bunch of luxon related unused variables and function calls, both from the node_modules library and in the local luxon related code.

The VE code, meanwhile, has no mention of luxon. Now I'm searching for @angular/forms and there's plenty of mentions in the ivy log, but none in the VE log. This is a curious situation. I expected these libraries to be present in the both the logs, but in the VE log there should be something saying they had been removed by some optimization.

So what pulls both angular/forms and luxon in Ivy? The verbose logs should also answer that question. Digging around I find this block:

 [nIj0] D:/sandbox/bmat-front-end-monorepo-angular/node_modules/@angular/forms/__ivy_ngcc__/fesm2015/forms.js + 1 modules 274 KiB {1} [built]
     [only some exports used: CheckboxRequiredValidator, DefaultValueAccessor, FormBuilder, FormControl, FormControlName, FormGroup, FormGroupDirective, NG_VALIDATORS, NG_VALUE_ACCESSOR, NgControl, NgControlStatus, NgControlStatusGroup, NgForm, ReactiveFormsModule, Validators, 傻angular_packages_forms_forms_y]
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/@angular/common/__ivy_ngcc__/fesm2015/common.js because of ./src/main.ts
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js because of ./src/main.ts
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/rxjs/_esm2015/internal/Observable.js because of ./src/main.ts
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/rxjs/_esm2015/internal/observable/from.js because of ./src/main.ts
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/rxjs/_esm2015/internal/operators/map.js because of ./src/main.ts
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/rxjs/_esm2015/internal/util/isArray.js because of ./src/main.ts
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/rxjs/_esm2015/internal/util/isObject.js because of ./src/main.ts
     harmony side effect evaluation @angular/forms [eNd9] D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/lib/production-category-models.ts 1:0-44
     harmony import specifier @angular/forms [eNd9] D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/lib/production-category-models.ts 5:28-38
... (more stuff, but not really important and mostly repetition)

This says something along the lines of "forms internally is called nIj0, suffered the following bailouts, and was included because of these modules below". There are many mentions of the [nIj0] internal name in the log, but only the one that's most "toplevel" (is listed less indented than others) matters. These internal names also vary wildly between builds, so if you're looking at your own logs the name will be different.

So forms was included because of libs/reporting-tool/reporting-tool-interfaces/src/lib/production-category-models.ts. This file is again not present in the VE log.

In turn this module is listed thus:

 [eNd9] D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/lib/production-category-models.ts 3.8 KiB {1} [built]
     [no exports used]
     harmony side effect evaluation ./lib/production-category-models [1d+Z] D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/index.ts 1:0-49
     harmony export imported specifier ./lib/production-category-models [1d+Z] D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/index.ts 1:0-49

It was included because of libs/reporting-tool/reporting-tool-interfaces/src/index.ts. This too is not in the VE log. Following it again:

 [1d+Z] D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/index.ts 269 bytes {1} [built]
     [only some exports used: MediaType, ReportStatus]
     ModuleConcatenation bailout: Module exports are unknown
     harmony side effect evaluation @bmat/reporting-tool/reporting-tool-interfaces [zUnb] ./src/main.ts + 198 modules 1:0-90
     harmony import specifier @bmat/reporting-tool/reporting-tool-interfaces [zUnb] ./src/main.ts + 198 modules 399:29-38
     harmony import specifier @bmat/reporting-tool/reporting-tool-interfaces [zUnb] ./src/main.ts + 198 modules 400:32-44

Here the trail goes a bit cold. This says @bmat/reporting-tool/reporting-tool-interfaces comes from the main.ts internal module, but that one contains 198 other modules. I don't quite know which one of those 199 has this import.

But I can just go see what the main module has in it to see if there's something interesting, and find this

 [zUnb] ./src/main.ts + 198 modules 2.22 MiB {1} [built]
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/libs/auth/auth-interfaces/src/index.ts (<- Module exports are unknown)
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/index.ts (<- Module exports are unknown)
     ModuleConcatenation bailout: Cannot concat with D:/sandbox/bmat-front-end-monorepo-angular/node_modules/@angular/common/__ivy_ngcc__/fesm2015/common.js because of D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/reporting-tool-interfaces/src/index.ts
...

This means "I couldn't pull libs/auth/auth-interfaces/src/index.ts into this module because the exports are unknown". Which is really odd since I opened that TS file and saw it had a bunch of export statements.

Trying to do the same for luxon shows me it's imported by the main module as well.

 [9hG1] D:/sandbox/bmat-front-end-monorepo-angular/node_modules/luxon/build/cjs-browser/luxon.js 236 KiB {1} [built]
     [only some exports used: DateTime]
     ModuleConcatenation bailout: Module is not an ECMAScript module
     harmony side effect evaluation luxon [zUnb] ./src/main.ts + 198 modules 1:0-33
     harmony side effect evaluation luxon [zUnb] ./src/main.ts + 198 modules 1:0-33
     harmony side effect evaluation luxon [zUnb] ./src/main.ts + 198 modules 1:0-33

But this one is easier to figure out because the main module contains some references to the luxon wrappers like this:

     |     harmony side effect evaluation @bmat/search-filters/search-filters-interfaces  D:/sandbox/bmat-front-end-monorepo-angular/libs/shared/luxon-tz-handler/utils-tz-mappings/src/lib/check-valid-period.ts 1:0-80

@filipesilva
Copy link
Contributor

Tried to find imports to @bmat/reporting-tool/reporting-tool-interfaces in the source and was pleasantly surprised to find a single one, in apps/reporting-tool/src/app/feature-lazy-channel-list/user-channel-tree/user-channel-tree.component.ts.

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:

WARNING in D:\sandbox\bmat-front-end-monorepo-angular\apps\reporting-tool\src\app\feature-lazy-channel-list\user-channel-tree\user-channel-tree.component.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.
...
WARNING in D:\sandbox\bmat-front-end-monorepo-angular\apps\reporting-tool\src\app\feature-lazy-channel-list\user-channel-tree\user-channel-tree.component.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

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 @bmat/reporting-tool/ and found @bmat/reporting-tool/data-access-reporting-tool-api in the Ivy logs:

     | D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/data-access-reporting-tool-api/src/index.ts 1.45 KiB [built]
     |     [only some exports used: ReportingToolAPIModule]
     |     harmony side effect evaluation @bmat/reporting-tool/data-access-reporting-tool-api  ./src/app/app.module.ts 9:0-93
     |     harmony import specifier @bmat/reporting-tool/data-access-reporting-tool-api  ./src/app/app.module.ts 53:16-38

This one is imported in ./src/app/app.module.ts. The Ivy logs contain a lot (308) of references to modules in reporting-tool/data-access-reporting-tool-api/src, but the VE logs only contain 9 references:

     | D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/data-access-reporting-tool-api/src/lib/tokens/reporting-tool-api-url.token.ts 134 bytes [built]
     |     [only some exports used: REPORTING_TOOL_API_URL]
     |     harmony side effect evaluation ./tokens/reporting-tool-api-url.token  D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module.ts 1:0-79
     |     harmony import specifier ./tokens/reporting-tool-api-url.token  D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module.ts 8:27-49
     |     harmony side effect evaluation ../../../../libs/reporting-tool/data-access-reporting-tool-api/src/lib/tokens/reporting-tool-api-url.token  ./src/app/app.module.ngfactory.js 64:0-130
     |     harmony import specifier ../../../../libs/reporting-tool/data-access-reporting-tool-api/src/lib/tokens/reporting-tool-api-url.token  ./src/app/app.module.ngfactory.js 66:14984-15010
     | D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module.ts 404 bytes [built]
     |     [only some exports used: ReportingToolAPIModule]
     |     harmony side effect evaluation ../../../../libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module  ./src/app/app.module.ngfactory.js 62:0-147
     |     harmony import specifier ../../../../libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module  ./src/app/app.module.ngfactory.js 66:10865-10891
     |     harmony import specifier ../../../../libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module  ./src/app/app.module.ngfactory.js 66:10893-10919

So this goes back to your comment about the libs: #16799 (comment).

src/app/app.module.ts contains this import:

import { ReportingToolAPIModule } from '@bmat/reporting-tool/data-access-reporting-tool-api';

In VE this imports only libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module.ts and the libs/reporting-tool/data-access-reporting-tool-api/src/lib/tokens/reporting-tool-api-url.token.ts referenced there.

In Ivy this import results in a lot more things from @bmat/reporting-tool/data-access-reporting-tool-api. Sourcemaps for the Ivy build show there are bytes there from the following files:

  • libs/reporting-tool/data-access-reporting-tool-api/src/lib/utils/get-tracks-datatable-params-factory/get-tracks-datatable-params-factory.ts
  • libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module.ts
  • libs/reporting-tool/data-access-reporting-tool-api/src/lib/claims/claims.service.ts
  • libs/reporting-tool/data-access-reporting-tool-api/src/lib/tokens/reporting-too-api-url.token.ts
  • libs/reporting-tool/data-access-reporting-tool-api/src/lib/channel-service/channel.contants.ts

The claims.service.ts file contains @Injectable({providedIn: 'root}), but the other files don't. There's other services in this package too, but they weren't included. In your comment you saw a lot of these being included in a normal build, but my build is different because it doesn't have the lazy modules.

The ivy log says it's included because it's being imported by the index.ts on that library:

     |     harmony side effect evaluation ./lib/claims/claims.service  D:/sandbox/bmat-front-end-monorepo-angular/libs/reporting-tool/data-access-reporting-tool-api/src/index.ts 8:0-44

But again nowhere to be found in the VE log. So it seems that the problematic thing seems to be that ./src/app/app.module.ts importing ReportingToolAPIModule from libs/reporting-tool/data-access-reporting-tool-api/src/index.ts in Ivy causes it to retain some other unrelated exports that are also listed in that index file (like libs/reporting-tool/data-access-reporting-tool-api/src/lib/claims/claims.service.ts), but this doesn't happen in VE.

@filipesilva
Copy link
Contributor

I also don't understand why I see so many of these warnings in the ivy compilation:

WARNING in D:\sandbox\bmat-front-end-monorepo-angular\apps\reporting-tool\src\app\feature-lazy-channel-list\user-channel-tree\user-channel-tree.component.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

WARNING in D:\sandbox\bmat-front-end-monorepo-angular\apps\reporting-tool\src\app\feature-lazy-channel-list\lazy-channel-list.module.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

WARNING in D:\sandbox\bmat-front-end-monorepo-angular\apps\reporting-tool\src\app\feature-lazy-side-menu\side-menu\side-menu.component.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

WARNING in D:\sandbox\bmat-front-end-monorepo-angular\apps\reporting-tool\src\app\feature-lazy-side-menu\lazy-side-menu.module.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

WARNING in D:\sandbox\bmat-front-end-monorepo-angular\libs\auth\feature-angular-auth-user-apps\src\lib\feature-angular-auth-user-apps.component.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

WARNING in D:\sandbox\bmat-front-end-monorepo-angular\libs\auth\feature-angular-auth-user-apps\src\lib\component\app-list-icon\app-list-icon.component.ts is part of the TypeScript compilation but it's unused.
Add only entry points to the 'files' or 'include' properties in your tsconfig.

This shouldn't happen because the tsconfig for this build contains explicit entry points:

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "outDir": "../../dist/out-tsc",
    "types": []
  },
  "files": ["src/main.ts", "src/polyfills.ts"],
  "exclude": ["src/test-setup.ts", "**/*.spec.ts"]
}

Ah but this one extends another one that actually has a broad include:

{
  "extends": "../../tsconfig.json",
  "compilerOptions": {
    "types": ["node", "jest"],
    "jsx": "react",
    "jsxFactory": "h"
  },
  "include": ["**/*.ts", "**/*.tsx"]
}

Removing that include removes the warnings, but doesn't affect file size.

Double checked my assertion that @bmat/reporting-tool/reporting-tool-interfaces was only found in a single file and found that to be false, I must have used a bad search before. It's found in loads of files, including a lot in libs/reporting-tool/data-access-reporting-tool-api. That is why it gets included.

@filipesilva
Copy link
Contributor

Tried completely removing all imports from libs/reporting-tool/data-access-reporting-tool-api/src/index.ts to see if the presence of more imports there made a difference. The main bundle gets 1kb smaller and sourcemaps show the only things from libs/reporting-tool/data-access-reporting-tool-api/src/ are the really imported ones (libs/reporting-tool/data-access-reporting-tool-api/src/lib/reporting-tool-data-access-reporting-tool-api.module.ts).

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 libs/reporting-tool/reporting-tool-interfaces/src/lib/production-category-models.ts, which was included because of libs/reporting-tool/reporting-tool-interfaces/src/index.ts. I still don't know what's importing that last one. But I'm pretty sure that if I comment it's contents out, the TS compiler will error out and tell me.

Sure enough, it points me towards libs/search-filters/feature-custom-filters/src/lib/filters-dialogs-list-item/filters-dialogs-list-item.component.ts, which imports two enums that are in libs/reporting-tool/reporting-tool-interfaces/src/lib/reporting-tool-reporting-tool-interfaces.ts, and re-exported through said index.ts.

I comment out everything in libs/reporting-tool/reporting-tool-interfaces/src/index.ts except the export that's actually used, and again main drops in size, this time by ~60kb. It's still not on par with the VE main file, but it's getting closer.

So there's definitely something going on with re-exports. In some causes they get retained in Ivy, but not in VE.

filipesilva added a commit to filipesilva/barrel-side-effects that referenced this issue Jan 31, 2020
@filipesilva
Copy link
Contributor

Ok, I was able to destil this down to a minimal repro:


BarrelSideEffects

This repository reproduces the problem in #16799.

git clone https://github.com/filipesilva/barrel-side-effects
npm install
npm run repro

What's happening?

This app contains the following files:

  • src/app/another/index.ts
export * from './another.module';
export * from './side-effects';
  • src/app/another/another.module.ts
import { NgModule } from '@angular/core';
import { CommonModule } from '@angular/common';

@NgModule({
  declarations: [],
  imports: [
    CommonModule
  ]
})
export class AnotherModule { }
  • src/app/another/side-effects.ts
import { DateTime } from 'luxon';

export const local = () => DateTime.local();
export const map = new Map([['key', 'value']]);
  • src/app/app.module.ts
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:

  • src/app/app.module.ts imports AnotherModule from src/app/another/index.ts
  • src/app/another/index.ts also exports src/app/another/side-effects.ts, a file that isn't used but loads a library (Luxon) and has a toplevel side effect (a Map instantiated with ['key', 'value']).

npm run repro will build the application first with Ivy, and then with View Engine (VE, the compiler before Ivy).

The Ivy build will contain the contents of src/app/another/side-effects.ts, but the VE build will not.
You can tell because the Ivy build is much larger. You can also open dist/barrel-side-effects/main.js and find Luxon referenced there, along with map (as new Map([["key","value"]])).

@dogmatico
Copy link
Author

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.

@filipesilva
Copy link
Contributor

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 "sideEffects": false to their package.json. You can also just drop a new package.json with {"sideEffects": false} on folders that you know are free from side effects, files in that folder and in any subfolder will be treated as such.

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.

@dogmatico
Copy link
Author

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 webpackConfig.resolve.mainFields of Angular CLI, to use Luxon ESM instead of CJS modules resulted in solving the issue for the reproduction repository. If the change in the behaviour of Ivy is intended and the correct one, we can see this issue as a bundler + app architecture problem and not an Angular one.

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.

@filipesilva
Copy link
Contributor

Just to be clear, when I said

There's something else you can do though. If your shared libraries are meant to be free from side effects, add "sideEffects": false to their package.json. You can also just drop a new package.json with {"sideEffects": false} on folders that you know are free from side effects, files in that folder and in any subfolder will be treated as such.

The shared libraries I meant where the ones in your libs/ folder.

It's not safe to patch the package.json of Luxon, because we don't know if it actually is free from side effects or not. It would be safe to use its ESM entry point (module in package.json), but we prioritize browser instead because often there are libraries that primarily have Node entry points and then put a browser entry point in browser.

@dogmatico
Copy link
Author

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,
Cristian

@DmitryEfimenko
Copy link

so does this mean that angular/forms should be marked as sideEffects = false?

@dogmatico
Copy link
Author

so does this mean that angular/forms should be marked as sideEffects = false?

In my particular case, @angular/forms was dragged to the main bundle by a custom library that provided a date/month picker and imported it. Once I checked that our own libraries were free from side effects, and marked it with module level package.json, it returned to similar a size and identical contents.

We didn't patch any third party library, nor @angular packages to achieve it, and I wouldn't take that path unless strictly necessary.

@clydin
Copy link
Member

clydin commented Feb 5, 2020

The @angular/forms package is currently marked as side effect free. All the client-side @angular packages are marked with the exception of the @angular/compiler package which is not used in a production (AOT compiled) application.

@filipesilva
Copy link
Contributor

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

@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 Mar 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: devkit/build-angular freq1: low Only reported by a handful of users who observe it rarely severity5: regression type: bug/fix
Projects
None yet
Development

No branches or pull requests

5 participants