Amplitude-TypeScript icon indicating copy to clipboard operation
Amplitude-TypeScript copied to clipboard

Angular non-optimized CommonJS or AMD module

Open ChrisJohns-me opened this issue 3 years ago • 4 comments

After installing @amplitude/analytics-browser v1.6.1 onto an Angular project, running ng serve causes Angular to throw a warning:

Warning: node_modules\@amplitude\analytics-browser\lib\esm\index.js depends on '@amplitude/analytics-types'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies
Warning: node_modules\@amplitude\analytics-browser\lib\esm\plugins\context.js depends on '@amplitude/ua-parser-js'. CommonJS or AMD dependencies can cause optimization bailouts.        
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

On Angular's website, they warn:

It is recommended that you avoid depending on CommonJS modules in your Angular applications. Depending on CommonJS modules can prevent bundlers and minifiers from optimizing your application, which results in larger bundle sizes. Instead, it is recommended that you use ECMAScript modules in your entire application. For more information, see How CommonJS is making your bundles larger.

This causes unoptimized and un-treeshakeable code, and causes our production code to be larger than intended (+100KB).

Expected Behavior

ng serve

Current Behavior

ng serve outputs a warning for CommonJS or AMD dependencies to not be used.

Possible Solution

Update the Amplitude codebase to use ECMAScript Modules.

Steps to Reproduce

  1. npm install @amplitude/analytics-browser
  2. Add an import to @amplitude/analytics-browser into your TypeScript codebase
  3. Run ng serve

Environment

  • Installation Method: npm, import { identify, Identify, init, track } from "@amplitude/analytics-browser";

ChrisJohns-me avatar Nov 08 '22 22:11 ChrisJohns-me

Hi @ChrisJohns-me, thank you for choosing Amplitude. The packages in this repo are available in both CommonJS and ESM formats.

For @amplitude/analytics-types, it's package.json includes: https://github.com/amplitude/Amplitude-TypeScript/blob/f556fd2194aebce71de5b2b53c12e611d8c9b5a7/packages/analytics-types/package.json#L8-L10

My understanding is that with module pointing to an ESM file, bundlers use this during bundle time to perform tree shaking. Let me know what you think. We're happy to work with you on this issue.

For @amplitude/ua-parser-js, this is actually a fork of https://github.com/faisalman/ua-parser-js and bloats our SDK bundle size (biggest offender). Our roadmap includes work to remove this as dependency.

kevinpagtakhan avatar Nov 08 '22 22:11 kevinpagtakhan

Hi @ChrisJohns-me, wanted to give you an update. Getting rid of our dependency on @amplitude/ua-parser-js is still very important to us and me personally. Once addressed, this should help with the warnings you have surfaced here. Thank you.

kevinpagtakhan avatar Jan 11 '23 09:01 kevinpagtakhan

Created a ticket on our end to optimize the bundle size by removing or improving the @amplitude/ua-parser-js.

yuhao900914 avatar Mar 09 '23 20:03 yuhao900914

+1 to this issue

hanna-becker avatar Apr 25 '23 14:04 hanna-becker