chore: switch to esm

[vasco-santos]

BREAKING CHANGE: built content includes ESM and CJS

Needs:

• feat: build esm modules using ipjs ipfs/aegir#865
• bundle action back in place

[vasco-santos]

This should be ready, just needs a new aegir. It would be helpful to have a review @achingbrain

[achingbrain]

I think all the files need to switch over to using named exports, otherwise this innocuous looking sort of thing:

/**
 * @typedef {import('foo').Bar} Bar
 */

module.exports = function baz () {}

Generates:

export type Bar = import('foo').Bar;
export default function baz(): void;

and ipjs says to not mix default and named exports.

[achingbrain]

I think all the files need to switch over to using named exports

Actually maybe not, we can just remove the @typedef and import the type directly in the function's @param annotation, this solves that problem.

Still getting some weird typeof import('path/to/uint8arrays/types/src/from-string') has no call signatures errors though.

[achingbrain]

No, we have to switch to named exports.

has no call signatures - this is caused by ts wanting:

const fromString = require('uint8arrays/from-string')

to be:

const fromString = require('uint8arrays/from-string').default

which compiles types but then fails at runtime in node due to "require" in the exports map resolving to the .cjs file which doesn't have a .default property:

"exports": {
  "browser": "./esm/src/from-string.js",
  "require": "./cjs/src/from-string.js",
  "import": "./esm/src/from-string.js"
}

Ignoring that for a second, if we run bundled cjs code in the browser, "exports" means the browser gets supplied the esm version, which does have a .default property so breaks - fixing this would require a PR to ipjs to change this line to supply the cjs version instead or at least make it an option. Do we want that? We want to live in the bundler-free future so probably not.

Ingoring that for a second, if we follow my suggestion above:

we can just remove the @typedef and import the type directly in the function's @param annotation

we're breaking compatibility because we previously exported a SupportedEncodings type from to-string and from-string which would be removed, so we'd need a major.

We could ditch cjs, which would require all cjs users to change all instances of require('uint8arrays/from-string') to require('uint8arrays/from-string').default so we'd need a major.

Instead we can switch to named exports, which will need a major, but then people don't need to use .default and we can retain the SupportedEncodings type.

As ever, this springs to mind:

[vasco-santos]

Thanks for the detailed feedback. I agree that the best solution is to make named exports and I will change it

[vasco-santos]

types tested on ipfs/js-ipfs#3774

[codecov-commenter]

Codecov Report

Merging #22 (fa58e00) into master (c7a0601) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
- Coverage   98.35%   98.15%   -0.20%     
==========================================
  Files           7        7              
  Lines         243      217      -26     
  Branches       44       44              
==========================================
- Hits          239      213      -26     
  Misses          4        4              

Impacted Files	Coverage Δ
src/compare.js	100.00% <100.00%> (ø)
src/concat.js	100.00% <100.00%> (ø)
src/equals.js	91.30% <100.00%> (ø)
src/from-string.js	100.00% <100.00%> (ø)
src/to-string.js	100.00% <100.00%> (ø)
src/util/bases.js	100.00% <100.00%> (ø)
src/xor.js	89.47% <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 c7a0601...fa58e00. Read the comment docs.

[vasco-santos]

this needs to be added again once aegir ships it as I could not make it use a branch

[achingbrain]

I've restored this but now CI is failing on this step with:

Error: Cannot find module '/github/workspace/index.js'

[vasco-santos]

oh damn, I will look into it

[achingbrain]

Why do we need to double up these exports? Could be quite error prone over time, maybe we could get ipjs to do this for us?

[vasco-santos]

We talked about this offline a couple of weeks. Anyway, we do not need this anymore. Removed now