graphql-request icon indicating copy to clipboard operation
graphql-request copied to clipboard

Switch to Microbundle to reduce library size by 80%

Open developit opened this issue 2 years ago • 7 comments

Hiya! I noticed the size of this library could be reduced considerably while doing other optimization work, and figured I'd make the suggestion since I'd done the work anyway.

This replaces tsc -d with microbundle, reducing total size of the JS files in dist/ from 60kb to 12kb. I also added in an ESM build and an export map.

developit avatar Jun 08 '22 21:06 developit

Thanks @developit (Same first name and we both live in Canada :D). I'm not convinced that libraries are the right place to do bundle optimization. Why not do this at the application layer?

jasonkuhrt avatar Jun 09 '22 03:06 jasonkuhrt

@jasonkuhrt most of the optimization still needs to be done at the app level, however many of the most valuable optimizations (dead code elimination, cross-module inlining, module splitting, etc) are severely impacted by, or even disabled be, packages publishing CommonJS or UMD modules.

In addition to that, bundlers and optimizers can't undo suboptimal compiled output. As an example, this package includes a number of modules, and these are transpiled to ES5 before being published to npm. Each module contains polyfills and shims that are generated to support the source code's use of things like async/await and object spread. See the first 80 lines of this file: https://unpkg.com/browse/[email protected]/dist/index.js

Because the transpilation process is done per-file and not by a bundler that is aware of the module graph, those helpers are duplicated in any file that uses those syntax features - see the first 48 lines in this other file: https://unpkg.com/browse/[email protected]/dist/graphql-ws.js

Finally, I should point out that this PR doesn't change the structure of the generated modules - it's not bundling anything, and minification could be turned off if deemed unnecessary. (This is where the name "microbundle" is perhaps misleading, haha)

The important thing here is to use a tool that produces ES Modules in addition to CommonJS. Ideally, a tool should also generate modern (ESnext) versions of the modules - useful for apps not targeting older browsers, but also for app build systems that can then control how this module's modern code is transpiled (to produce fewer duplicate polyfills and more consistent syntax that compresses better).

For a bit of context - I'd originally opened this issue because this module was 10% of the JS weight of an application I'm working on (even after bundle optimization).

Hope that helps illuminate things!

developit avatar Jun 09 '22 21:06 developit

Thanks @developit that starts to make sense for me. Some initial thoughts I have are:

  1. Can we leverage TS 4.7 to natively emit an ESM build to support applications building optimal bundles.
  2. Can we improve our TS config settings, e.g. make sure we're depending on tslib, make sure we target something modern including native Async Await, etc.

I'm all for bringing in more tooling when it is needed, but I suspect we haven't hit the ceiling yet on what is possible now with TS.

jasonkuhrt avatar Jun 10 '22 02:06 jasonkuhrt

TS can emit ESM, no need to change versions to get that. Just set the {"module":"ESNext"} in the "compilerOptions" section of tsconfig.json.

To improve the output quality, you could set the "target" property to something like "es2017".

That being said, there are still a lot of applications that don't transpile things found in node_modules. Changing the projects tsconfig would be a big improvement, but would mean lots of folks wouldn't be able to upgrade and get those improvements. This is probably the main reason projects involve a tool like Rollup (or microbundle or tsdx, which are both just wrappers around Rollup): to generate multiple formats from the same source code. It's important for ESM and CJS, but also "ESM with modern syntax". It's a pain to do with tsconfig.

developit avatar Jun 10 '22 02:06 developit

I haven't dug deep yet but I was under the impression that TS 4.7 can emit proper ESM now such as imports that include file extensions, etc. More here: https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#ecmascript-module-support-in-node-js. But maybe I'm missing your point.

It's a pain to do with tsconfig.

This might be where we differ, I don't actually find maintaining multiple tsconfigs too bad. I do a bit of that in my lib template repo for example https://github.com/jasonkuhrt/template-typescript-lib (but I haven't updated it to use 4.7 and emit proper ESM yet).

jasonkuhrt avatar Jun 10 '22 02:06 jasonkuhrt

Ah - no, you're exactly right, I'd forgotten about the file extensions issue.

Interesting approach with the multiple tsconfigs - that actually doesn't seem bad at all. Not sure why I didn't think of running tsc multiple times.

The approach in that repo looks good to me FWIW. Think it'd be hard to airlift into this module?

developit avatar Jun 10 '22 02:06 developit

Yeah I think we can bring in the changes without too much effort. Do you have an ESM project you are using this on? Even if not, would you be up to Guinea pig canary releases to make sure we get this done optimally?

jasonkuhrt avatar Jun 10 '22 13:06 jasonkuhrt

Hi @jasonkuhrt! We are using this library in the Shopify CLI and would like it to make it an ESM dependency to improve performance (so that it can be loaded concurrently). So we are happy to help testing a canary release if needed. Thanks!

gonzaloriestra avatar Jan 12 '23 16:01 gonzaloriestra

Hey @gonzaloriestra! You can try out the next pre-release that will include https://github.com/jasonkuhrt/graphql-request/pull/455.

jasonkuhrt avatar Feb 18 '23 03:02 jasonkuhrt

@jasonkuhrt thanks a lot! We'll give it a try

gonzaloriestra avatar Feb 23 '23 17:02 gonzaloriestra

We have been testing the pre-release and it looks good, but we'll better wait for the next stable release to include it. Any plans for it? Thanks again.

gonzaloriestra avatar Feb 27 '23 11:02 gonzaloriestra

In March for sure.

jasonkuhrt avatar Feb 27 '23 14:02 jasonkuhrt