Upstream changes to ESM in nodejs/node

[MylesBorins]

During today's meeting there was discussion about what should we do with upstream changes to core. Some suggestions were

• nothing, allow changes to happen as they have been
• feature freeze. Allow bug fixes and infrastructure changes but do not land new features
• stop all changes against the esm implementation
• remove esm from core and start from scratch

It is worth noting that this team does not currently have any ability to demand that any of the above be implemented, but we do have enough representation from collaborators within this group that we can definitely make it work in practice if we have consesnsus

None of these solutions are ideal, but we do need to figure out a compromise.

One thing I think will be important if we do any sort of freeze on development, or removal of code, is that we set clear deadlines and expected deliverables for when that freeze is over, or when code is brought back.

[MylesBorins]

/cc @nodejs/tsc who will likely want to follow this conversation

[mhdawson]

Not to say this is the right solution, but early development for N-API was done in a repo under the Node.js org which was a clone of master. This did result in a big PR that was hard for people to review but otherwise worked fairly well until we believed it as stable enough to pull into master.

[devsnek]

imo the best course is to allow changes that work with whatever our current direction is. that way we can stick to maintaining the direction and the people in core can stick to making code. if we change direction then all we have to do is change some code (i'm willing to do this, i hope other people are too). as long as we keep modules experimental while we figure this all out its pretty fine.

[weswigham]

TBQH, I don't think anyone's wholly against people continuing to write code (beyond philosophical issues to do with writing code without full direction), so long as "The code already exists" isn't used as a reason to silence or force discussions and designs within the group (they may be counter to any existing implementation), or to block future work that may eliminate current implementations. But I shouldn't presume to speak for others.

[benjamingr]

I'm strictly +1 on allowing changes in nodejs/node and improvements on the existing loader - it's under a flag and marked experimental anyway and we can discuss design while it's worked on anyway.

[mcollina]

Not to say this is the right solution, but early development for N-API was done in a repo under the Node.js org which was a clone of master. This did result in a big PR that was hard for people to review but otherwise worked fairly well until we believed it as stable enough to pull into master.

It worked well for http2 too. That being said, we have shipped some esm support under a flag. I think we should not discard that option for specific topics, i.e. named exports, interop and other things were a single PR would create a lot of traffic to nodejs/node. For less contentious topics, doing a PR against nodejs/node will be fine.

I'm 👎 to freezing, removing and "starting over". I think we should leverage the current releases to gather feedback from users. As long as the code stays within the experimental boundaries, we can also test different ideas and gather feedbacks.

[bmeck]

I have voiced my opinions repeatedly and would prefer not to halt PRs. In particular, I wish not to halt progress on bug fixes or missing features that are non-controversial by nature such as import() support which is mandated by spec, or import.meta.url which was non-controversial to land.

If we halt progress, I strongly would desire to remove all ESM code from core and move it to a fork that can continue to progress. I would like to create a level playing field for all implementations again at that point since that is what a moratorium on PRs would signal to me. It seems that there is disagreement on how to progress so I am unsure if that fork would continue to be in the nodejs organization but due to perceptions of bias. I would also probably encourage us not to keep it under the nodejs organization if we take that route.

[MylesBorins]

A quick proposal.

How would people feel about allowing for semver-patch changes to land as per usual, but to hold off on landing semver-minor prs until the group has reached consensus on an implementation?

[bmeck]

@MylesBorins if we can go forward with that on a feature level of consensus I think that is fine. I do still want to work on the user land Loader bits since thats most of what I'm doing right now and isn't the disambiguation mechanism. If desired I can pause that too since there are other APIs that would be nice but are not directly related to Modules that would aid things. I'm also unsure if we need to stop the vm.* bits since they shouldn't be much more than hooks into the actual specification.

[TimothyGu]

I'm also unsure if we need to stop the vm.* bits since they shouldn't be much more than hooks into the actual specification.

👍 on this. Work on the VM module has not been controversial.

[mhdawson]

+1 on landing at least semver minor changes

[jkrems]

+1 on landing changes until we have "official" opinions (use cases, goals, scope) and are a chartered working group. It seems weird to change process in nodejs/node based on what is - at least at this point - just "people talking to each other".

[giltayar]

+1 on landing any change. I am pleased that we are seeing progress being done in ESM, and would not want it to stall. If we decide to reboot the initiative, or make minor changes to the existing stuff, we can still do that, given that it's under an experimental flag.

Having said that, the experimental flag should be removed only after this group reaches consensus.

[MylesBorins]

I'm +1 to landing changes that improve general DX and don't drift from current expectations
I'm -1 to landing changes that are introducing new mechanisms that have not had support before. I think it is reasonable to try and minimize churn.

For example. I think landing a change to do named exports is a++. I think we should hold off on landing the change regarding a new mechanism in package.json for package specific loaders.

This would mean that I'm -1'ing import.meta.require for the time being, a mechanism I think we need long term, fwiw

[MylesBorins]

In todays meeting we concluded that we would like to request from the Node.js TSC the following

• Any SemverPatch changes can land
• For any Semver Minor Changes or Semver Major changes we would like an opportunity to discuss as a working group and make a recommendation when we think specific features should not land.

I'm picking the wording here really carefully to make sure that we stay within the bounds of what we currently have authority over. With the number of TSC members and collaborators in the group, if we saw anything as a major issue we could flag through the normal process. Having an agreement regarding our ability to review changes first will give us the opportunity to make the process more agreeable and less confrontational.

Does anyone object to me presenting the above to the TSC next week?

[MylesBorins]

No objections from the TSC. Closing the issue

[devsnek]

to be closed after we start allowing upstream changes

[BridgeAR]

Any SemverPatch changes can land

Could this be clarified for me: since ESM is currently experimental, there is actually no semver-minor and semver-major as any change may land as semver-patch, if I am not mistaken.

[devsnek]

@BridgeAR it goes by how the change would be versioned if esm wasn't experimental

[BridgeAR]

For any Semver Minor Changes or Semver Major changes we would like an opportunity to discuss as a working group and make a recommendation when we think specific features should not land.

I am also not sure how this shall work: I assume semver-minor and semver-major changes as in: "new features" and "breaking the current API" while being experimental.

The working group should always be able to give a recommendation for any PR. I do not see any limitations about that. Any individual collaborator could also give a +1 or -1 as with all other PRs as well. Discussing a PR in the working group is something somewhat independent for me that should always be possible.

Here it is explicitly worded to give recommendations if something should not land. Trying to follow the intent, I think it would be best to give a recommendation for PRs that should land and all others are blocked until then. Because right now I see a couple of PRs upstream that are stalled without any further progress and it would be great to know what to do with these. There is no explicit "-1" from the working group as far as I can tell by quickly scanning some comments. So by strictly following the above, it would actually seem they should be fine to land (but I guess the intent was different). Is there a timeframe in which the working group will give a recommendation for these PRs (I might also just have missed such comment)?

[devsnek]

@BridgeAR currently this wg has decided to go back to the drawing board. we are working on coming up with use cases and turning those use cases into features. as we meet every other week i would estimate a few more months before we have concrete features we want and then i assume the implementors will be let loose but i'm not sure of anything at this point

[MylesBorins]

@BridgeAR I think the idea was to not allow progress around bug fixes to be blocked by the WG as we only meet every two weeks.

I've gone through and added the blocked label to the PRs that are waiting on this. In future I'll try to make sure that blocked PRs have the appropriate label and reference this issue.

[GeoffreyBooth]

I just discovered this issue. In particular, this comment speaks to me:

TBQH, I don’t think anyone’s wholly against people continuing to write code (beyond philosophical issues to do with writing code without full direction), so long as “The code already exists” isn’t used as a reason to silence or force discussions and designs within the group (they may be counter to any existing implementation), or to block future work that may eliminate current implementations.

The reason I asked to join this group in the first place is because I felt that exactly this was happening, and I feel that it still is. I’d rather not name names and link to specific comments, but my sense of the discourse on GitHub is that it’s rather toxic, with criticism of the current implementation not given the respect it deserves. Personally I would vote for removing the current implementation from Node, because commenters are pointing to the choices made in its implementation to imply that those choices are approved and final. Based on this group’s discussions so far, it seems likely to me that some of those choices may change, and it would be a shame for users to build downstream tools based on the current experimental implementation under the assumption that it will be released largely as is.

That said, I would certainly be open to suggestions for how to speed up the modules group’s process. Perhaps we could meet every week, or make meetings two hours, or designate subcommittees for subsets of features. I understand the contributors’ eagerness to get this feature built and approved, and I support that mission.

[devsnek]

@GeoffreyBooth i just want to make an important distinction between saying a feature can't be done because the current implementation doesn't do it and saying it can't be done because of some other reason, and maybe pointing to something within our existing implementation as an example. When i'm reading through stuff here i'm thinking about how code can be written to make that happen, whether it be by modifying v8, node, etc. If i see something that defies a principle of the current spec or codebase we have i think its important to make people aware of that so they can become more informed throughout this process, the same way i was informed by jdalton's talk on std/esm or reading through the source of coffeescript after you applied to be an observer.

[MylesBorins]

Closing as I feel we have consensus on this