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

Doc: Phase 2 for new modules implementation #196

Merged
MylesBorins merged 5 commits into from
Oct 25, 2018
Merged

Doc: Phase 2 for new modules implementation #196

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
281f2fb
Phase 2
GeoffreyBooth Oct 7, 2018
383cd86
Revisions per call with @MylesBorins, @SMotaal, @jdalton and @Geoffre…
GeoffreyBooth Oct 19, 2018
77c6b6e
fixup: update with notes from meeting
MylesBorins Oct 24, 2018
63ce88a
fixup: whoops
MylesBorins Oct 24, 2018
1adb522
Whoops 2
GeoffreyBooth Oct 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions doc/plan-for-new-modules-implementation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,42 @@ These features will be part of the first phase of development:
- No extension adding.
- No directory resolution, including no support for `index.js` or `index.mjs`.
- No support for `main` field for ESM.

* Remove current VM implementation

* Remove current Loader implementation

These changes are implemented in https://github.com/nodejs/ecmascript-modules/pull/6

## Later Phases
## Phase 2

@robpalme robpalme Oct 15, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested intro line:

The purpose of Phase 2 is to introduce uncontentious UX enhancements before we deal with user-land extensibility ("loaders").


* A `--mode` field to enable ESM support in the cases of `--eval` and STDIN input, _or_ any file: `node --mode=esm index.js`.

@ljharb ljharb Oct 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure “or any file” is something that has consensus.

@ljharb ljharb Oct 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(also this should probably clarify that the mode only applies to the entry point)

@ljharb ljharb Oct 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think that the name and value of “mode” should match whatever’s chosen for a package.json field, so I’m not sure those can be separated.

@jdalton jdalton Oct 8, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb The idea of the --mode item was to cover having a way to signal the parse goal. The specifics (name, values, etc.) are something to be ironed out. The reason the flag was chosen instead of other ways was that a flag is likely the most simple, in that realm, to iron out.

@ljharb ljharb Oct 8, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The challenge is that a mode flag would only cover the entry point; but a package.json field would have to cover any file; the former is easier but IMO should be constrained by the latter.

@mcollina mcollina Oct 8, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still skeptical of a flag for the entrypoint. This cannot be placed in a shebang. I'm ok with this being in phase 2, but we should add another entry in the Future Phases to resolve the problem Unix shebang usecase.

@SMotaal SMotaal Oct 9, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I this brings us closer to a starting point without favouring any controversial preferences, but obviously this is more like the one equally hated (not preferred) by everyone 😉

@robpalme robpalme Oct 15, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest rewording to clarify we're talking about entrypoints and that the specifics of the flag are not yet defined. Suggested replacement text:

  • A startup flag to enable ESM support for the entrypoint code.
    • This will cover both:
      • STDIN input, e.g. node --mode=esm --eval
      • file input, e.g. node --mode=esm index.js

- Using this in `package.json` or being scoped to packages is put off for a later phase.
- This will be supplemented/replaced by more robust configurability such as designed in ([#160](https://github.com/nodejs/modules/pull/160)) in a later phase.

* `createRequireFromURL`, to complement the just-added `createRequireFromPath`.

@SMotaal SMotaal Oct 9, 2018 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MylesBorins were you looking to rewrite this to something like:

  • Provide complimentary functions to existing ones in core which more suited for use in ESM modules in order to promote a level of interoperability with existing CJS modules.


* Re-introduce VM module integration.
- Implemented in: https://github.com/nodejs/ecmascript-modules/pull/8.

## Future Phases

@robpalme robpalme Oct 15, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now have two PRs in-flight that define future phases. The wording here seems good for Phase 2.

For future phases, I much prefer the intent-driven descriptions of Phase 3 & 4 in #193 rather than the feature-list style we have here because it allows people to first agree on and rally around why we the phases exist before we decide what to put in them. The actual wording doesn't matter so much - it's more about ensuring we're all clear on the purpose of the future phases.

@GeoffreyBooth GeoffreyBooth Oct 15, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re rewording the entrypoint, sure, I agree with that change.

Re the other PR, this PR is meant to supersede the other one. We included the Phase 2 items from that one in this one.

I don’t mind removing the Future Phases section, but I don’t think having vague language about the topics for each phase helps us much. I think phases can include PRs/features that span different topics, just as Phase 2 does.

@SMotaal SMotaal Oct 15, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly think that "future phases" make it possible to safely consider and discuss things so that when it is time to write "Phase n" we are collectively more comfortable with locking into those priorities.


The following features will follow soon in upcoming phases:

* Loaders ([#82](https://github.com/nodejs/modules/issues/82)), ([#96](https://github.com/nodejs/modules/issues/96)).

* `package.json` metadata support, including an ESM-compatible design for the `main` and possibly `module` fields; and user-configurable map for file extensions to parse goals, a.k.a. `mimes` field ([#160](https://github.com/nodejs/modules/pull/160)).

* Browser-compatible specifier resolution ([#109](https://github.com/nodejs/modules/issues/109)), a.k.a. bare imports. See [package name maps](https://github.com/domenic/package-name-maps).

* Browser-compatible dynamic path searching.

* Multi-mode packages ([#94](https://github.com/nodejs/modules/issues/94)).

* ESM in executable files ([#152](https://github.com/nodejs/modules/issues/152)).

* Callable resolver ([#157](https://github.com/nodejs/modules/issues/157)).

* Mock modules (injection) ([#98](https://github.com/nodejs/modules/issues/98)).

There **will** be future phases. We will **not** ship the code produced by Phase 1. This first phase lacks support for important use cases and will not be released as the new modules implementation.
This is an incomplete list. More features will be added in future phases, based on the [features list in our README](https://github.com/nodejs/modules/#features).