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

chore: Migrate build tool #270

Merged
merged 5 commits into from
Apr 19, 2021
Merged

Conversation

mttrbrts
Copy link
Member

@mttrbrts mttrbrts commented Apr 19, 2021

Migrate from webpack to rollup for bundling. Rollup is more appropriate for a library such as this, whereas webpack is better suited to web apps. This also aligns this repo with @accordproject/web-compoennts

Changes

  1. The browser bundle has moved from umd/concerto.js to dist/concerto.min.js.
  2. Tuning .npmignore.

Signed-off-by: Matt Roberts <[email protected]>
Signed-off-by: Matt Roberts <[email protected]>
Signed-off-by: Matt Roberts <[email protected]>
@jeromesimeon
Copy link
Member

jeromesimeon commented Apr 19, 2021

Migrate from webpack to rollup for bundling. Rollup is more appropriate for a library such as this, whereas webpack is better suited to web apps. This also aligns this repo with @accordproject/web-compoennts

Changes

  1. The browser bundle has moved from umd/concerto.js to dist/concerto.min.js.
  2. Tuning .npmignore.

This looks nice, but also slightly disconcerting. A bit curious about:

  1. tradeoffs between webpack and rollup: why is rollup better for this kind of library? also based on the description, shouldn't web-components use webpack?
  2. why do we care about bundling at all. Could we just remove it completely? wouldn't web components end up bundling concerto anyway?
  3. why now? what problem are we trying to solve?

@mttrbrts
Copy link
Member Author

  1. tradeoffs between webpack and rollup: why is rollup better for this kind of library? also based on the description, shouldn't web-components use webpack?

There are lots of comparisons out there, e.g. https://betterprogramming.pub/the-battle-of-bundlers-6333a4e3eda9

web-components is a library for building web apps, but is not a web app itself.

  1. why do we care at all, wouldn't web components end up bundling concerto anyway?

By bundling and performing optimisations such as tree-shaking and minimisation, all dependencies of concerto benefit from reduced package sizes. Optimising here also gives us greater control than relying than on upstream dependants. Our npmjs listing today, says that our package size is >1MB which is misleading at best (the compressed version of the UMD bundle is ~80kB), and suggests many other poor things at worst. As the root package in the AP stack, I wanted to start my investigations here because it will give us the biggest ROI.

  1. why now? what problem are we trying to solve?

I originally attempted to solve a different problem, then rolled-back and this PR is what remained.

Currently, in the npm package that is published by this repo, we include two things that are redundant:

  1. The source files in the lib/ folder
  2. A UMD bundle of the source files in the umd/ folder.
    You can run npm pack @accordproject/[email protected] to view the contents of the published package, or just npm pack to build the package .tgz locally.

UMD is compatible with Node JS and browsers, but other formats are preferable (ESM for Node, and IIFE for browsers). However, after investigation, I wasn't confident that I had a solution that solved this problem and retained CommonJS for developing / testing locally. I might pick this up again, but I need to move onto other things now.

I was also hoping that a switch to Rollup would give us an improvement in the size of the UMD bundle; it didn't, it's broadly the same.

Another conclusion was that to really benefit from a performance optimisation, changing the bundle format (UMD vs IIFE vs CommonJS vs ESM) makes margin impact because of the dominance of the parser which is more than 90% of the total package size. This is further motivation for #55 (comment)

@jeromesimeon
Copy link
Member

  1. tradeoffs between webpack and rollup: why is rollup better for this kind of library? also based on the description, shouldn't web-components use webpack?

There are lots of comparisons out there, e.g. https://betterprogramming.pub/the-battle-of-bundlers-6333a4e3eda9

Another benefit of WebAssembly was staring us in the face all along.

web-components is a library for building web apps, but is not a web app itself.

gotcha.

  1. why do we care at all, wouldn't web components end up bundling concerto anyway?

By bundling and performing optimisations such as tree-shaking and minimisation, all dependencies of concerto benefit from reduced package sizes. Optimising here also gives us greater control than relying than on upstream dependants. Our npmjs listing today, says that our package size is >1MB which is misleading at best (the compressed version of the UMD bundle is ~80kB), and suggests many other poor things at worst. As the root package in the AP stack, I wanted to start my investigations here because it will give us the biggest ROI.

Understood, but clearly this looks like an investigation and if we like it, it feels like we are signing up for applying this to the rest of the AP stack (or feels like we should which is a pretty significant effort).

  1. why now? what problem are we trying to solve?

I originally attempted to solve a different problem, then rolled-back and this PR is what remained.

Currently, in the npm package that is published by this repo, we include two things that are redundant:

  1. The source files in the lib/ folder
  2. A UMD bundle of the source files in the umd/ folder.
    You can run npm pack @accordproject/[email protected] to view the contents of the published package, or just npm pack to build the package .tgz locally.

UMD is compatible with Node JS and browsers, but other formats are preferable (ESM for Node, and IIFE for browsers). However, after investigation, I wasn't confident that I had a solution that solved this problem and retained CommonJS for developing / testing locally. I might pick this up again, but I need to move onto other things now.

I was also hoping that a switch to Rollup would give us an improvement in the size of the UMD bundle; it didn't, it's broadly the same.

Another conclusion was that to really benefit from a performance optimisation, changing the bundle format (UMD vs IIFE vs CommonJS vs ESM) makes margin impact because of the dominance of the parser which is more than 90% of the total package size. This is further motivation for #55 (comment)

Looks nice all around though, I'll approve and let's see how it goes.

Copy link
Member

@jeromesimeon jeromesimeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice as a migration away from webpack.

@mttrbrts mttrbrts merged commit 21262aa into accordproject:master Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants