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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: all RxJS operators pulled into prod bundle when using lettable imports from 'rxjs/operators' #9069

Closed
Splaktar opened this issue Jan 2, 2018 · 56 comments
Labels
P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix

Comments

@Splaktar
Copy link
Member

Splaktar commented Jan 2, 2018

Versions

Angular CLI: 1.6.3
Node: 8.9.0
OS: darwin x64
Angular: 5.1.2
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, platform-server, router
... service-worker

@angular/cdk: 5.0.2
@angular/cli: 1.6.3
@angular/flex-layout: 2.0.0-beta.12
@angular/material: 5.0.2
@angular-devkit/build-optimizer: 0.0.36
@angular-devkit/core: 0.0.22
@angular-devkit/schematics: 0.0.42
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.9.3
@schematics/angular: 0.1.11
@schematics/schematics: 0.0.11
typescript: 2.5.3
webpack: 3.10.0

Repro steps

https://github.com/Toxicable/issue-lettable-bundle-size

Observed behavior

From #8854 (comment), "There appears to be a bug in Webpack that is causing the CLI to import all of the RXJS operators if you use any lettable operator imports without doing each as a deep import individually.

Upgrading to lettable operators in CLI 1.6.3 caused RxJS to go from 60 KB to 122 KB for me. It sounds like a fix is 'coming soon', but I haven't been able to track down the issue to follow."

Another user mentioned a bundle size increase when moving to lettable operators in the CLI: #8720 (comment)

Desired behavior

From the 5.0 blog post:
"These new operators eliminate the side effects and the code splitting / tree shaking problems that existed with the previous ‘patch’ method of importing operators."
"RxJS now distributes a version using ECMAScript Modules. The new Angular CLI will pull in this version by default, saving considerably on bundle size."

I desire a considerable reduction in bundle size, not an increase.

Mention any other details that might be useful (optional)

I tracked this down across Slacks, GitHub, and Twitter as I was told that there was a known Webpack problem that would be fixed soon and then pulled into the CLI. As far as I can tell, any similar Webpack issue was solved in 3.9.0 and the current CLI already includes it as it is using 3.10.0 now.

I can provide source map explorer screenshots if needed, but the linked repo above should enable you to better reproduce and create your own source map explorer output.

RxJS Docs for lettable operators and builds: https://github.com/ReactiveX/rxjs/blob/master/doc/lettable-operators.md#build-and-treeshaking

Workaround

Breaking out all of your imports into individual (one import per line) deep imports seems to avoid this issue, but it is very painful and not desirable to have these extra lines of imports in projects of any significant size or scope.

@filipesilva filipesilva self-assigned this Jan 2, 2018
@filipesilva filipesilva added P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix labels Jan 2, 2018
@filipesilva
Copy link
Contributor

@IgorMinar
Copy link
Contributor

I'll look into this

@IgorMinar
Copy link
Contributor

IgorMinar commented Jan 2, 2018

The problem is that you have -vc turned on. Vendor chunking results terrible size regressions. This is because webpack 3 generates a ton of cross-chunk module-stitching code that prevents uglify from dead-code-eliminating the unused operators. Once you turn off vendor chunking everything will work correctly.

If things go as planned webpack 4 should significantly improve this situation, but until then definitely always always turn off vendor chunking if you care about payload size. This is why -vc is disabled in cli by default.

with vendor chunking:
screen shot 2018-01-02 at 2 43 38 pm

without vendor chunking:
screen shot 2018-01-02 at 2 43 42 pm

@Splaktar
Copy link
Member Author

Splaktar commented Jan 2, 2018

I'm not using -vc in my project and I'm still seeing this issue. Here's the command that I'm running:
ng build --target=production --env=staging

My env for staging is the following:

export const environment = {
  production: true,
  firebaseConfig: { ... },
  googleAnalyticsTrackingId: '...',
  mapsApiKey:  '...'
}

I'll post the source map explorer screenshots in a minute.

@Splaktar
Copy link
Member Author

Splaktar commented Jan 2, 2018

According to https://github.com/angular/angular-cli/wiki/build, --vendor-chunk (aliases: -vc) default value: true.

Perhaps the issue is that the default should now be false?

@Splaktar
Copy link
Member Author

Splaktar commented Jan 2, 2018

ng build --target=production -sm
Pre-lettable operators (60.63 KB for RxJS):
screen shot 2018-01-02 at 6 43 44 pm

ng build --target=production -sm
With lettable operators (122.39 KB for RxJS):
screen shot 2018-01-02 at 6 45 08 pm

ng build --target=production -sm --vc=false
With lettable operators (122.39 KB for RxJS):
screen shot 2018-01-02 at 6 49 10 pm

ng build --target=production -sm --vc false
With lettable operators (122.39 KB for RxJS):
screen shot 2018-01-02 at 6 51 28 pm

ng build --target=production -sm -vc false
With lettable operators (122.39 KB for RxJS):
screen shot 2018-01-02 at 6 59 47 pm

ng build --target=production -sm -vc=false
With lettable operators (122.39 KB for RxJS):
screen shot 2018-01-02 at 7 04 04 pm

ng build --target=production -sm -vc=false -cc=false
With lettable operators (122.39 KB for RxJS):
screen shot 2018-01-02 at 7 20 41 pm

@filipesilva filipesilva reopened this Jan 3, 2018
@filipesilva
Copy link
Contributor

@Splaktar is your project public maybe? That way I could take a look.

Are you using lazy bundles as well? Can you try removing them and checking if there's a difference in rxjs?

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

@filipesilva the source isn't public. The staging and production sites are.

There is currently one major lazy loaded module. The other routes just go to simple components that are fairly static.

const routes: Routes = [
  {path: '', pathMatch: 'full', component: LandingComponent, children: []},
  {path: 'agents', loadChildren: './agents/agents.module#AgentsModule'},
  {path: 'profile/:id', component: ProfileComponent, canActivate: [UsersService], children: []},
  {path: 'terms', component: TermsComponent, children: []},
  {path: 'privacy', component: PrivacyComponent, children: []},
  {path: '**', component: PageNotFoundComponent, children: []}
];

@NgModule({
  imports: [RouterModule.forRoot(routes, {useHash: false})],
  exports: [RouterModule],
  providers: []
})
export class AppRoutingModule {}

@filipesilva
Copy link
Contributor

If you turn that lazy route into a normal route, or remove it altogether, does the rxjs size in the main bundle change?

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

Removing my lazy loaded route and re-running with ng build --target=production -sm:
screen shot 2018-01-02 at 7 29 16 pm
RxJS size is smaller, but I still see tons of operators that I am not using (especially w/o the primary AgentsModule in the build).

Here's the full source map explorer:
screen shot 2018-01-02 at 7 33 14 pm

@devoto13
Copy link
Contributor

devoto13 commented Jan 3, 2018

Note that some operators come from libraries. So they are still needed in the bundle even if you don't use them directly.

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

In the AppModule, I use shareReplay, switchMap, filter, debounceTime.
In the AgentsModule (lazy loaded), I use those plus distinctUntilChanged, take, map.

@IgorMinar
Copy link
Contributor

@Splaktar can you create a minimal reproduction similar to how @Toxicable created the one earlier?

I see that you are using firebase in your app and you are using the cjs distro of that library. I'm wondering if that's why all the operators are retained. A minimal reproduction would confirm or reject that theory.

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

I'm using "firebase": "4.7.0", and including it via .angular-cli.json like this:

      "scripts": [
        "../node_modules/hammerjs/hammer.js",
        "../node_modules/firebase/firebase.js"
      ],

It looks like I should be doing this instead:

      "scripts": [
        "../node_modules/hammerjs/hammer.js",
        "../node_modules/firebase/firebase-app.js",
        "../node_modules/firebase/firebase-auth.js",
        "../node_modules/firebase/firebase-database.js"
      ],

But I'm not sure how much that will help. I looked at their web setup page and Firebase JS SDK GitHub and I don't see any way to pick different types of modules.

I'll dig into that a bit more, try out firebase 4.8.0, and then work on building a new repo for a public reproduction.

Update: Firebase 4.8.0 and deeper imports did not change anything.

@filipesilva
Copy link
Contributor

Hm... importing "../node_modules/firebase/firebase.js" in scripts shouldn't result in dependencies insofar as importing is concerned, because importing should not work at all in scripts.

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

OK, I've created a reproduction repo here. Time to get to sleep now...

Using ng build --target=production --env=prod -sm:
screen shot 2018-01-03 at 5 11 48 am

@filipesilva
Copy link
Contributor

@Splaktar awesome, I was just putting up a repro myself. This way it's much easier. Will take a look.

@filipesilva
Copy link
Contributor

filipesilva commented Jan 3, 2018

Running some baseline tests, measuring size of node_modules in main.*.bundle.js and where it comes from via ng build --target=production --env=prod -sm and source-map-explorer:

  • normal build: 1.09 MB node_modules (@angular/* 575 KB, rxjs 122 KB)
  • commenting out lazy route: 0.987 MB node_modules (@angular/* 542 KB, rxjs 56KB)
  • forcing lazy module to be non-lazy: 1.01 MB (@angular/* 543 KB, rxjs 56 KB) (lazy bundle used to be ~32kb)

I think having a lazy bundle is having the same effect on bundle size as the vendor chunk (described in #9069 (comment)). This makes sense because vendor chunk is not special, it's just another chunk, just like a lazy chunk is just another chunk.

This makes me think that the best hope to address this problem is Webpack 4. Support is being tracked in #8611.

Note: regarding #9069 (comment), when Build Optimizer is used then vendor chunk will default to false (https://github.com/angular/angular-cli/wiki/build#--build-optimizer-and---vendor-chunk)

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

@filipesilva thank you very much for looking into this!

Oh that's really interesting about Build Optimizer and Vendor Chunk options. Perhaps https://github.com/angular/angular-cli/wiki/build can be updated with that information?

I guess at this point, we should recommend that teams do not use lettable operators if they are using the Angular CLI with lazy loaded routes (this is every developer as we have been telling them for a year+ to lazy load every possible route). Do you see any alternative to this?

I need to advise multiple active clients on this atm in addition to my own projects.

@devoto13
Copy link
Contributor

devoto13 commented Jan 3, 2018

@Splaktar I remember facing this problem some time ago when importing from rxjs/operators, so I switched to using rxjs/operators/<name> imports everywhere and haven't had problem with RxJS bundle size ever since. It's not a big deal, since imports are added by IDE automatically and I never have to write them manually anyway.

Maybe one option for you is to advice using deep imports (at least as a temporary solution). I've updated your reproduction from #9069 (comment) to use this approach and RxJS bundle size is back to normal.

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

@devoto13 thank you very much for looking into that! It was a question that I was just asking myself.

What you have shown though is that using lettable operators with deep imports is not a solution for "saving considerably on bundle size". I can't advise my clients to add a considerable amount of boilerplate (yes it can be auto filled by IDEs in most cases) and change to a new syntax if there is no significant impact on bundle size.

I'm going to push up a branch w/o lettable operators for comparison.

@devoto13
Copy link
Contributor

devoto13 commented Jan 3, 2018

@Splaktar Thanks! Please do. Will be very interesting to see how much better it gets without lettable operators.

@Splaktar
Copy link
Member Author

Splaktar commented Jan 3, 2018

I don't expect it to be better w/o lettable operators other than that it matches most of the existing docs and apps that are out there today. The issue is that it's not clear how lettable operators are "considerably better" in Angular w/ the Angular CLI today.

Non-lettable operator branch (i.e. old fashioned): https://github.com/DevIntent/angular-example/tree/withoutLettableOperators

The only difference that I can see is the savings of the add operator when using lettable operators. This saves 612 B only.
screen shot 2018-01-03 at 2 20 08 pm

@blackholegalaxy
Copy link

any news on that? Rxjs documentation states bundle size can be fixed through Webpack configuration. Is that intended to be fixed through webpack 4 upgrade?

Ref: https://github.com/ReactiveX/rxjs/blob/master/doc/pipeable-operators.md#build-and-treeshaking

@Lakston
Copy link
Contributor

Lakston commented Feb 21, 2018

I just found this issue (should have looked for it earlier...) after spending my morning trying to figure out why I had all of rxjs being bundled on our app (@angular/cli": "1.6.7")

Also I haven't checked thouroughly yet but it seems like we have the same tree shaking problem with date-fns/esm pulling everything into the prod bundle.

I'll second @blackholegalaxy with his/her question, or if anyone can point us to an issue where we can track / fix this ?

Thanks !

@filipesilva
Copy link
Contributor

@blackholegalaxy @Lakston this the the correct issue to track resolution.

The only way to truly achieve tree shaking of RxJs (in the current context) is to use Webpack 4 and have the package marked as being side effect free.

Without these two preconditions, it is only possible to treeshake RxJs operators in the main bundle, and only if the vendor bundle is disabled (as it is by default on prod builds).

@blackholegalaxy
Copy link

Sorry about my lack of knowledge of the "under the hood angular CLI logic", but could the solution on the link I provided above, by Rxjs team, work in the CLI context? It seems CLI relies on webpack 3 right?

@filipesilva
Copy link
Contributor

@blackholegalaxy we already implement that in the CLI actually.

@blackholegalaxy
Copy link

Ok nice to know. We will then wait for webpack 4 improvements. Thanks for precision @filipesilva :)

@benlesh
Copy link
Contributor

benlesh commented Mar 2, 2018

If you have Observable and an operator or two, that's a working app as far as RxJS is concerned.

Most applications use between 8-20 operators. I'd estimate the average is around 12-15.

@tonny008
Copy link

still have this issue in Angular CLI 6 with rxjs6 with no rxjs-compact.

@alinnert
Copy link

I found this issue because I was also experiencing this issue with a custom Webpack 4 + Babel 6 setup. I then tried Rollup just for fun to see if it makes any difference. Using RxJS and one/two pipeable operators resulted in a ~10 kb bundle with Rollup. Webpack on the other hand created a 120 kb bundle. But I noticed that Rollup requires the following line in .babelrc:

{
  "presets": [
    [
      "env",
      {
        "modules": false // <- this
      }
    ]
  ]
}

After adding this line in the .babelrc file of my Webpack setup as well its created bundle file shrank by ~110 kb to about the same size that Rollup produces.

Now I guess the Angular CLI uses TypeScript instead of Babel. From reading the documentation it seems to have a similar option in tsconfig.json called compilerOptions.module and a matching value of "None".

I didn't try this (and I don't have any Angular projects right now) but maybe this helps you finding a solution? But at least to me this also means that this is not necessarily a Webpack issue, right?

@alexeagle alexeagle added this to the 7.0.0 milestone Sep 14, 2018
@ngbot ngbot bot removed this from the 7.0.0 milestone Sep 14, 2018
@Lakston
Copy link
Contributor

Lakston commented Nov 26, 2018

Having this issue as well with all rxjs in my bundle, as well as all lodash-es when I only use 2 imports

Rx: 6.3.3
CLI: 7.0.6

@juarezpaf
Copy link

image

I have an Angular app using v6.1.8, CLI v6.2.3 and rxjs v6.3.2. Not importing anything from the internal path and it's still appearing inside of the bundle, in addition to that I am seeing bunch of operators that we are not using at all.

@alex-okrushko
Copy link

@juarezpaf One of the possibilities why this could be happening is that one of the deps libraries that you use also has rxjs in their deps/devDeps. At the same time that library could be configured to use compilerOptions.module: commonjs in their tsconfig.json file.
That's what was happening in my case.

@Splaktar
Copy link
Member Author

I'm not sure when the exact fix went in, but this is no longer an issue in Angular CLI 7.3.8 as you can verify in my updated fork of the original reproduction.

@KishoreBarik
Copy link

Facing similar issue

image

Environment:

image

Note
Earlier I was seeing the left panel (900+ KB) was occupied by polyfills(at that time Size of Rx as nearly 53KB). So to solve this I found out I had to mention "es5BrowserSupport": true in angular.json and commented all imports in polyfills.ts except import "zone.js/dist/zone";.
Now I am seeing similar amount of size is being taken by RxJs. If I uncomment the polyfill imports the RxJs is back to around 53 KB but polyfills take 900+KB.
image

I just want to get rid of this 900+ KB sized thing.

@Splaktar
Copy link
Member Author

Splaktar commented Aug 7, 2019

@KishoreBarik this issue is closed. Please open a new issue and link to this issue from your new issue.

@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
P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.