This repository was archived by the owner on Sep 2, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 2, 2023. It is now read-only.
Upstream changes to ESM in nodejs/node #42
Description
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.
Activity
MylesBorins commentedon Feb 28, 2018
/cc @nodejs/tsc who will likely want to follow this conversation
mhdawson commentedon Feb 28, 2018
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 commentedon Mar 1, 2018
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 commentedon Mar 1, 2018
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 commentedon Mar 1, 2018
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 commentedon Mar 1, 2018
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 commentedon Mar 6, 2018
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, orimport.meta.urlwhich 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
nodejsorganization but due to perceptions of bias. I would also probably encourage us not to keep it under thenodejsorganization if we take that route.MylesBorins commentedon Mar 6, 2018
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 commentedon Mar 7, 2018
@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 commentedon Mar 7, 2018
👍 on this. Work on the VM module has not been controversial.
mhdawson commentedon Mar 7, 2018
+1 on landing at least semver minor changes
hybrist commentedon Mar 13, 2018
+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 commentedon Mar 13, 2018
+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.
3 remaining items
MylesBorins commentedon Apr 4, 2018
No objections from the TSC. Closing the issue
devsnek commentedon Apr 28, 2018
to be closed after we start allowing upstream changes
BridgeAR commentedon Apr 28, 2018
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 commentedon Apr 28, 2018
@BridgeAR it goes by how the change would be versioned if esm wasn't experimental
BridgeAR commentedon Apr 28, 2018
I am also not sure how this shall work: I assume
semver-minorandsemver-majorchanges 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 commentedon Apr 28, 2018
@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 commentedon Apr 28, 2018
@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 commentedon Apr 28, 2018
I just discovered this issue. In particular, this comment speaks to me:
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 commentedon Apr 29, 2018
@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 commentedon Jul 11, 2018
Closing as I feel we have consensus on this