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

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.

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".

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

@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.

@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.

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..?

@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 :-)

@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.

Wonderful, thanks for clarifying. My concerns are addressed.

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?

@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.

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.

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

(spoiler, it breaks :P)

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.

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

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 :-)

Ah right, that works yes.

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.

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

Landed in 357a992.