Support es modules #18
Conversation
|
hmm, even though its not that much code but still doesn't seem ideal to maintain two copies of the source code that need to be manually synced. Can't we write the source code once and transpile accordingly with rollup as we are doing with the other packages? |
|
@djskinner yes we can! refactored. |
| "./lib/fingerprint": { | ||
| "browser": "./lib/fingerprint.browser.js", | ||
| "node": "./lib/fingerprint.js" | ||
| } |
There was a problem hiding this comment.
is this export being used by other packages?
There was a problem hiding this comment.
I doubt it to be honest, but I was leaving it in for the sake of backwards compatibility
| "license": "MIT", | ||
| "devDependencies": { | ||
| "browserify": "17.0.0", | ||
| "@rollup/plugin-alias": "^5.1.1", |
There was a problem hiding this comment.
this handles the #fingerprint import flipping between node and browser versions
index.js
Outdated
| discreteValues << 0) | ||
| .toString(base), blockSize); | ||
| } | ||
| export { fingerprint, isCuid }; |
There was a problem hiding this comment.
is rollup not complaining about mixing default and named exports for the cjs output? I thought it would. Just wondering if we should do this for the esm output only because this isn't well defined for cjs:
const cuid = require('@bugsnag/cuid')
const { isCuid } = cuid
Which isCuid is this referring to, the one on export default cuid, or the one from export { isCuid }?
gingerbenw
force-pushed
the
PLAT-13269/modulify
branch
from
aa44481 to
5bd7b9a
Compare
Add es modules and appropriate exports so package can be used in project which require esm syntax.
Uses rollup v2 to maintain node12 support, but could also separate out the build and test step to use a newer version