fix(amazon-cognito-identity-js): avoid unnecessary crypto polyfills in webpack builds

[nnyath]

Issue #, if available:

#7313

Description of changes:

Webpack bundle sizes are unnecessary large when using anything that has a dependency/sub-dependency on amazon-cognito-identity-js@^4.5.2 or @aws-amplify/auth@^3.4.9

This is due to Webpack using polyfills for crypto node libs (https://stackoverflow.com/a/45862270). In this case it was added as a dynamic require in this PR #7136.

Even though it's a conditional require, Webpack will still evaluate the line and include it into the final build raising bundle sizes by at least ~125kb (gzipped)

Recreate:

You can recreate this with a fresh CRA install and adding any one of these deps (but there's probably more affected by this)

import { API, Auth, PubSub } from 'aws-amplify'

import API from '@aws-amplify/api'
import Auth from '@aws-amplify/auth'
import PubSub from '@aws-amplify/pubsub'

import * as CognitoJS from 'amazon-cognito-identity-js'

Analysis:

NOTE: Sizes are gzipped

A simple Hello World bundle size gives me
41.21 KB

Adding @aws-amplify/auth gives me an increase of
235.4 KB (+194.19 KB)

Here's what the source-map-explorer output is (notice a lot of browserify-sign, bn.js, elliptic, etc)

which corresponds to the SO answer referenced above.

After this fix, the bundle drops a respectable amount
108.95 KB (-126.44 KB)

Here's the source-map-explorer after the fix which looks much better.

Though, I think there still might be a few other polyfills being added due to seeing node-lib-browser

Caveats:

This will only work for Webpack configs that specifically look for and apply .web. files, which fortunately CRA has supported for a while now. But other custom/dated configs or tooling that reference the ESM output could still run into this issue. *Edit - Not familiar with other tooling options (rollup), but I suppose it's up to how they generally handle references to node libs and whether or not to error out or provide a polyfill

Here's some specific Webpack things that might work

• https://webpack.js.org/guides/code-splitting/#dynamic-imports
• https://webpack.js.org/api/module-methods/#requireensure
• Conditional based onprocess.env would omit the require, but not enforceable enough I think

but again, these seem specific to Webpack, and probably don't belong in the source code. Not sure if there's a universal fix here, but maybe someone more knowledgeable in this area can chime in.

*Edit - Might be fine since consumers shouldn't need to worry about the building the source code using configs/tooling outside of this repo, as long as the build outputs correctly we should be good

*Edit - The other alternative would be to only use the crypto functions needed and providing those via a cross platform compatible lib/implementation as suggested by the SO answer.

However, I think the current proposed approach of adding in .web. variant in is a step in the right direction and not likely to introduce additional regressions.

I also see there's some work on monitoring bundle sizes in #7500 which is great, but I think an issue like this would have flown under the radar.
*Edit: Overlooked the real world bundle size section, I would think this would be caught in that

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov]

Codecov Report

Merging #7521 (83af45c) into main (2a53bef) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #7521   +/-   ##
=======================================
  Coverage   73.96%   73.96%           
=======================================
  Files         213      213           
  Lines       13376    13376           
  Branches     2622     2622           
=======================================
  Hits         9894     9894           
  Misses       3283     3283           
  Partials      199      199           

Impacted Files	Coverage Δ
packages/core/src/Platform/version.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa54c4f...83af45c. Read the comment docs.

[amhinson]

Thanks for this @nnyath! 🙏

I was able to validate the bundle size improvements with a CRA app. As you mentioned, there is still room for improvement regarding this crypto module as it relates to other custom configurations and framework defaults, but this seems like a quick win right now for CRA developers. Because of the various other platforms that this library supports, implementing any of the other alternative solutions you provided would be rather difficult or impossible at this time.

I'd also like to get @ericclemmons's thoughts on this before merging.

[ericclemmons]

I'll add this test to #7500 as well!

I'm 👍 for including this. Unfortunately, it doesn't fix "default" behavior with webpack@4, but it will benefit CRA users, who make up a sizeable % of installs.

[github-actions]

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.