Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Experimental Status #564

Closed
guybedford opened this issue Oct 20, 2020 · 18 comments
Closed

Experimental Status #564

guybedford opened this issue Oct 20, 2020 · 18 comments
Labels
modules-agenda To be discussed in a meeting

Comments

@guybedford
Copy link
Contributor

Currently the entire modules implementation is marked as experimental status.

With the completion of the major backports to v12, we effectively have a natural stability for the primary features of the implementation.

If we make this natural experimental status explicit by changing the experimental status of the entire implementation, this will help give users confidence in upgrading their libraries and applications to using modules in Node.js.

Individual modules features can still be retained as having experimental status. These include:

  • All features with experimental flags
  • Loaders

The following newer features for modules may or may not be suitable to remain in experimental status:

  • Top-level await
  • CommonJS named exports
  • Subpath imports
  • Subpath patterns

Perhaps we can put some time to discussing the details of this stability change further.

@MylesBorins MylesBorins added the modules-agenda To be discussed in a meeting label Oct 20, 2020
@bmeck
Copy link
Member

bmeck commented Oct 20, 2020

I think the only thing I might want is more tests for Subpaths really. Should probably split that up into much more granular topics (exports, imports, conditions, patterns, require() [no tests for exports?] /import)

@ljharb
Copy link
Member

ljharb commented Oct 20, 2020

Are patterns backported to 12 and released? (i think imports is already in 12, right?)

@MylesBorins
Copy link
Contributor

@ljharb patterns and cjs auto named exports are not backported yet but we have one last Semver-Minor for 12.x to make sure we can land that. Just letting it bake on 14.x / 15.x a tiny bit more

@GeoffreyBooth
Copy link
Member

In my mind the question is whether there are any still-unfulfilled goals we have related to ES modules that might necessitate breaking changes to the implementation we’ve built. I think the only outstanding goals are hot module reload and instrumentation; is there anything else I’m forgetting? If not, both of those are part of loaders, which I’m sure will stay experimental, so it seems fine to “release” the rest.

@ljharb
Copy link
Member

ljharb commented Oct 21, 2020

The only other thing I can think of is node-style resolution, but it seems clear that some of the node collaborators here would block on it, so it’s likely not worth the effort to discuss.

@GeoffreyBooth
Copy link
Member

The only other thing I can think of is node-style resolution, but it seems clear that some of the node collaborators here would block on it, so it’s likely not worth the effort to discuss.

Yes. But that can be added at any point without it being a breaking change, so it’s not a factor in whether we keep the status as experimental.

@robertdumitrescu
Copy link

I definitely agree that ESM modules should not be experimental anymore. Mocha/Chai/Nyc/C8 frameworks are all waiting for that to go Stable to implement them. Currently for coverage data, you need to do loads of hacks in the IDE's to get it working with ESM modules.

@MylesBorins
Copy link
Contributor

MylesBorins commented Oct 21, 2020

It might be a good idea to create an enumerated list of the "features" that make up ESM (as Guy did above), determine the status of each of those features, and determine which of those features need to be stable to consider ESM as a whole stable. It would likely also be a good idea to separate the pure ESM features from package features effecting both ESM + CJS

Looking at the list below it would seem to me the only thing I would want to see be a bit more stable before flipping ESM itself to stable would be to have commonjs named exports bake a bit more.

For the Package related features we actually have a stability level for each feature and I've opened a PR to stabilize all features aside from subpath patterns which likely need a bit more time to bake.


ESM only

Stable:

  • ESM loader
  • ESM named exports
  • node: imports
  • data: imports
  • import.meta
  • builtin modules
  • createRequire
  • Resolution algorithm

Experimental:

  • Top-level await
  • CommonJS named exports
  • JSON modules
  • WASM modules
  • Loaders
  • experimental specifier resolution

Package Features (esm + cjs)

stable:

  • type: 'module'
  • --input-type
  • package exports / imports
  • conditional exports / imports
  • Subpath exports / imports
  • exports sugar
  • nested conditions
  • user conditions

Experimental:

  • Subpath patterns

Unsolved Features:

  • VM ESM API
  • Hot Module Reload
  • Module Cache
  • Instrumentation

@SimenB

This comment has been minimized.

MylesBorins added a commit to MylesBorins/node that referenced this issue Oct 23, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564
@bizob2828
Copy link
Contributor

This is def a selfish response but I worry the experimental flag will be removed before there is a way to instrument esm.

@GeoffreyBooth
Copy link
Member

This is def a selfish response but I worry the experimental flag will be removed before there is a way to instrument esm.

The plan is to permit this via loaders, which are remaining experimental. Loaders can already do some amount of instrumentation, but we don’t consider the user story satisfied without chained loaders, among other TODO features of loaders.

@bizob2828
Copy link
Contributor

This is def a selfish response but I worry the experimental flag will be removed before there is a way to instrument esm.

The plan is to permit this via loaders, which are remaining experimental. Loaders can already do some amount of instrumentation, but we don’t consider the user story satisfied without chained loaders, among other TODO features of loaders.

Got it. I know I I asked this last year @GeoffreyBooth but I can’t seem to find it. Can you point me to a good example of instrumenting ESM with loaders? I seem to remember you may have wired one up as a POC.

@MylesBorins
Copy link
Contributor

@bizob2828 FWIW the flag is already removed on 14 and 12

@bizob2828
Copy link
Contributor

@bizob2828 FWIW the flag is already removed on 14 and 12

@MylesBorins i misspoke above. I meant removing experimental status from ESM. But if the plan is to keep loaders experimental then I guess my comments are irrelevant. I still don't see a way to instrument module code with the loaders currently defined. As an agent maintainer, I'll be in a difficult spot and lose coverage if they are using ES modules.

@GeoffreyBooth
Copy link
Member

Can you point me to a good example of instrumenting ESM with loaders? I seem to remember you may have wired one up as a POC.

The closest we have to a POC is https://nodejs.org/api/esm.html#esm_transpiler_loader. That example shows how the source code of any loaded module can be modified before Node parses it, which would let you inject anything you want. See also the getGlobalPreloadCode hook.

MylesBorins added a commit to nodejs/node that referenced this issue Oct 29, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@bizob2828
Copy link
Contributor

Can you point me to a good example of instrumenting ESM with loaders? I seem to remember you may have wired one up as a POC.

The closest we have to a POC is https://nodejs.org/api/esm.html#esm_transpiler_loader. That example shows how the source code of any loaded module can be modified before Node parses it, which would let you inject anything you want. See also the getGlobalPreloadCode hook.

Got it. @GeoffreyBooth is there a plan to provide a hook that can operate before/after compiling the ESM code? That's the kind of hook point I'm interested in because of the type of work we do to make our agent work via monkey patching

@GeoffreyBooth
Copy link
Member

a hook that can operate before/after compiling the ESM code

What do you mean by “compiling”?

@bizob2828
Copy link
Contributor

a hook that can operate before/after compiling the ESM code

What do you mean by “compiling”?

I apologize if I'm using improper terms. I'm comparing to what you'd get when you require. So the value of the module.exports in cjs land.

targos pushed a commit to nodejs/node that referenced this issue Nov 3, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit to nodejs/node that referenced this issue Dec 8, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit to nodejs/node that referenced this issue Apr 29, 2021
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
modules-agenda To be discussed in a meeting
Projects
None yet
Development

No branches or pull requests

8 participants