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

feat: add esm entry point #440

Merged
merged 1 commit into from
Nov 14, 2021
Merged

feat: add esm entry point #440

merged 1 commit into from
Nov 14, 2021

Conversation

seanparmelee
Copy link
Contributor

Summary

I noticed the build produces an amplitude.esm.js file, but is not referenced in the package.json. This PR adds the module entry point to the package.json so folks using ESM along with bundlers like webpack, parcel, etc will automatically use the ESM version of amplitude rather than the UMD/CJS version.

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?: No

Sorry, something went wrong.

@qingzhuozhen qingzhuozhen merged commit 3e98d50 into amplitude:main Nov 14, 2021
github-actions bot pushed a commit that referenced this pull request Nov 16, 2021
# [8.11.0](v8.10.0...v8.11.0) (2021-11-16)

### Bug Fixes

* add missing setLibrary API to snippet distribution ([#445](#445)) ([ff44909](ff44909))

### Features

* add esm entry point ([#440](#440)) ([3e98d50](3e98d50))
@taytestokes
Copy link

taytestokes commented Jan 25, 2022

It appears that trying to use a named import of the Identify constructor from amplitude-js breaks after this change.

import { Identify } from 'amplitude-js'

The project that I was working with was currently on v8.8 and the named import of the Identify constructor was working, but after updating to v8.11+ it failed with a type error that Identify wasn't a function.

The resolution was to target the Identify constructor through the prototype of the defaulted export which gets set here:

Amplitude.prototype.Identify = Identify;

Example of use through the prototype:

addUserProperty(property, value) {
    const identify = new amplitude.Identify().add(property, value);
    this._instance?.identify(identify);
  }

Maybe I am missing something about how the imports/exports should work after setting the module field in the package.json, but I just wanted to bring this up.

CC: @seanparmelee @qingzhuozhen

@seanparmelee seanparmelee deleted the esm branch January 25, 2022 20:27
@seanparmelee
Copy link
Contributor Author

Sorry about that @taytestokes. I'll defer to one of the Amplitude devs on whether it's expected that Identify isn't exported as a named export, but looking at the docs, it seems like the expectation is to use new amplitude.Identify()

https://developers.amplitude.com/docs/javascript#setting-a-user-property
https://amplitude.github.io/Amplitude-JavaScript/Identify

@taytestokes
Copy link

taytestokes commented Jan 25, 2022

No worries @seanparmelee! Thanks for the quick reply. :) From looking at the code briefly, It doesn't seem like it should be a named export so maybe I missed something there. As well as looking through the docs earlier it did seem like the expectation was to use new amplitude.Identify() so I wasn't entirely sure where the named import pattern came from and how it was working, but I just thought I'd share my findings!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants