-
Notifications
You must be signed in to change notification settings - Fork 43
Conditional exports naming usability discussion #452
Comments
Removing "default" will break the current implementation in node 13 for the packages I've published with "exports" - I hope we don't do that. |
I like this direction. Furthermore, We should remove |
That we can technically get away with it doesn't mean that downstream users won't suffer as a result; this isn't imposing a cost on module authors, it's imposing a cost on their consumers. Could we add and prefer "module" and "commonjs", but still support "default" until the next major? |
We also need to add to the docs some information about how conditions are recursive. For example: {
"type": "module",
"main": "./index.cjs",
"exports": {
"node": {
"commonjs": "./index.cjs"
},
"module": "./index.js"
}
} This formulation should cause all Node consumers to load the CommonJS |
Let's please phrase this as: Remove the default condition and add a module condition. The default condition was sugar for a clean array fallback. If there's any restrictions on "module", it's fundamentally a different condition (which isn't bad, just something I think should be made explicit). So the correct way to rewrite this: {
"type": "module",
"main": "./index.cjs",
"exports": {
"require": "./index.cjs",
"default": "./index.js"
}
} Would be to use the non-default-using: {
"type": "module",
"main": "./index.cjs",
"exports": [{
"require": "./index.cjs",
}, "./index.js"]
} There's other uses of default (e.g. "node" or "browser" vs. "default") where replacing default with module doesn't really make sense: {
"type": "module",
"main": "./use-inline-crypto.cjs",
"exports": {
"node": "./use-node-crypto.cjs",
"browser": "./use-web-crypto.cjs",
"default": "./use-inline-crypto.cjs"
}
} There's no ES modules involved, so replacing "default" with "module" here would just break the package. But the following works perfectly fine: {
"type": "module",
"main": "./use-inline-crypto.cjs",
"exports": [{
"node": "./use-node-crypto.cjs",
"browser": "./use-web-crypto.cjs",
}, "./use-inline-crypto.cjs"]
} I'm a fan of the array fallbacks, so I can live with getting rid of "default". And I agree that adding a "module" condition makes sense, especially since we shipped without a "require" guard in 13 which limits what packages can do. |
This proposed scheme is confusing to me as |
The confusing thing about the whole distinction is that we'll realistically not make this about the format. It will always be about the supported loader. E.g. |
I'd agree with @jkrems here, it gets more complicated if we use the term |
Well that was what I liked about So if the conditions describe the target files, and I remember the array syntax this time, my example above could be better written as: {
"type": "module",
"main": "./index.cjs",
"exports": [{
"commonjs": "./index.cjs"
}, {
"module": "./index.js"
}]
} In this case, both Node loaders will load If the conditions instead describe the loader/method of importation, then I would call them "exports": {
"node": {
"require": "./index.cjs",
"import": "./index.cjs"
},
"import": "./index.js"
} This feels counterintuitive to me, like it’s configuration for Node rather than metadata describing the package. Wouldn’t this also potentially introduce issues if the capabilities of loaders change over time? |
Would “modern” and “legacy” work better?
Basically the two names here refer to the new loader and the old loader and
that’s it. The selection has nothing to do with the module format really.
This change is about usability though, and since the loaders are _the
commonjs loader_ and _the esm loader_, loosening the name meanings in the
name of usability for Node.js users was the goal.
Semantically, “default” and “import” and “require” are the perfectly
correct names. The problem is usability - does this work for our users.
…On Thu, Dec 5, 2019 at 12:53 Geoffrey Booth ***@***.***> wrote:
I'd much prefer for commonjs / module within exports to indicate format
rather than supported loader.
The confusing thing about the whole distinction is that we'll
realistically not make this about the format. It will always be about the
supported loader.
Well that was what I liked about commonjs / module: that (I thought) it
described the files in the package, not the loader to use. In general a
package.json feels like it should be metadata about the package. Hence
commonjs makes sense to me as “this is the CommonJS file for this path.”
Even stuff like browser makes sense as I read it as “this is the
browser-environment file for this path.”
So if the conditions describe the target files, and I remember the array
syntax this time, my example above
<#452 (comment)>
could be better written as:
{
"type": "module",
"main": "./index.cjs",
"exports": [{
"commonjs": "./index.cjs"
}, {
"module": "./index.js"
}]
}
In this case, both Node loaders will load index.cjs, as both support
"commonjs"-type files and that’s defined first in the array, making it
top priority.
If the conditions instead describe the loader/method of importation, then
I would call them require and import to make that connection clear. But
then in order to achieve the same desired result (both Node loaders get
CommonJS, other runtimes get ESM) you’d have to write as:
"exports": {
"node": {
"require": "./index.cjs",
"import": "./index.cjs"
},
"import": "./index.js"
}
This feels counterintuitive to me, like it’s configuration for Node rather
than metadata describing the package. Wouldn’t this also potentially
introduce issues if the capabilities of loaders change over time?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#452?email_source=notifications&email_token=AAESFSWNDTNLDXB53V42UYLQXE5ZPA5CNFSM4JVTMXZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGBRXAY#issuecomment-562240387>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAESFSX4TUWUTET2VMYIZW3QXE5ZPANCNFSM4JVTMXZA>
.
|
I do not believe this would cause issues, but loaders could potentially read/provide flags when delegating. |
I prefer |
How about I realize I'm coming at this from the perspective of someone who ships mostly client-side code these days, but I'm personally planning on shipping UMD bundles for the libraries I maintain instead of strict CommonJS. People who use modules get ESM, nomodule (UMD) for everyone else. Also, this may be out of scope but one thing that is sorely missing but is already being implemented in bundlers like webpack is the idea of dev vs. prod builds. Would love, love, love to see some discussion around this. |
I'm probably missing something here - what's the reason this isn't sufficient to achieve dual-mode? {
"type": "module",
"main": "./index.cjs",
"exports": {
"module": "./index.js"
}
} Originally, based on discussions with various Node folks, my assumption was that the following would work for shipping ES5+CJS and EScurrent+MJS: {
"main": "index.cjs",
"exports": {
".": "index.js"
}
} |
It seems there is some confusion. The flags currently are specifying data for the system loading a package, the flags are not set due to the right hand side of the export mapping (the dependency) nor are the flags set due to the format/context of call site of the load operation (the dependent). |
@developit exports shadows main. It also isn't an issue of only 1 / 2 loaders, we need to be able to support many types of entries. So an idea... why don't we support this allows for a variety of entry points and resolution with fall back, does not break existing shipped implementation (default). I'm not 100% we should be supporting the recursive syntax, I don't think we do currently but could be mistaken. @GeoffreyBooth are you passionate about that syntax? It seems like it could significantly complicate writing maps as well as parsing them. |
@guybedford I'm not in favor of "legacy"; CJS is not legacy, it's just one of two module systems. |
It's clear that the exports object should describe two things:
Node has two builtin loaders, namely "CJS" and "ESM" as can be seen in lib/internal/modules. I would explicitly use their internal names as keys, rather than "legacy", "current", etc. {
"exports": {
"node": {
"cjs": "./index.cjs",
"esm": "./index.mjs"
},
"browser": {
}
}
} The rationale is that there may be a possible future where the builtin ESM loader is superseded by another loader (e.g. "ESM2", etc.). This would result in multiple "legacy" loaders existing. |
@DerekNonGeneric I disagree and I think expanding this in exports significantly complicates things. I don't think we need to specify the difference between the environment and the loader here. I think we can fairly succinctly cover almost tall cases that node requires with the 4 keys I suggested above. Other environments are able to then extend with whatever keys they want. I would very much like us to avoid the recursive case |
There’s some confusion here. There are really two decisions at play here:
I think there’s probably consensus that if the condition names represent the format of the files, then the names However if the condition names represent names of loaders, then And currently, the condition names do represent the names of loaders. That’s probably why I think it’s safe to say that there’s a lot of confusion about this point. I certainly was confused; I thought that we weren’t just renaming things but also making the conditions be about the files rather than about the loaders (as in the conditions are metadata, not configuration). So before we get into what the best condition names are, can the conditions be describing the formats of the files rather than configuring each loader what it should load? That seems to be the preference of several of the folks on this thread, or at least it’s the assumption many people seem to be making about how this works. |
I do not believe so. Code like the following seem to be impossible to reconcile with that idea: // foo.cjs
require('bar'); // sets the "require" condition
import('bar'); // does not set the "require" condition |
I disagree that this is a problem, I think it's a major justification of conditional exports. Consider if |
@coreyfarrell that is allowed, I don't understand the disagreement. The problem with interpreting the conditions as formats is multiple.
Neither of these relate to the dependent or dependency formats. I do not believe that the conditions can actually be tied to the format of a dependency nor of the dependent given our current designs and doing so seems impossible since things like |
Here my 2 cents/opinions: Like: 2 opposite conditions for Dislike: Dislike: Having a list of condition ordered and match in this fixed order: node, commonjs, module, default Like: Like: Idea: Dislike: versions like |
No, I am not OK with renaming "default", it will break packages out in the wild on node v13, and it will prevent backporting conditional exports to v12 for the same reason. |
I'm just here to say that i like "default" for the simple reason that... {
"exports": {
"require": "./index.cjs",
"default": "./index.js"
}
} "require" and "default" have the same number of characters. |
I think the current PR is adding |
Disagree. People have been doing this for years with {
"type": "module",
"main": "foo.js", // this is ESM
"exports": {
".": "bar.js" // this is also ESM
}
} {
"main": "foo.js", // this is CommonJS
"exports": {
".": "bar.js" // this is also CommonJS?
}
} Only way to make a package that supports both is like this? {
"main": "foo.js", // this is CommonJS
"exports": {
".": "bar.mjs" // this is ESM
}
} This is pretty confusing IMO. Having |
@devongovett Right, but your alternative means that if you If you don't want to change file extensions, you can always create a minimal |
I see. This needs to be very well documented then. I've already seen several examples similar to the ones above, assuming that anything inside |
Wait, couldn't that always be ESM since you're importing from ESM? You cannot |
In node, you can import a CJS module. The |
if someone wouldn't mind clarifying or pointing me at the right doc -- if a node import map mixes CJS with ESM, what happens if i do |
I'm not 100% sure if I understand the question correctly but from what you describe: We explicitly do not support " If the |
Though the |
thank you both for the clarifications. the situation is much clearer to me now 😄 |
nodejs/node#30799 has landed, so now we have
|
I'm not sure I follow this argument. How is it redundant with "exports": {
"browser": "./browser.cjs",
"default": "./node.cjs" // this would break with the `import` condition
} EDIT: Replace |
It’s redundant because any runtime that supports or will ever support So if it doesn’t provide any independent functionality, its only value is as a shorthand or alternate syntax, or for “completeness” if we feel like we need a condition to represent the “no condition” case. The question is whether these benefits are worth the cost of added API (one more thing to maintain, one more thing to learn, one more thing developers can use incorrectly, etc.). |
… weighed against the cost of breakage in the wild if it's removed, and ESM is ever backported to an LTS node like v12. |
So you mean that "exports": {
"browser": "./browser.cjs",
"import": "./generic.cjs",
"require": "./generic.cjs"
} That seems a bit... ugly? At that point, I would hope we'd suggest the much less verbose variant which is also much less likely to be used incorrectly by accident (by forgetting a field etc): "exports": [{
"browser": "./browser.cjs"
}, "./generic.cjs"] So yes - |
@jkrems the last example you gave gets reformatted by any tool which modifies package.json. The result becomes: "exports": [
{
"browser": "./browser.cjs"
},
"./generic.cjs"
] This is an example if why I would use |
Let’s consider what these terms mean. As I understand them:
Putting a CommonJS file in Why not CommonJS in |
What about... JSON? What about any future file format that may be usable from
It reads like "if it's an apple, do X. if it's affected by gravity, do Y". It's a distracting level of detail that has nothing to do with the intent - which is "for everything that's not an apple".
I don't believe that's true. Bundlers have been reading a "browser" field with browser CJS code for a long time. And it's possible to run CJS in browsers (not efficiently or in production), just not with the built-in ESM loader. But that aside - there will always be packages that point something like To me it's the same principle as shipping a version of your website to user-agents you don't recognize. Yes, they may fail to understand the page. But it's not on the server to prevent them from trying. So yes, if all you have is CJS, I do believe that you set |
Closing as resolved. |
@guybedford do we remove it from minutes in #461 or keep for update(s)? |
Further to discussions from today re resolver stability and trying to incorporate the current feedback on conditional exports I've posted the PR at nodejs/node#30799 to open discussion around conditional exports naming and behaviours.
From the PR description -
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:
"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:
instead of the previous:
the UX improvement being that the former seems like it will look more natural to most users unfamiliar with "exports".
The text was updated successfully, but these errors were encountered: