Open williazz opened 1 week ago
This CR restores treeshaking by compiling to native ES5 modules instead of CommonJS.
Question for my understanding since I have limited understanding of this - Will this lead to issues with folks with CJS applications to be able to use the package?
Question for my understanding since I have limited understanding of this - Will this lead to issues with folks with CJS applications to be able to use the package?
Ultimately, no because this package is for the Browser runtime, whereas CommonJS is Node.js and meant for servers. However, we may see issues in some dev environments, similar to what happened with the web vital attributions imports.
I can add additional integ tests for cwr.js
and npm builds to give us more confidence, as well as attach evidence of manual e2e testing.
This only affects the CDN build cwr.js
for folks using importing our package in HTML. Those who use CommonJS would use NPM to import aws-rum-web
, which is not affected by these changes.
Update: I have updated the configuration to target the default browser targets. This reduces the build size to the following:
As a follow up, I am working on improving support for older targets by adding polyfills via corejs; however that is still a WIP. While the polyfills work on all browsers on unix/macos, none of them work on windows yet. Those polyfills will cost us 5-20 kb, so up for discussion as well once I raise another PR.
Update: after internal discussion, we will move forward with fixing tree shaking. If we need additional support, we can explore expanding some polyfills, or releasing an alternate build.
Follow up action is being tracked here: https://github.com/aws-observability/aws-rum-web/pull/567
Browsers list defaults
0.5%, last 2 versions, Firefox ESR, not dead
These are the browsers now supported. This is a thoroughly vetted, standardized list and does not often change. The only risk of change is if a dead browser falls below 0.5% circulation, which has already happened with IE.
If there are additional browsers we need to target, then we can add them in the future.
Summary
These changes reduce the size of
cwr.js
from ~230KB to 141KB. The problem is that we are including unused dependencies because our tree shaking is broken. From webpack documentation: https://webpack.js.org/guides/tree-shaking/This PR restores treeshaking by compiling to native ES5 modules instead of CommonJS.
However, this only fixes the CDN build and does not impact the NPM build in any way. While the CDN build is compiled by webpack and outputs to
build/assets/cwr.js
, the NPM build is managed bytsc
and outputs to thedist
directory.Example
We have a dependency on UUID's
v4
function that is currently taking 15 kb. However, nearly all of that is stuff that we do not need, such as v1, md5, etc.After tree shaking, we now only import
v4
and drop everything else, reducing the stat size from 24 kb to 3 kb, and the parsed size is much lower.Backwards compatibility
We can reduce package size even further by specifying browser targets. I've used the default targets from documentation, which look good against a quick check of which browsers people actually use.
In general, we can trade backwards compatibility for package size by setting browser targets to earlier versions. By default, the TerserPlugin for Webpack attempts to provide the broadest support possible.
Risk
Before deploying, we should do some manual testing and run the smoke tests for backward compatibility since we are switching the CDN build from CommonJS to ES15.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.