search

nodejs / modules

Node.js Modules Team
MIT License
413 stars 42 forks source link

Moving forward with Dynamic Modules? #252

Closed bmeck closed 4 years ago

bmeck commented 5 years ago

I would like to propose that we move forward with Dynamic Modules. I would just like to see if we have consensus moving forward. We don't have an alternative, and it satisfies a large category of usage. I would like to reach consensus if we can move forward using Dynamic Modules and allow us to plan on integrating it into our implementation. Do we have outstanding objections? I would like to add this (albiet late) to the TC39 agenda so it can at least be discussed.

jdalton commented 5 years ago

Do we have outstanding objections?

Yes. Dynamic Modules as written today is something I, as a TC39 delegate, would not feel comfortable agreeing to because of its syntax poisoning and the inconsistent/confusing developer experience it creates. While I understand the drive to push something through, I'm not okay pushing through a solution that further confuses/complicates the developer story.

A possible path forward could be to:

Treating all modules as dynamic reduces ecma262 complexity and gives Node the freedom to create modules that interop as it needs (even supporting export * from "cjs"). It is possible for Node to then create dynamic modules that are 💯 compliant when interacting with other ES modules. For example, Node could pass the test262 test suite, while also supporting CJS, WASM, or other module formats as they come.

bmeck commented 5 years ago

@jdalton If that is the case we need an alternative, otherwise I feel like we have no choice but to not ship any support for named exports from CJS by our April deadline. This feels like a regrettable, but inevitable choice. To be clear, I feel we must reach consensus on something, and if not we are going to have to remove any sort of named export support if we want to ship something (whatever it is). I feel your complaints are able to be handled in a follow on but there is a lack of alternative proposal being presented by your repeated comments. As such, would you be willing take over championship as we are lacking consensus and it seems you are seeking to prevent forward progress on the current proposal? As it stands, if we lack consensus, we should do as we have done to other features and remove that which is unable to reach consensus. However, if we reach quorum in the group to go forward with Dynamic Modules, I feel that would be preferable to not having anything.

jdalton commented 5 years ago

If that is the case we need an alternative, otherwise I feel like we have no choice but to not ship any support for named exports from CJS by our April deadline. This feels like a regrettable, but inevitable choice.

I understand. That may be.

I feel your complaints are able to be handled in a follow on but there is a lack of alternative proposal being presented by your repeated comments

I believe it's more a fundamental design difference that isn't suited as a follow-on.

As such, would you be willing take over championship as we are lacking consensus and it seems you are seeking to prevent forward progress on the current proposal?

Take on championship of dynamic modules? Yes (in whatever capacity is needed)

As it stands, if we lack consensus, we should do as we have done to other features and remove that which is unable to reach consensus.

Okay.

However, if we reach quorum in the group to go forward with Dynamic Modules, I feel that would be preferable to not having anything.

I prefer interop too, but not at this cost. I feel the proposal, as is, will not pass the TC39.

bmeck commented 5 years ago

Given that you could block it at TC39 that seems certain, not likely.

On Mon, Jan 21, 2019, 11:09 AM John-David Dalton <notifications@github.com wrote:

If that is the case we need an alternative, otherwise I feel like we have no choice but to not ship any support for named exports from CJS by our April deadline. This feels like a regrettable, but inevitable choice.

I understand. That may be.

I feel your complaints are able to be handled in a follow on but there is a lack of alternative proposal being presented by your repeated comments

I believe it's more a fundamental design difference that isn't suited as a follow-on.

As such, would you be willing take over championship as we are lacking consensus and it seems you are seeking to prevent forward progress on the current proposal?

Take on championship of dynamic modules? Yes (in whatever capacity is needed)

As it stands, if we lack consensus, we should do as we have done to other features and remove that which is unable to reach consensus.

Okay.

However, if we reach quorum in the group to go forward with Dynamic Modules, I feel that would be preferable to not having anything.

I prefer interop too, but not at this cost. I feel the proposal, as is, will not pass the TC39.

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

ljharb commented 5 years ago

It’s worth noting that it already was considered acceptable by the subset of delegates present in November; but certainly with both your delegate objection, and lacking consensus in the modules group, it proceeding is unlikely.

zenparsing commented 5 years ago

Do we have outstanding objections?

Yes.

I have the following concerns with dynamic modules:

There are two alternatives:

  1. The one that @jdalton mentioned, where essentially everything becomes a dynamic module. The only changes required to ecma262 are the ones mentioned above, although to be honest I'm not sure what the implementation would look like in Node. It would likely be quite different from --experimental-modules.
  2. Non-transparent interop.

Ultimately, I believe that this isn't so much about the 262 spec as it is about answering the question of what kind of interop we want. TC39 and V8 will want to know that we have consensus on a strategy before making changes. Perhaps we should shift the conversation in that direction.

Let me ask a question: it seems to me that non-transparent interop, where we cannot import from CJS but must use import.meta.require is similar, can be a viable solution for our users. Do you agree or disagree?

bmeck commented 5 years ago

If we are unwilling to move forward with named exports from CJS currently, and due to our LTS cutoff, would people be ok completely tabling this and aim for next year's release to discuss named exports from CJS?

@zenparsing

Let me ask a question: it seems to me that non-transparent interop, where we cannot import from CJS but must use import.meta.require is similar, can be a viable solution for our users. Do you agree or disagree?

I think it isn't a yes/no answer so your questions isn't really something I can reply to. However, I will point out that it doesn't serve any users which expect to be able to use named imports from CJS and the alternatives that will eventually allow such remain unclear if the demands in this thread are held over time. It appears that there is a desire to frame Dynamic Modules as not suitable for 1 case, and therefore it is acceptable to not serve any cases that it might fulfill.

robpalme commented 5 years ago

If we are unwilling to move forward with named exports from CJS currently, and due to our LTS cutoff, would people be ok completely tabling this and aim for next year's release to discuss named exports from CJS?

I think our group has already decided that is the procedure to ensure forward progress. Controversial items get punted to later phases.

bmeck commented 5 years ago

@robpalme I agree, just want to be clear about tabling all discussions of named exports from CJS seems like what we are agreeing to since it seems we will not be able to move forward with Dynamic Modules. Being explicit about that should help us focus on making our April deadline and avoid discussing named exports from CJS; thus allowing us to more readily reach consensus on other issues that are still likely to be resolved before then.

ljharb commented 5 years ago

We are already bound forever to explicit parse goal disambiguation, whether by extension or package.json, due to the ambiguous parsing between the Module and Script goals in the spec. Dynamic Modules don’t change this.

As for this “April cutoff”, I’m pretty sure there was never consensus for any kind of cutoff - i, for one, am not on board with any sort of time-based deadline that might lead to shipping something incomplete.

bmeck commented 5 years ago

@ljharb this has been discussed in the past, the LTS branch is cut off in April, regardless of what we do in this group. If we do not upstream something by then, we will wait until the next April for the next LTS. It has been lightly discussed that we should upstream the minimal kernel at least.

MylesBorins commented 5 years ago

Fwiw we will still be able to make changes after April, experimental apis can receive breaking changes, even during LTS

The biggest challenge is that if we need to make semver major changes outside of the scope of esm then we will be stuck (e.g. package.exports)

On Mon, Jan 21, 2019, 1:17 PM Bradley Meck notifications@github.com wrote:

@ljharb https://github.com/ljharb this has been discussed in the past, the LTS branch is cut off in April, regardless of what we do in this group. If we do not upstream something by then, we will wait until the next April for the next LTS. It has been lightly discussed that we should upstream the minimal kernel at least.

— 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/252#issuecomment-456160908, or mute the thread https://github.com/notifications/unsubscribe-auth/AAecVxdAcfTniiylpPmOB1ARaTyx9a6dks5vFgRKgaJpZM4aLA3I .

ljharb commented 5 years ago

If it remains behind a flag, i wouldn’t block that.

For the record, i do want this to advance, i do think that even with this restriction, it’s a massively better situation than without dynamic modules, and i strongly disagree that no interop is better than partial interop, especially when the excluded portion is the very rare edge case of a star export from a CJS module combined with a circular dependency.

bmeck commented 5 years ago

I would not like to see us take yet another year to land ESM support unflagged. We do have some minimal agreement on a kernel and it seems unlikely to change in a breaking way. If we can focus on what would be unable to be seen as stable enough to potentially unflag over the LTS I would rather prioritize those as there is a chance we could unflag during LTS over the next year.

ljharb commented 5 years ago

I don’t find it acceptable to ever unflag with only the minimal kernel - my concern is about ecosystem adoption, and those behavior patterns will become set once we unflag, and as such, i think we must have a complete implementation first.

robpalme commented 5 years ago

@ljharb The list of features that need to be in place before we would consider unflagging is a useful topic of discussion. Maybe it deserves a dedicated issue?

GeoffreyBooth commented 5 years ago

As I see it:

If we ship something where export doesn’t support all the forms listed on the MDN page, for example, users will claim that Node’s ESM implementation isn’t spec compliant. And arguably, they’d be right. This has been a red line for us from the start.

I think we can ship something flagged that has this issue, but not unflagged. So this raises two questions:

  1. Surely we can at least target shipping our new implementation behind a flag by April, even if it’s lacking essential features such as this?
  2. April is months away. Why can’t we just do the work @jdalton is proposing and get this working the way users expect before April?
bmeck commented 5 years ago

@GeoffreyBooth

If we ship something where export doesn’t support all the forms listed on the MDN page, for example, users will claim that Node’s ESM implementation isn’t spec compliant. And arguably, they’d be right. This has been a red line for us from the start.

I note your dissent, but it is not shared amongst all members of this WG; it has not been a red line except to specific members finding it to be required. I do not find it to be required when faced with zero named exports as the alternative.

Surely we can at least target shipping our new implementation behind a flag by April, even if it’s lacking essential features such as this?

Removing all named exports would be viable since the claim is to not reuse the work of Dynamic Modules as it exists today per previous Modules WG meetings. We remove things as they are unable to reach consensus, and this feature certainly seems to fall into that category given the comments in this thread. We can try for next year and allow ourselves time to focus on what we can do this year.

April is months away. Why can’t we just do the work @jdalton is proposing and get this working the way users expect before April?

It is 3 months away and 7 Modules WG meetings to create the proposal, review, and reach consensus. It also is not enough time to attend TC39 to get a clear gauge on the reaction there. Reaction from the November TC39 is the reason that export * from was removed from @guybedford 's Dynamic Modules. This leads to another reason to not land any named exports if we are not going to be using Dynamic Modules.

The proposal by @jdalton to use some other alternative has been discussed in multiple meetings since the November TC39 meeting and it was supposed to be presented to the group in time for the January TC39 meeting. The official deadline for the January agenda of TC39 has passed. Also, I have objections to @jdalton 's approach as that would mean never using Source Text Module Records which are the foundation of ESM. It is tantamount to just using a different module system from the ECMAScript spec by never using ECMAScript's standard and instead making one of our own. To myself that goes against the entire point of this WG which is to use the standard modules and to land that in Node's core.

ljharb commented 5 years ago

Note that the January agenda deadline is only for stage advancement; there's still time to get something on this month's agenda.

GeoffreyBooth commented 5 years ago

I have objections to @jdalton ‘s approach as that would mean never using Source Text Module Records which are the foundation of ESM. It is tantamount to just using a different module system from the ECMAScript spec by never using ECMAScript’s standard and instead making one of our own. To myself that goes against the entire point of this WG which is to use the standard modules and to land that in Node’s core.

What difference does it make what happens under the hood? As long as Node behaves according to spec as far as users can observe, that’s all that matters. No user is going to file a bug report complaining that Node’s implementation of ESM under the hood doesn’t follow spec, if Node runs the ESM syntax as users expect and users get the results they expect. The “spec” that users care about is the syntax. export * from 'commonjs-package' should work.

If @jdalton wants to spearhead a working group to produce a branch that implements his proposal, and it can happen by February or March in time for everyone to review it, I say he should go for it.

ljharb commented 5 years ago

@GeoffreyBooth there is more to be concerned about than "whether a user files a bug report"; just because someone doesn't notice a violation doesn't mean there isn't one (and just because they think there is one, doesn't mean there is).

jdalton commented 5 years ago

My suggestion aimed to reduce complexity absorbed by ecma262 while pushing the freedom to implement and burden of compliance to Node.

It looks like we're at draw here. Which brings us to @zenparsing's alternative 2. I think we may make more progress on the non-transparent interop front for Node, leaving more robust forms of interop to user-land/transpilers/loaders.

ljharb commented 5 years ago

I feel that non-transparent interop is simply never an option, due to the existence of the npm ecosystem. If the default node experience does not include such interop, then I don't think it's worth shipping any ESM support whatsoever.

jdalton commented 5 years ago

@ljharb

If the default node experience does not include such interop, then I don't think it's worth shipping any ESM support whatsoever.

That's another route for sure and may be the reality if there's no wiggle.

GeoffreyBooth commented 5 years ago

As a user, I find the idea of dropping either transparent interop, or dropping part of the ESM syntax, because the spec says a certain thing should happen a certain way under the hood, ridiculous. If the spec defines the implementation to such detail, then the spec should be changed. Users want both transparent interop and they want the ESM syntax they’ve been promised since 2015.

ljharb commented 5 years ago

The current dynamic modules proposal is the “wiggle”, as i see it, and the best possible option available (although i ofc still hope for better).

My understanding is that people using export * won’t have circular deps (considered a bad practice anyways) and so won’t run into any “uncanny valley”.

jdalton commented 5 years ago

@ljharb I don't want to re-hash the objections to the current dynamic modules proposal, but let's just say there's no wiggle to be had on that front. We can wiggle on having no-named exports support for user provided CJS (so like --experimental-modules), leaving named exports to user-land/transpilers/loaders.

zenparsing commented 5 years ago

I feel that non-transparent interop is simply never an option, due to the existence of the npm ecosystem.

I don't think we should accept the impossibility of ecosystem adaptation to non-transparent interop without a careful analysis. It may be that if we give the ecosystem the right tools things will work out just fine.

robpalme commented 5 years ago

then I don't think it's worth shipping any ESM support whatsoever.

Demand & adoption of ES modules as an authoring format is high despite lack of Node runtime support. Growth charts of npm packages show that most of the world's JS code will be written over the next 12 months. That code could be shipped as ESM if we add support to Node soon.

IMO those fundamentals are strong enough to overcome transitory impediments, such as lack of transparent interop.

I'm not arguing for dropping the interop, just that we don't need to draw red lines, because that can lead to paralysis which is the worst outcome.

DanielRosenwasser commented 5 years ago

Can I understand why we believe there's an "uncanny valley" situation (or just a major concern around inconsistent user experiences)? As far as I can tell, @std/esm also exhibits issues around circularities when the cjs option is enabled and interoperating between ESM/CJS, but is pitched as a "just works" solution.

Issues around circularities in TypeScript have been reported at most a handful of times in the span of 4 years of ES2015 modules being supported on top of CommonJS, and in the end the truth is that users just shrug it off, get something working, and move on. I'd hazard a guess that the same is true of Babel. So I really want to take a pragmatic approach and challenge the notion that the circularity issue is a blocker for moving forward.

jdalton commented 5 years ago

@DanielRosenwasser

Can I understand why we believe there's an "uncanny valley" situation (or just a major concern around inconsistent user experiences)?

With the current proposal syntax like export * from "mod" may-or-may-not throw depending on the module type of "mod".

As far as I can tell, @std/esm also exhibits issues around circularities

Its successor, the esm loader, can support circular ESM/CJS modules though bug reports are welcomed.

So I really want to take a pragmatic approach and challenge the notion that the circularity issue is a blocker for moving forward.

I don't think we're fully blocked. For me support for CJS named exports aren't worth poisoning the ESM syntax. If needed, there are multiple ways to move forward without support for them.

ljharb commented 5 years ago

To add to that, I’m also confused why “99% of named exports” is considered worse than “0% of named exports”.

ljharb commented 5 years ago

If esm can support this syntax without violating the spec, then that’s how we can do it in node - if it can’t, then it doesn’t actually support it.

Not allowing named imports from cjs is poisoning the ESM syntax experience; at the moment, it’s a question of whether you want “everything to work minus a rare edge case”, or, “less than half of the syntax to work”. I can’t conceive of why there’s any user-prioritizing argument that would argue for killing the common cases for the sake of an edge case, when all the common cases can work otherwise.

jdalton commented 5 years ago

@ljharb

If esm can support this syntax without violating the spec, then that’s how we can do it in node - if it can’t, then it doesn’t actually support it.

The approach of the esm loader more closely aligns with the approach of treating all modules as dynamic modules.

Not allowing named imports from cjs is poisoning the ESM syntax experience

It's less than optimal but it's not poisoning the syntax.

I can’t conceive of why there’s any user-prioritizing argument that would argue for killing the common cases for the sake of an edge case

I'd dig support for CJS named exports too, but not at this expense. If you feel super strong about named exports support, and you have some spare cycles, it'd be rad if you could work with me and @zenparsing to poke at treating all modules as dynamic đŸ€

bmeck commented 5 years ago

I have strong objections to not shipping actual ESM and calling whatever we ship as "esm". If we do go with shipping a different module system than the standard, we should call it something else.

robpalme commented 5 years ago

If we forget the implementation, what would be the observable user-facing behaviours of switching to dynamic modules rather than the "actual ESM" that @bmeck describes? Would it retain the same level of browser compatibility?

bmeck commented 5 years ago

@jdalton is not talking about the spec we have, but a different one that hasn't been made available yet. We cannot review the delta without that.

guybedford commented 5 years ago

I've added the modules agenda to this issue, which can hopefully include the update from TC39 here as well.

guybedford commented 5 years ago

I've closed out the ECMA262 dynamic modules PR, and left a note as to the blocks on this - https://github.com/tc39/ecma262/pull/1306#issuecomment-467761625. Happy to discuss further with anyone who wants to pick up the work here.

ljharb commented 5 years ago

I don’t see why it’s closed; someone can pick up the work from the open PR.

MylesBorins commented 5 years ago

Are we still planning on revisiting this with tc39?

ljharb commented 5 years ago

I’d like to see that happen - with the recently merged changes around cyclic modules, and the STMR change suggestion, @guybedford what do you think about adjusting your proposal?

guybedford commented 5 years ago

The proposal can certainly be updated to be against the new cyclic modules, but that doesn't change any of the constraints of the proposal. The consensus issues still stand around concerns of the empty namespace state, export * support being impossible to support without some kind of back-referencing in ECMA262, and the complaints against having dynamic modules features in ECMA262 to begin with. If anyone wants to pick this up I'd be glad to discuss further.

ljharb commented 5 years ago

My understanding was that domenic suggested removing STMRs from the spec entirely, thus allowing node to implement dynamic module records on top of abstract module records - can you clarify if i'm misunderstanding? if that's right, then is that a potential direction?

guybedford commented 5 years ago

That seems like it would cater to Domenic's concern about them being referenced in ECMA262, but the namespace empty state would still be an ECMA262 concern I think? The architectural consensus around dynamic modules still remain in any form though. If Node.js entirely forks the module system that would have implementation implications for the v8 integration too.

ljharb commented 5 years ago

HTML is creating “Synthetic Modules” https://github.com/heycam/webidl/pull/722

Is that something we could build off of, or emulate, to make dynamic modules possible?

devsnek commented 5 years ago

it has no difference in actual semantics to our current dynamic module wrapper used in translation.

ljharb commented 5 years ago

Right - but if the web is doing it regardless, then any TC39 concerns would not really apply, and node would be able to do it too?

devsnek commented 5 years ago

@ljharb it (Synthetic Modules) doesn't provide any new capabilities over what we already have.

The set of exported names is static, and determined at creation time (as an
argument to [$CreateSyntheticModule$]), while the set of exported values can be
changed over time using [$SetSyntheticModuleExport$]. It has no imports or
dependencies.

Our current synthetic system (createDynamicModule) is a bit like:


The set of exported names is static, and determined at creation time (as an
argument to createDynamicModule), while the set of exported values can be
changed over time using reflect.exports.$name.set(). It has no imports or
dependencies.
SMotaal commented 5 years ago

Right - but if the web is doing it regardless, then any TC39 concerns would not really apply, and node would be able to do it too?

@ljharb It seems that there is wiggle room for Synthetic Modules moving to the spec