fix(exports): node 13.0-13.6 require a string fallback#6
fix(exports): node 13.0-13.6 require a string fallback#6lukeed merged 1 commit intolukeed:masterfrom ljharb:patch-1
Conversation
package.json’s "engines" field claims cliui supports node >= 6; node v13.0-v13.6 are included in this semver range. This change is required to be able to require() from escalade successfully in these versions. See yargs/yargs#1776
Codecov Report
@@ Coverage Diff @@
## master #6 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 20 20
=========================================
Hits 20 20 Continue to review full report at Codecov.
|
lukeed
left a comment
There was a problem hiding this comment.
Hey,
I checked out the imports locally. TIL!
However, why is it pointing to the CommonJS files? In Node 13.0-13.6, export is supported.
I know this means require wouldn't work, but still. Sending the CommonJS file would make something like a kleur/colors module (with named exports only) fail; for example:
// Node 13.4, with this PR's approach
import * as colors from 'kleur/colors';
//=> { default: { red, blue, ... } }
import { red } from 'kleur/colors';
// SyntaxError: The requested module 'kleur/colors' does not provide an export named 'red'
// Node 14.x
import * as colors from 'kleur/colors';
//=> { red, blue, ... }
import { red } from 'kleur/colors';
// OKSo it'd skip the immediate module-loader error, but it either requires an interop-check in all places or fails silently because the exports don't align.
|
You can't require ESM files, and the bug I'm trying to fix is being able to require yargs (which requires escalade) in node v13.0-v13.6. It's impossible to support both CJS and ESM in those versions, and since CJS can be both required and imported, that's the only approach that makes sense imo. |
|
In other words, I think CJS is a far, far more important consumer than ESM, because most node users aren't using ESM yet - and because for escalade to be consumed as ESM, everything that consumes it must also consume with ESM. Being ESM-only is viral - being CJS-only is not. |
|
Gotcha :) While certainly not ideal, I agree it's the safer/wider support range. Anyway, thanks! |
|
Published |
|
Thank you! |
package.json’s "engines" field claims cliui supports node >= 6; node v13.0-v13.6 are included in this semver range. This change is required to be able to require() from escalade successfully in these versions.
See yargs/yargs#1776
I've manually tested this; it fixes the bug. Since the behavior can only be observed by installing escalade into a node_modules folder and then trying to
requireit, it's exceedingly difficult to test in CI.