Modules: Managing fork and agreed upon minimal kernel

[MylesBorins]

Making this issue to track discussion and gain consensus around what a minimal kernel would look like in that fork.

[MylesBorins]

what would we want to call the fork? node-esm?

[devsnek]

how about a branch called esm, i'd like to still get eyes from collaborators who aren't node modules team members on our changes.

[MylesBorins]

So this is a good thing to discuss

Fork vs branch.

With branches on node core we have one less thing to worry about (a new
repo) but are creating lots of noise for other collaborators (feature?)

A new repo allows us a sandbox, but creates a separate place for
conversations to go (aside from the modules repo)

We could mount node as a Subtree in the modules repo and do pull requests.
This is a bit of a spaceship though.

On Tue, Aug 14, 2018, 7:37 PM Gus Caplan [e-mail] wrote:

how about a branch called esm

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/modules/issues/166#issuecomment-413049407, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV2tJAv5E3xIFYj8w24X2QjAPV1mjks5uQ182gaJpZM4V9PxZ
.

[GeoffreyBooth]

The other thing to discuss is what should go in Node core. Do we keep the current implementation there? Replace it with a minimal one that grows over time? Remove it and leave nothing, and various implementations are all relative to an ESM-less core?

[giltayar]

+1 for fork. Until we know where this is going, the less noise, the better.

On Wed, Aug 15, 2018 at 8:05 AM Geoffrey Booth [e-mail]
wrote:

The other thing to discuss is what should go in Node core. Do we keep the
current implementation there? Replace it with a minimal one that grows over
time? Remove it and leave nothing, and various implementations are all
relative to an ESM-less core?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/modules/issues/166#issuecomment-413095318, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAYnRBoi7ACb_StNumCRiGnzeZ6ufAwNks5uQ6wjgaJpZM4V9PxZ
.

[giltayar]

What should go in? Leave everything as is, as this is the easiest, and
start discussing what should be taken out. (or put in.)

On Wed, Aug 15, 2018 at 10:15 AM Gil Tayar [e-mail] wrote:

+1 for fork. Until we know where this is going, the less noise, the better.

On Wed, Aug 15, 2018 at 8:05 AM Geoffrey Booth [e-mail]
wrote:

The other thing to discuss is what should go in Node core. Do we keep the
current implementation there? Replace it with a minimal one that grows over
time? Remove it and leave nothing, and various implementations are all
relative to an ESM-less core?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/modules/issues/166#issuecomment-413095318,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAYnRBoi7ACb_StNumCRiGnzeZ6ufAwNks5uQ6wjgaJpZM4V9PxZ
.

[giltayar]

Let me suggest a minimum implementation. The idea of this minimum is to 1) have all ESM functionality included, and 2) avoid a debate on all the thorny issues.

So it includes:

• the ability to import an ESM either with import from in an ESM file, or using dynamic import from a CJS or ESM

Avoiding the "file extension debate"

• import will not guess the file extension and will import the file as ESM:
  
  
  
  • e.g. import ... from './foo.js' or import ... from './foo.mjs' will both import an ESM from the corresponding files because the extensions have no meaning. It could be ./foo.foo for all it cares! It is important to note that this will probably not be the end result, but is again, a minimal decision that will likely change, and that avoids discussing the file extension problem till we're ready.
  
  
• If the thing being imported is a folder, it will look for package.json main property and will load that as ESM. Again, will not guess the file extension.
• It will not look for index.js because that will provoke a "file extension debate", and we all want to evade that.
• The regular CJS folder searching algorithm will be used (e.g. node_modules in parent dir...), except that it will not look for index.js (or index.mjs).

Avoiding the "transparent interop debate"

• CJS will be able to dynamically import an ESM, because that is in the JS spec, and is mostly a consensus in the group (I hope...).
• ESM will not be able to require a CJS file.
  
  
  
  • If this is too harsh, we can add the import.meta.require PR to enable this, as this is easily removable later if another interop mechanism is chosen.
  
  

That's it.

[devsnek]

It will not look for index.js because that will provoke a "file extension debate", and we all want to evade that.

node looks for index.{any registered file extension} atm, will we still support that with your proposal?

[GeoffreyBooth]

I think folder lookup can be something else that gets added later. If you _really_ want to be minimal, you could also leave out package resolution, to allow for either the CommonJS require algorithm or the package name maps proposal.

[MylesBorins]

Minimal kernel imho:

• No transparent interop
• import only supports ESM
  
  
  
  • no json
  
  
  • no common
  
  
  • no native modules
  
  
  • perhaps explicit ban on .js extension for now
  
  
• No extension or folder searching (move https://github.com/nodejs/node/pull/21729 to the fork and iterate)
  
  
  
  • support for node_modules?
  
  
• createRequireFunction to allow for cjs in esm interop (move https://github.com/nodejs/node/pull/19360 to the fork and iterate)
• dynamic import to allow for esm in cjs interop

[devsnek]

i would still say that any situation where we land without node resolution enabled by default is unworkable, and so it should be part of the kernel.

[MylesBorins]

@devsnek I don't agree with you, and thus we don't have consensus. We can't remove it later and thus I think it shouldn't be in the kernel

[giltayar]

@MylesBorins - I'm assuming your minimal kernel also doesn't assume an extension, and the extension has to be explicitly in the name?

[MylesBorins]

@giltayar indeed

[targos]

@MylesBorins thanks for creating the repo. Should we start by pushing node's master branch to it?

[MylesBorins]

I'm trying to push right now. Super slow wifi 😅

On Wed, Aug 22, 2018, 12:32 PM Michaël Zasso [e-mail]
wrote:

@MylesBorins https://github.com/MylesBorins thanks for creating the
repo. Should we start by pushing node's master branch to it?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/modules/issues/166#issuecomment-415152094, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV_3G_dE92zy3teRzp9tX6wfmtv2bks5uTbHSgaJpZM4V9PxZ
.

[MylesBorins]

Fork is up and running

--> https://github.com/nodejs/ecmascript-modules

[MylesBorins]

edit: to be explicit this is not something that would ever be shipping. The purpose of this kernel is to build a subset implementation that can be used to implement all features we are currently debating to allow pull requests for specific features or full proposals to build on top of the same code base (and likely share more code)

what is a minimal implementation

• both browser + node need bare imports
  
  
  
  • implementation to be discussed
  
  
• we cannot have dynamic path searching
  
  
  
  • requiring the full path is an issue for tooling and a long term solution is required
  
  
  • migration strategies also have issue with this
  
  
• common.js backwards compat
  
  
  
  • createRequireFunction does
  
  
  • import.meta.require does not fail early enough
  
  
  • hold off on importing common.js until more progress is made on dynamic modules spec
  
  
• .mjs will be the only way to import ESM
  
  
  
  • intention is to move forward with format databases to map extensions and support multiple use cases
  
  
• only supporting importing ESM
  
  
  
  • no JSON
  
  
  • no native modules
  
  
  • createRequireFunction used to get these
  
  
  • will come back with format database
  
  

Steps to get there

• removing importing of formats other than ESM
  
  
  
  • no common.js
  
  
  • no JSON
  
  
  • no nativemodules
  
  
• removing of dynamic path searching
  
  
  
  • no extension adding
  
  
  • no directory resolution
  
  
  • no support for index.js
  
  
  • still maintaining support for the main field
  
  
• adding createRequireFunction

[michael-ciniawsky]

pkg.module ? Or will pkg.main still lookup the file extension (as an exception)? This feels inconsistent/confusing if paths searching is removed...

⚠️ Note I'm not talking about support .js for ESM or the like, just how this is supposed to work

package.json

{
  "main": "./file" // What happens if e.g ./file.js || ./file.mjs is explicitly defined?
}

package.json

{
  "main": "./file.js",
  "module": "./file.mjs"
}

?

[MylesBorins]

@michael-ciniawsky I think the intent was to support package.main as a means to specify the entry point of a module, and that this path would need to be explicit (no path searching).

We were debating if this should even be supported, perhaps it should be dropped.

[MylesBorins]

here's a doc so people can comment and offer suggestions for changes. If you would like to suggest alternative proposals please do so in the issue

https://docs.google.com/document/d/1uq3v1h3dUXmjMAabQCsxdj7Ig2lOpaty1u4HcUi8whQ/edit?usp=sharing

[michael-ciniawsky]

So no 'dual-mode' packages (for now) ?

[MylesBorins]

indeed. It is something that would be part of a future enhancement to the kernel, dependent on the feature set

[MylesBorins]

I've opened a PR to add documentation about our minimal kernel to the repo. Please comment / approve / reject in there rather than the google doc https://github.com/nodejs/modules/pull/180

[MylesBorins]

We have a PR open to solve this, going to close the issue