Modules lkgr rebase no loader #35
Conversation
Refs: nodejs/modules#180 PR-URL: #6 Reviewed-By: Myles Borins <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Refs: nodejs/modules#180 PR-URL: #6 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Refs: nodejs/modules#180 PR-URL: #6 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Refs: nodejs/modules#180 PR-URL: #6 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: #6 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
PR-URL: #6 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
| When using `--experimental-modules`, this informs the module resolution type | ||
| to interpret the top-level entry into Node.js. | ||
|
|
||
| Works with stdin, `--eval`, `--print` as well as standard execution. |
There was a problem hiding this comment.
(Not sure if in scope for this review.) It may be good to explicitly note/warn that it won't work for CLI tools via shebang.
There was a problem hiding this comment.
I think this is just a rebase? Could you open a PR like #33 to change this, or an issue on this repo to remind us to address this?
| PACKAGE_MAIN_RESOLVE(_packageURL_, _pjson_) | ||
| > 1. If _pjson_ is **null**, then | ||
| > 1. Throw a _Module Not Found_ error. | ||
| > 1. If _pjson.main_ is a String, then |
There was a problem hiding this comment.
This means that we create an entrypoint field that is closed for extension ("we can never add a new extension w/o breaking the else: CJS") unless it's paired with a 2nd package.json field ("type": "module"). This feels a bit awkward. I assume reusing main is temporary?
There was a problem hiding this comment.
Yes, it was discussed in the PR where this was landed that I was going to propose changing this, as the current implementation makes dual CommonJS/ESM packages impossible. That will require new package.json configuration, though, so it’ll take a new proposal and more review etc., so we allowed merging this in with the understanding that dual-type packages would be addressed in the future.
Edit: Added #36 to remember to address this.
There was a problem hiding this comment.
Added #38 to also cover future extensions. For me that's even more important than dual-mode packages because it's not limited to a transitional time.
|
landed in bc72470...968c8f1 |
This is the same as #34 but without 531b6d4
as discussed in nodejs/modules#259
/cc @jkrems