Native ESM via published .mjs

[jaydenseric]

This PR adds support for native ESM for Node.js in --experimental-modules mode by shipping sibling .mjs ESM files next to the existing .js CJS files, and by removing the file extension from the package.json main field.

This allows people use simultaneously use Babel and native ESM to consume this package without showstopping default interop errors (see https://github.com/babel/babel/issues/7294 and https://github.com/babel/babel/issues/7998).

While I was at it:

• Fixed faux ESM imports. As a rule for .mjs files, if the file being imported does not have the .mjs extension and contain ESM, do not import from it using ESM named imports.
• Replaced yarn for npm run where relevant. This is best practice; Yarn understands the npm scripts, but npm does not understand Yarn. Now both Yarn and npm can be used to run the package scripts.
• Simplified scripts to not have to use bin paths everywhere.
• Fixed a few minor .mjs related tool misconfigurations.

For reference, here is how I approach package scripts and native ESM support in my packages:

https://github.com/jaydenseric/extract-files/blob/v5.0.1/package.json#L60

[codecov[bot]]

Codecov Report

Merging #3 into master will decrease coverage by 0.03%. The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master       #3      +/-   ##
==========================================
- Coverage   97.05%   97.02%   -0.04%     
==========================================
  Files          12       12              
  Lines         170      168       -2     
  Branches       25       25              
==========================================
- Hits          165      163       -2     
  Misses          1        1              
  Partials        4        4

Impacted Files	Coverage Δ
src/lib/util/isBuffer.mjs	100% <ø> (ø)	:arrow_up:
src/lib/util/isReadable.mjs	100% <100%> (ø)	:arrow_up:
src/lib/FormData.mjs	95.61% <80%> (ø)	:arrow_up:

---

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 8844457...47e236e. Read the comment docs.

[codecov[bot]]

Codecov Report

Merging #3 into master will decrease coverage by 0.03%. The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master       #3      +/-   ##
==========================================
- Coverage   97.05%   97.02%   -0.04%     
==========================================
  Files          12       12              
  Lines         170      168       -2     
  Branches       25       25              
==========================================
- Hits          165      163       -2     
  Misses          1        1              
  Partials        4        4

Impacted Files	Coverage Δ
src/lib/util/isBuffer.mjs	100% <ø> (ø)	:arrow_up:
src/lib/util/isReadable.mjs	100% <100%> (ø)	:arrow_up:
src/lib/FormData.mjs	95.61% <80%> (ø)	:arrow_up:

---

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 8844457...be0c8ab. Read the comment docs.

[jaydenseric]

Note that there is a faux ESM named import here, that can't be fixed because there is a mix of named and default exports:

https://github.com/octet-stream/form-data/blob/master/src/test/__helper__/server.mjs#L3

Generally APIs are better as either all named exports, or only a default export, and not a mix.

It is only in a test file and AVA is forgiving, so it doesn't compromise native ESM for the published lib.

[octet-stream]

Thanks for the PR. I will take a look. But first of all I want to ask: why did you replaced Yarn with NPM? I don't get this point.

[jaydenseric]

why did you replaced Yarn with NPM? I don't get this point.

Because when I created the make, make:mjs and make:js scripts I didn't want to perpetuate the bad practice of using yarn instead of npm run. Like I said, you can still use Yarn; it knows to take over the npm run commands.

Environments that don't have Yarn (the majority) will error when attempting to run package scripts using yarn. Your package is not doing it, but there should be a prepare script that does the build. That way users can install forks/PRs of the package as a git dependency, and their npm install command will run prepare after the package is cloned from git. Most users do not have Yarn, so this process would fail if you don't use npm run in your scripts.

[jaydenseric]

Compare the result of installing the current master branch from git:

{
  "dependencies": {
    "formdata-node": "github:octet-stream/form-data"
  }
}

Vs. this PR, now that I've added a prepare script:

{
  "dependencies": {
    "formdata-node": "github:jaydenseric/form-data#native-esm"
  }
}

[octet-stream]

Now that's sounds reasonable. I will merge it and publish as soon as I can. Sorry for the wait.

[octet-stream]

Thanks for your help :)

[octet-stream]

Published to NPM as 1.4.0

I have one more question about your PR: Why don't you use named imports for native Node.js modules (fs, stream and path). I mean use this:

import {createReadStream} from "fs"

// ...

instead of

import fs from "fs"

// ...

It seems to work fine, so I don't see any particular reason to import the whole object to the module namespace when I need only few methods from each of them.

[jaydenseric]

Why don't you use named imports for native Node.js modules (fs, stream and path).

Because your package declares support for Node.js v8 in package.json engines, and earlier versions of --experimental-modules mode did not provide ESM named exports for the built in modules. I can't remember off the top of my head what Node.js version introduced them, maybe in v10 or 11? The default export works in all versions.

Try, in test.mjs:

import { createReadStream } from 'fs'

And with Node.js v8.5, run:

node --experimental-modules test.mjs

And it will error.

When you decide to only support newer versions of Node.js you can switch to named imports.