Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsidering this proposal #6

Closed
guybedford opened this issue Jun 5, 2018 · 5 comments
Closed

Reconsidering this proposal #6

guybedford opened this issue Jun 5, 2018 · 5 comments

Comments

@guybedford
Copy link
Contributor

I really like the simplicity of this proposal... it could be really worthwhile to flesh it out again and work through some of the cases given current developments. For example how it might interplay with ".mjs" which looks like it may well be here to stay at this point.

Here's my best shot at this -

  • require.import is now provided by the dynamic import, so is effectively already implemented.
  • An ".mjs" package.json "main" or index.mjs would likely still need to be supported in lookups (at least to fit in with the current Node implementation). In addition "node x.mjs" would likely need to automatically enable the "--module" flag. Other than that, ".mjs" holds no further meaning.

I must say I do still have concerns over the idea that there is no "main" field for ES modules, and they must use a "default.js". Perhaps a "default" in the package.json could be considered here? I do really like that this is entirely optional though, that seems a very compelling feature of this proposal.

I don't see an issue with subpath requires being unavailable for ESM -> CJS boundaries, but perhaps the "looseness" of the definition of whether a given file is ESM or CJS is my greatest concern, which is avoided by the "mode" proposal (nodejs/node#18392).

The more I look at this, the more it seems like it makes the right tradeoffs overall to me though... it would be very interesting to prototype the approach and see how it feels in Node.

@zenparsing
Copy link
Owner

Thanks for taking another look at this! As I said, it's been quite a while since I wrote this stuff down and I need a bit more time to ponder it. That said:

require.import is now provided by the dynamic import, so is effectively already implemented.

Agreed.

An ".mjs" package.json "main" or index.mjs would likely still need to be supported in lookups (at least to fit in with the current Node implementation).

Do you mean that we would need a way to auto-detect ESM for (package.json).main targets, given that "main" is so heavily used in the current ecosystem? Perhaps. I always disliked the idea that the module resolution algorithm has to dig into package.json files at all. It seems like a poor separation of concerns.

But, if the ability to specify a folder entry-point within package.json is a requirement, another option (if we want to avoid relying on file extensions) might be to introduce a new key: "default". Hopefully that key isn't already used for something else. 🙄

In addition "node x.mjs" would likely need to automatically enable the "--module" flag.

I think the requirements for node <file> are a little more forgiving than for the "importing across package boundaries" case. For instance, it might be acceptable in this one case do some some parsing to autodetect. I haven't thought through the details though.

A question: does top-level await interfere with this solution? Does it essentially kill any solution that would allow a CJS module to require an ESM package? If we want to support the ability to require an ESM package, such that the package can be upgraded to ESM without forcing a change on those depending on it, don't we need the ability to synchronously evaluate modules?

@zenparsing
Copy link
Owner

Ah, I see that I wrote this:

There is no change to the behavior of require. It cannot be used to import ES6 modules.

I'm still catching up 😉

@guybedford
Copy link
Contributor Author

Do you mean that we would need a way to auto-detect ESM for (package.json).main targets, given that "main" is so heavily used in the current ecosystem? Perhaps. I always disliked the idea that the module resolution algorithm has to dig into package.json files at all. It seems like a poor separation of concerns.

You may well be right that .mjs can be ignored for these cases as "main" shouldn't refer to a module, and index shouldn't ever either.

But, if the ability to specify a folder entry-point within package.json is a requirement, another option (if we want to avoid relying on file extensions) might be to introduce a new key: "default". Hopefully that key isn't already used for something else. 🙄

We may well need something like this. Getting agreement on skipping package.json for resolution could be nice, but seems like it shouldn't be assumed. Perhaps such a feature can be separated out into a named extra which can easily be separated from the proposal. Checking the "default" key would be worthwhile too.... Daniel was using some large query system to check all code on github at the last meeting... perhaps something like that can be done for this.

I think the requirements for node are a little more forgiving than for the "importing across package boundaries" case. For instance, it might be acceptable in this one case do some some parsing to autodetect. I haven't thought through the details though.

In general parsing approaches are a difficult discussion as the "unambiguous grammar" discussions got caught up on the inconsistencies of sources without import or export being ambiguous. But it would definitely be a nice feature to add as an extra, and maybe even get it in later on. Initially ".mjs" detection should be fine though.

In terms of next steps, I could likely get an implementation of this in Node going one weekend if you're interested in hacking on it further. Then the text should probably be freshened up if we want to move it forward.

@guybedford
Copy link
Contributor Author

Just to note, I'm currently working on an an experimental implementation of this in Node on a custom branch, will share soon.

@guybedford
Copy link
Contributor Author

guybedford commented Jun 14, 2018

Ok, I've got this running at https://github.com/guybedford/node/tree/module-default.

Usage:

node --experimental-modules --module x.js

Also -m can be used instead of --module for short - node -m x.js being the intended workflow when the flag is no longer experimental.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants