[WIP] Reduce bundle size. #2661

benjamn · 2018-12-13T21:43:41Z

Part of issue #2743, whose description previously appeared here.

rosskevin · 2018-12-21T17:15:33Z

Reimplementing surprisingly heavy dependencies locally, a la 4bac03a.

@benjamn as noted in the code removed, lodash.flowright is slated to and should be removed altogether. It is not part of this package and is re-exported. I would definitely rather not see us copy something into this project that should not have been here to begin with. Is there an argument for keeping/supporting it from here instead of dropping it?

Transforming imports to be more specific using a compiler plugin, so that import { SpecificThing } from "react-apollo" becomes import { SpecificThing } from "react-apollo/lib/specific-thing". If we end up pursuing this approach for apollo-client, we will no doubt reuse the same tools here.

This should be unnecessary with tree shaking. Aside from the conveniences of browser vs server indexes, file specific paths should be a thing of the past and we should be able to confidently use an index to generate the rollup bundle and have dead code eliminated in the application bundling phase.

Introducing a production bundle in addition to the development bundle, with more aggressive elimination of unnecessary code (e.g. error strings).

👍

Moving away from monolithic bundles and towards individual modules.

I'm not sure this makes sense.

The key distinction here: let's not conflate a rollup bundle size produced with the size of the library in an application bundled with a modern tool like webpack or parcel that eliminates dead code.

While perhaps there is an argument for different entry points for server vs browser, I do not understand going further down this path (file-specific imports, more split modules) when it should have a zero impact on app bundling over the current approach. With that said, things like prod vs dev bundles and #2677 can definitely help.

rosskevin · 2018-12-21T17:35:06Z

WOW - my error. We are only producing umd for consumption - which appears to be the root cause of this.

We need to configure our rollup to be producing an es module to aid in dead code elimination. Some examples:

We definitely need to be shipping an es/esm variant otherwise we are stuck with a non-shakeable (or poorly shakeable depending on the tool) umd.

JoviDeCroock · 2018-12-21T17:45:50Z

WOW - my error. We are only producing umd for consumption - which appears to be the root cause of this.

We need to configure our rollup to be producing an es module to aid in dead code elimination. Some examples:

https://github.com/final-form/react-final-form/blob/master/rollup.config.js

https://github.com/dmtrKovalenko/material-ui-pickers/blob/develop/lib/rollup.config.js

We definitely need to be shipping an es/esm variant otherwise we are stuck with a non-shakeable (or poorly shakeable depending on the tool) umd.

I can add this in my PR / make a seperate one for that if needed?

We are offering the index.js as a module or is this not sufficient, because afaik it should be enough.

rosskevin · 2018-12-21T18:38:00Z

@JoviDeCroock definitely separate.

benjamn · 2018-12-21T19:27:32Z

Right, I would have thought it was sufficient that the module field of package.json points to lib/index.js, which is ESM? Isn't that what Rollup cares about?

JoviDeCroock · 2018-12-21T19:48:13Z

Afaik the tsc compiled bundle suffices for ESM, ESM is just ecmascript module syntax. Some packages give an .mjs there now but that isn't doable in frontend.

I think it's safe to say it's meant to be an ES bundle and not ESM

Maybe it should be es6 target in tsconfig that's still a question i'm playing with.

rosskevin · 2018-12-21T19:56:11Z

Right, I would have thought it was sufficient that the module field of package.json points to lib/index.js, which is ESM? Isn't that what Rollup cares about?

My error once again, I saw the lack of target in the rollup.config as well as "module": "src/index.ts". I just verified https://unpkg.com/[email protected]/package.json and in my local app build that we are tree shaking - no sign of the umd. But with that said, the tsc is producing es5 instead of es2015. So we aren't actually providing an es module, but a transpiled down es5 src.

So this issue is about optimizing bundle size - is there a reason to focus on the umd bundle size? Is anyone focused on bundle size in their app using a umd build? If so why? perhaps I'm too far down my own path to understand what the problem we are attempting to solve.

One thing I am sure of - we need a js file next to a .d.ts in the artifact to satisfy downstream usage of ts definitions. There currently (as of two months ago) was no rollup for .d.ts files.

I would think we could provide src as we do currently, but change the module to point to a rollup-produced es2015 target. This would cover dev workflow and optimal bundling (I think, unless I'm missing something. Seems like we just touched on this in material-ui-pickers referenced above).

JoviDeCroock · 2018-12-23T09:53:15Z

I have been giving this some thought and for the pkg.json wouldn't it be more normal to make the es5 tsc compiled index.js the main and a new rollup generated ES2015 bundle the module. This in turn allows for better tree shaking. I'll make a POC PR this afternoon/evening when I find some time

rosskevin · 2018-12-27T17:38:06Z

Agreed @JoviDeCroock, just like I PR'd in apollo-client apollographql/apollo-client#4261:

main exploded cjs files and .d.ts
module esm bundle
browser umd bundle

The umd bundle is legacy and helps with putting your finger on the pulse of bundlesize (though not a foolproof indicator as we have discussed).

Users concerned with tree shaking should ensure they are using the targeted module bundle.

JoviDeCroock · 2018-12-30T16:55:30Z

I was wondering, now we are creating a seperate bundle for the umd libraries, can't this be solved with the rollup 1.0 feature to have multiple entry points?

rosskevin · 2018-12-31T16:16:34Z

I was wondering, now we are creating a seperate bundle for the umd libraries, can't this be solved with the rollup 1.0 feature to have multiple entry points?

Looks like there could be a few different ways to tackle this, one would be using the same index inputs and have multiple outputs (just makes the config more DRY):

{
  input: 'main-b.js',
  output: [
    {
      file: 'dist/bundle-b1.js',
      format: 'cjs'
    },
    {
      file: 'dist/bundle-b2.js',
      format: 'esm'
    }
  ]
}

Or the multiple inputs with pattern named outputs:

input
Type: string | string [] | { [entryName: string]: string }
CLI: -i/--input <filename>

The bundle's entry point(s) (e.g. your main.js or app.js or index.js). If you provide an array of entry points or an object mapping names to entry points, they will be bundled to separate output chunks. Unless the output.file option is used, generated chunk names will follow the output.entryFileNames option. When using the object form, the [name] portion of the file name will be the name of the object property while for the array form, it will be the file name of the entry point.

This will generate at least two entry chunks with the names index-a.js and index-b.js:

// rollup.config.js
export default {
  ...,
  input: {
    a: 'src/main-a.js',
    b: 'src/main-b.js'
  },
  output: {
    ...,
    entryFileNames: 'entry-[name].js'
  }
};

Definitely plenty of good/new options there.

Pajn · 2019-01-09T12:32:01Z

Is it possible to try out this PR, eg. is it published anywhere?

It would be interesting to see if the esm bundle would allow some tree shaking.

JoviDeCroock · 2019-01-09T12:34:18Z

Is it possible to try out this PR, eg. is it published anywhere?

It would be interesting to see if the esm bundle would allow some tree shaking.

I'd also love to see this being tried out. Been reading some things about webpack being not that good at treeshaking bundles but leaving that to the tersers dead code elimination. If we can get it tested before we do a real publish we could still alter the course for that matter.

Use our own flowRight-like function instead of using lodash.flowright. In addition to being smaller, this implementation does not require using CommonJS require to import a separate package.

This reduces the minified size of lib/react-apollo.umd.js from 28074B to 22773B (18.9%). The minified+gzip size drops from 6930B to 6451B (6.9%). The gzip percentage is smaller because there was some repetition between the multiple declarations of the __extends function, and gzip is good at compressing long repeated substrings.

* chore: optimize build time * fix: remove comments * tests: revert transforms * fix tests * remove babel key from pkg.json * chore: apply pr remarks * chore: undo all and add esm bundle with potential todo * remove unused babel plugins * cjs * some optimizations * add question * chore: follow pr * ignore rpt_cache

@JoviDeCroock

@JoviDeCroock @Pajn @rosskevin I'm hoping this prerelease helps with testing the changes in PR #2661.

benjamn · 2019-01-20T16:23:01Z

According to BundlePhobia, version 2.5.0-bundle-size.0 is now 25.4KB minified and 7.4KB after gzip, compared to 32.7KB minified and 8.7KB after gzip for the latest official version, 2.4.0. Since the gzip size is what really matters here, that's a 15% improvement! 🎉

JoviDeCroock · 2019-01-20T16:31:34Z

According to BundlePhobia, version 2.5.0-bundle-size.0 is now 25.4KB minified and 7.4KB after gzip, compared to 32.7KB minified and 8.7KB after gzip for the latest official version, 2.4.0. Since the gzip size is what really matters here, that's a 15% improvement! 🎉

Why does it indicate 2.4.0 as tree-shakeable, the UMD build provided there should not be tree shakeable as far as I know.

Also will use the bundle size version in my production application, that way we can see if there are any unforseen consequences.

benjamn · 2019-01-20T16:34:40Z

Why does it indicate 2.4.0 as tree-shakeable

I wonder if that just means there's an ESM module field in package.json?

This shaves a tiny amount of bundle size, but more importantly it implements shallowEqual using TypeScript, and avoids one of the few remaining uses of require.

JoviDeCroock · 2019-01-21T07:42:27Z

Does anyone have an SSR application using react-apollo. To test the new bundle-size build? Because I don't know how node reacts to the new pkg.json entries.

JoviDeCroock · 2019-01-24T19:17:22Z

Something that comes in my mind when thinking about reducing bundlesize is maybe implementing our render props/hoc/... with a hook. That way we get the advantage of exposing a hook AND our components can be reduced to only functions.

This reduces the transpiling overhead of classes needing to be added.

Just throwing it out there because well I can see as to why you could be opposed to this idea (supporting more react versions)

rosskevin · 2019-01-24T19:25:19Z

So that will be interesting....

Implementation in hook
Render callbacks use hook
hoc uses render callback

I'm already on render callbacks but will switch to hooks when they are ready. Is there an official roadmap for that now that hooks have been merged in react and will be released next?

JoviDeCroock · 2019-01-24T19:28:51Z

probably very soon since I just saw the PR about the changelog etc... So yes, I don't mind working on it I've already implemented quite a few hooks libraries, I just don't know if this is something apollo needs right now since it will probably be a major release. Will safe a massive amount of bundle size though.

Also needs to be looked at if ssr has compat (I assume so by now)

OneCyrus · 2019-01-26T09:30:03Z

why is the module field in package.json pointing to a ts-file? IMHO rollup should produce ES modules and this field should point there.

JoviDeCroock · 2019-01-26T09:35:25Z

why is the module field in package.json pointing to a ts-file? IMHO rollup should produce ES modules and this field should point there.

This is the case in master but not in 2.5.0-bundle-size.0
https://github.com/apollographql/react-apollo/pull/2661/files#diff-f764c456bc424cc70f25f27247ccf338R23

OneCyrus · 2019-01-26T09:47:21Z

ah ok i see. but the tsconfig.json still transpiles to target:es5 which doesn't produce ES modules

JoviDeCroock · 2019-01-26T10:07:05Z

ah ok i see. but the tsconfig.json still transpiles to target:es5 which doesn't produce ES modules

It does, it literally sais "es", it is just a transpiled down version of es6. No library transpiles to es6 in their module field. This would imply very long builds since your node modules should be transpiled to support all browsers

Similar in spirit to apollographql/apollo-link#959 This inlining was first introduced in PR #2661 with the following commit: de2b5fc At the time, inlining made sense because TypeScript was injecting copies of the __extends, __rest, etc. helpers into every module that used them. Depending on the tslib package seemed undesirable because the available bundle size measurement tools (e.g. bundlephobia.com) mistakenly counted the entire tslib package against react-apollo, without acknowledging the possibility of sharing that package between multiple Apollo packages. It seemed safer to inline only the helpers we needed at the top of lib/react-apollo.esm.js. Now that we have a more holistic way to measure bundle sizes (#2839), and react-apollo works better with tree-shaking tools (#2659, #2661, #2677), we know that overall application bundle sizes benefit from sharing a single copy of the tslib helper package, even if no tree-shaking is happening. Of course, with tree-shaking, that one copy of the tslib package can be shrunk to contain just the helpers that are actually used.

benjamn assigned benjamn and hwillson Dec 13, 2018

benjamn requested a review from hwillson as a code owner December 13, 2018 21:43

benjamn added help-wanted refactor in-progress PR: encouraged labels Dec 13, 2018

benjamn force-pushed the wip-reduce-bundle-size branch 2 times, most recently from 00fa9be to f3da910 Compare December 14, 2018 18:40

rosskevin mentioned this pull request Dec 21, 2018

chore: provide esm bundle #2677

Merged

rosskevin mentioned this pull request Dec 28, 2018

2.1.0 breaks compose if you're using lodash-webpack-plugin #1831

Closed

benjamn mentioned this pull request Jan 20, 2019

[Ongoing] Reduce bundle sizes. #2743

Closed

3 tasks

JoviDeCroock and others added 5 commits January 20, 2019 10:55

Chore/reduce bundle size (#2659)

20c8048

Use our own flowRight-like function instead of using lodash.flowright. In addition to being smaller, this implementation does not require using CommonJS require to import a separate package.

Move walkTree export to react-apollo/walkTree, to avoid bundling it.

2757f63

Bump react-apollo to version 2.5.0-bundle-size.0.

f0c395b

@JoviDeCroock @Pajn @rosskevin I'm hoping this prerelease helps with testing the changes in PR #2661.

benjamn force-pushed the wip-reduce-bundle-size branch from 63a83ab to f0c395b Compare January 20, 2019 16:17

Stop importing shallowEqual from fbjs/lib/shallowEqual.

b72dfd6

This shaves a tiny amount of bundle size, but more importantly it implements shallowEqual using TypeScript, and avoids one of the few remaining uses of require.

benjamn force-pushed the wip-reduce-bundle-size branch from a21216d to b72dfd6 Compare January 20, 2019 16:53

Merge branch 'master' into wip-reduce-bundle-size

e2ef37a

benjamn changed the base branch from master to release-2.5.0 January 28, 2019 21:09

benjamn added this to the Release 2.5.0 milestone Jan 28, 2019

benjamn merged commit dca7f4c into release-2.5.0 Jan 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Reduce bundle size. #2661

[WIP] Reduce bundle size. #2661

benjamn commented Dec 13, 2018 •

edited

Loading

rosskevin commented Dec 21, 2018 •

edited

Loading

rosskevin commented Dec 21, 2018 •

edited

Loading

JoviDeCroock commented Dec 21, 2018 •

edited

Loading

rosskevin commented Dec 21, 2018

benjamn commented Dec 21, 2018

JoviDeCroock commented Dec 21, 2018 •

edited

Loading

rosskevin commented Dec 21, 2018

JoviDeCroock commented Dec 23, 2018

rosskevin commented Dec 27, 2018

JoviDeCroock commented Dec 30, 2018

rosskevin commented Dec 31, 2018

Pajn commented Jan 9, 2019

JoviDeCroock commented Jan 9, 2019

benjamn commented Jan 20, 2019

JoviDeCroock commented Jan 20, 2019 •

edited

Loading

benjamn commented Jan 20, 2019

JoviDeCroock commented Jan 21, 2019

JoviDeCroock commented Jan 24, 2019

rosskevin commented Jan 24, 2019

JoviDeCroock commented Jan 24, 2019

OneCyrus commented Jan 26, 2019

JoviDeCroock commented Jan 26, 2019

OneCyrus commented Jan 26, 2019

JoviDeCroock commented Jan 26, 2019

[WIP] Reduce bundle size. #2661

[WIP] Reduce bundle size. #2661

Conversation

benjamn commented Dec 13, 2018 • edited Loading

rosskevin commented Dec 21, 2018 • edited Loading

rosskevin commented Dec 21, 2018 • edited Loading

JoviDeCroock commented Dec 21, 2018 • edited Loading

rosskevin commented Dec 21, 2018

benjamn commented Dec 21, 2018

JoviDeCroock commented Dec 21, 2018 • edited Loading

rosskevin commented Dec 21, 2018

JoviDeCroock commented Dec 23, 2018

rosskevin commented Dec 27, 2018

JoviDeCroock commented Dec 30, 2018

rosskevin commented Dec 31, 2018

Pajn commented Jan 9, 2019

JoviDeCroock commented Jan 9, 2019

benjamn commented Jan 20, 2019

JoviDeCroock commented Jan 20, 2019 • edited Loading

benjamn commented Jan 20, 2019

JoviDeCroock commented Jan 21, 2019

JoviDeCroock commented Jan 24, 2019

rosskevin commented Jan 24, 2019

JoviDeCroock commented Jan 24, 2019

OneCyrus commented Jan 26, 2019

JoviDeCroock commented Jan 26, 2019

OneCyrus commented Jan 26, 2019

JoviDeCroock commented Jan 26, 2019

benjamn commented Dec 13, 2018 •

edited

Loading

rosskevin commented Dec 21, 2018 •

edited

Loading

rosskevin commented Dec 21, 2018 •

edited

Loading

JoviDeCroock commented Dec 21, 2018 •

edited

Loading

JoviDeCroock commented Dec 21, 2018 •

edited

Loading

JoviDeCroock commented Jan 20, 2019 •

edited

Loading