Narrow dependency importing to improve commonjs bundling

[timfish]

A couple of the JavaScript contexts in Electron have to use commonjs which can't be tree shaken. This means simply importing Store gives you a base bundle of 360KB.

After these changes, it's down to 150KB as reflect-metadata now isn't required either.

[zewa666]

Have taken a look where the rest is coming from? 150kb is still quite a lot.

[timfish]

RxJs is just under 120k of that.

[timfish]

Actually, I think this might work but it'll need more testing with webpack:

import { Observable } from "rxjs/internal/Observable";
import { BehaviorSubject } from "rxjs/internal/BehaviorSubject";

[timfish]

The above helped a bit, but the main culprit is the top level export of test-helpers which brings in all the operators.

export * from "./test-helpers"

Commenting that out and the bundle is down to 45k:

The Rxjs type definitions seem to suggest that you can import the individual operators using rxjs-compat but I don't think we should take a dependency on that.

Would moving test-helpers out from the top level export be a semver major change?

[zewa666]

I thought that Rx is the issue. Sadly the new way of v6 is very bad compared to v5 where patched imports allowed much better optimizations. Found some hints here in a link about path definitions https://chat.stackoverflow.com/rooms/172455/discussion-on-question-by-royi-namir-rxjs-v6-loads-many-unnecessary-modules. I'm just not very good with Webpack.

Anyways with regards to test-helpers doesnt changing its imports to internal reduce the size as well? Excluding them from the barrel export would be a breaking change

[zewa666]

Here is the link i've been searching for https://github.com/ReactiveX/rxjs/blob/d231053616ab4e5d2541818314110df626e6bf5a/doc/pipeable-operators.md#build-and-treeshaking

[zewa666]

Also you shouldnt import from internal, as the name suggests, as those are likely to be changed.

[zewa666]

Also maybe give the last comment here a look. angular/angular-cli#9069

Really i think the trouble is more about the bundler, specifically webpack, than the Store plugin itself.

[timfish]

Thanks for the helpful links. It does looks like the Rxjs issue is webpack related.

I've reverted the changes to only the Aurelia references which simply stops aureila-teamplating and aurelia-binding from being included.

[zewa666]

Ok I've been setting up an empty sample project:

// webpack.config.js
module.exports = {
  entry: './src/index.ts',
  module: {
    rules: [
      {
        test: /\.tsx?$/,
        use: 'ts-loader',
        exclude: /node_modules/
      }
    ]
  },
  resolve: {
    extensions: [ '.tsx', '.ts', '.js' ]
  },
  output: {
    filename: 'bundle.js',
    path: path.resolve(__dirname, 'dist')
  }
};

// tsconfig.json
{
  "compilerOptions": {
    "target": "es5",                       
    "module": "commonjs",                
    "strict": true,                        
    "esModuleInterop": true         
  }
}

// src/index.ts
import "aurelia-store";

1. Original: 345kb
2. With your changes: 156kb
3. With 2 + Observable type-only import: 152kb
4. With 3, without test-helpers barrel export: 77kb
5. With 4 and BehaviorSubject import from internal: 93kb

So in summary this is pretty much what I expected. Importing from internal is pretty much unoptimized and should be avoided. Your changes to removing imports from aurelia-framework won't likely help for normal browser apps, since its very likely they happen from a user base. But for the scenario of a node-app this should significantly reduce the size. Also removing test-helpers will benefit for a naked app but since I expect quite a lot of apps to import directly from rxjs/operators the benefit is gone as soon as the user makes use of any which is definitely likely to happen.
Going the rxjs-compat approach is suboptimal as it lets you treat your v6 in v5 mode, which will become deprecated over time.

As a key take-away I think removing the barrel export for test-helpers won't help anything as likely most apps will make use of at least one operator, which again will bring back the bundle-size. What could at least reduce 4kb would be to switch to type only imports for Observable.
So this line becomes this:

public readonly state: import("rxjs").Observable<T>;

and the import of Observable should be removed. Can't do the same for BehaviorSubject since we need the actual class implementation as well.

[zewa666]

Sadly as a recap I have to fully agree with @benlesh's tweet here https://twitter.com/BenLesh/status/914909312056672256.

Rollup is really mature when it comes at those micro-optimizations and I'd really like the world see stopping to worry about stupid bundlers and get http2 and proper lazy-loading going on, so great people like him can continue on working on great products vs having to care about ugly bundler configurations.

[timfish]

I agree that we shouldn't spend too much time on ES5 + CommonJs bundling output.

In Electron where no network is involved, a 150k bundle size is perfectly acceptable.

[zewa666]

thx for the help @timfish