module: conditional exports condition renaming proposal for usability

[guybedford]

A major priority for the modules implementation is resolver stability, and the dual mode story through conditional exports is a big remaining piece of this.

Common usability feedback out of various discussions on conditional exports so far has been that the "default" field may be seen to be a confusing name, and that it isn't clear when the "require" condition will match either.

To try and improve the overall usability this PR makes the following condition name changes:

• Add a new "import" condition as the converse of the "require" condition, only applying for the ESM loader.

All conditions (except for "default") remain behind the --experimental-conditional-exports flag.

This makes the dual mode workflow look like:

{
  "type": "module",
  "main": "./index.cjs",
  "exports": {
    "require": "./index.cjs",
    "import": "./index.js"
  }
}

instead of the previous:

{
  "type": "module",
  "main": "./index.cjs",
  "exports": {
    "require": "./index.cjs",
    "default": "./index.js"
  }
}

the UX improvement being that the former seems like it will look more natural to most users unfamiliar with "exports".

Modules group discussion in nodejs/modules#452, which this PR can be considered blocked on for now.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[ljharb]

I mentioned this on the modules repo thread but it seems to have been ignored, so I'm posting this here to be sure we make decisions with eyes open. (I'm happy to discuss this in a different medium if this isn't the best venue, but i think it's important to surface it here)

---

Removing the "default" field will break existing packages' non-main exports when they exist, and their main exports when they used the object form without array fallback notation. A specific example (i have multiple; eg i've created 4 new packages in the last week, all with "exports", to add to existing highly used dep graphs like deep-equal and resolve and tape): https://npmcdn.com/es-get-iterator@1.0.1/package.json

Assuming 13.4 is what includes this PR: 1. node >= v13.4: `require('es-get-iterator/package')` and `require('es-get-iterator/package.json')` break; the 13.0/13.1 string fallback continues to work for the "main" 1. node v13.2.x - v13.3.x: unchanged ofc; `require('es-get-iterator')` and `require('es-get-iterator/package')` and `require('es-get-iterator/package.json')` all continue to work. 1. node v13.0.x - v13.1.x: unchanged ofc; these break on the object syntax, and will fall back to the "main" entry point. They are already unable to `require('es-get-iterator/package')` and `require('es-get-iterator/package.json')` due to bugs in "exports". 1. node v12.x, assuming no backport: unchanged: these can require any file inside the package. 1. node v12.x, assuming a backport: `require('es-get-iterator/package')` and `require('es-get-iterator/package.json')` are broken, and what worked in an LTS node stops working.

The first item can be technically considered acceptable for an experimental feature in node 13, altho it's highly unfortunate and inconvenient.

The last item, however, strikes me as something that an LTS policy doesn't/shouldn't allow (correct me if i'm wrong on the former). Thus, removing "default" entirely seems like it would prevent backporting to v12 (without restoring it somehow for the backport).

Is there any way we could keep "default" for now, and remove it as part of v14, rather than breaking (arguably, capriciously) consumers of these packages?

(Note that I can and certainly will publish a patch for affected packages that adds "require" alongside "default"; this is a solution for anyone who isn't expecting to be forced to update their lockfile on a non-major upgrade of node, which seems like "nobody")

[GeoffreyBooth]

Package exports are experimental, and "default" has only been in the wild since 2019-11-21. If we’re going to remove it, better to do so as soon as possible before usage increases any further.

Anyone who uses experimental features in production assumes the risks of breaking changes. For the sake of downstream users, it would be advisable not to use experimental features in stable versions of public packages.

[ljharb]

It's experimental in node 13; it's "nothing" in node 12. The primary risk I'm talking about is future backport risk for v12. (also, everything published is stable; semver communicates breakage not stability - if we want feedback, these features have to be used in published stable packages).

My understanding of the deferring of conditional exports was about the other keys, not about changing "default".

[guybedford]

It would be great to get this one landed... @jkrems @MylesBorins @nodejs/modules-active-members, please review if you can.

[ljharb]

@guybedford we haven't discussed it in the modules meeting yet, and it seems like my very real concerns aren't being considered or addressed. Please do not rename "default" casually.

[guybedford]

@ljharb this PR doesn't remove or rename "default", rather it supports "import" alongside "default" to ensure that users can provide a more readable "import" / "require" pattern. Any discussion over deprecation of "default" would be separate and is out of scope for this. I'm happy either way on that question personally.

[jkrems]

LGTM in general, two questions.

[jkrems]

Any discussion over deprecation of "default" would be separate and is out of scope for this. I'm happy either way on that question personally.

I think it's fair to call this a "doc only deprecation of the default condition" though..?

[ljharb]

@guybedford thank you very much for clarifying; that means "default" won't break, which is great. Will "import" and/or "require" take precedence over "default" if all three are specified?

@jkrems doc-only is something i'm 100% fine with, ftr :-)

[guybedford]

@ljharb yes "import" and "require" both have a higher precedence than default. And "node" has a higher precedence than both of those as well. "default" is always the lowest precedence of all future conditions too.

[ljharb]

Wonderful, thanks for clarifying. My concerns are addressed.

[MylesBorins]

I left a note about including default in the resolution docs. I'm personally not a fan of deprecating that entry and I think it should be officially documented.

[guybedford]

So one problem we have with this PR landing - is that currently we can recommend the "default" pattern to users without them using the --experimental-conditional-exports flag, whereas this pattern will not work at all unless the flag is set.

For this reason I think we should revert the documentation on this PR to still use the "require" / "default" pattern, and then instead we can change the documentation to recommend the "import" / "require" pattern when we unflag conditional exports.

The next risk as well with this pattern is that it will break on the current 13 versions that there is no valid "exports" resolution.

Perhaps that does mean we are too late to make this change after all?

[ljharb]

@guybedford it seems like packages could use this pattern to work on every minor version of node i'm aware of:

      "exports": {
		".": [
			{
				"import": "./main.mjs",
				"require": "./main.js"
			},
			{
				"default": "./main.js"
			},
			"./main.js"
		]
	},

(perhaps the two objects could be combined; i'd have to test it across node versions once the conditional changes are available)

although I like the idea of the docs recommending the "unflagged" pattern instead of a flagged one, I think that this change - given node 13's "experimental" status, and given that a backport to node 12 would hopefully include unflagged conditional exports - seems like a reasonable back compat story to me, which means we could continue with the renaming.

[guybedford]

Thanks @ljharb for the thoughts on this.

Admittedly the dual mode pattern as we are advocating does break when using CJS require itself though in 13 currently (cannot require module at "default"), so it's not as if we're recommending a pattern that doesn't already cause breaking workflows for users since users shipping dual mode support like the docs mention will already be breaking all their users using require() on Node.js 13.x.

Perhaps we should just clarify these patterns are likely to break until they are unflagged?

My worry with this is if these patterns break now, how can we ever encourage people use them if they simply can't work.

[guybedford]

On that note, @ljharb if you are shipping "exports" have you tested require() in Node.js 13?

[guybedford]

(spoiler, it breaks :P)

[ljharb]

I’m shipping “exports” with a dot set to default with a string fallback, and testing it in every node minor since 0.6 (es-get-iterator, via its dependents) and afaict it works in every one.

[guybedford]

Specifically though, did you test require('pkg') in Node.js 13.1 13.2?

[ljharb]

Yes, and it works if my tests are to be believed. Note that I’m talking about CJS only; i won’t ship ESM anywhere until conditionals are unflagged :-)

[guybedford]

Ah right, that works yes.

[guybedford]

Thanks for working this through with me here.... just helps to have some feedback on the compat implications.

My main concern is just that we have enough guidance here to note that setting a "require" or "import" condition in the package.json will result in packages that will be highly unstable and likely to break. I know we have a note on this, but I don't think we are stressing it enough.

I've added another warning to the docs, and kept the overall workflows the same for this PR since it's breaks either way.

Further suggestions welcome, otherwise I think this is good to land.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/27547/

[guybedford]

Landed in 357a992.