-m alias; determine type based for symlinks based on targets; error on mismatched type flag and package scope

[GeoffreyBooth]

This PR fills in a few to-dos from the last entry points PR:

• A new flag -m is an alias for --type=module.

• Module type is now determined based on a symlink’s target rather than its source. This allows a symlink like /usr/local/bin/foo to point to /usr/local/node_modules/foo/bin.mjs and the type is determined by bin.mjs (or foo/package.json if bin.mjs were bin.js). The existing Node flags --preserve-symlinks and --preserve-symlinks-main are still supported, which would cause the type determination to be based on the source instead. With tests.

• Addressing feedback from @bmeck, an error is now thrown when --type and file extension mismatch; or when --type and package scope mismatch. For example: node -m file.cjs, or node --type=commonjs index.js where the nearest parent package.json contains "type": "module". The spec has been updated to account for this.

• Addressing feedback from @devsnek, within the codebase the type strings are now either 'module' or 'commonjs' to match the values accepted by --type and "type":.

Supersedes #39.

[devsnek]

as long as esm is being renamed, can it pretty please be changed to source-text-module or stmr or something. we're going to have at least stmr and wasm in the future, and probably more.

and type is very generic for a flag to the node binary, it should probably be something more specific like --entry-type

and i don't think adding the shortcut is a good idea. we generally don't add shortcuts for other things like this where they change major runtime behaviour, because it becomes difficult to reason about.

[MylesBorins]

This is going to need a rebase

I'm -1 on calling them source text module records. That is spec text that the average developer is not going to know.

What is the problem with either esm or module?

[GeoffreyBooth]

@devsnek User-facing references to 'esm' were renamed to 'module' a few PRs ago. This PR just updates the last source code references to 'esm' correspondingly, as you requested. --type was also added in a previous PR.

We had a lot of discussion on the naming and we settled on --type=module / "type": "module" to match browsers’ <script type="module">. Users have been familiar with the latter since September 2017, and have been using the package.json "module" key for years before that, so we feel that this is what makes the most sense from a users’ perspective.

[GeoffreyBooth]

@MylesBorins It’s been rebased.

@devsnek If you’d like to propose renaming some things, can that be a separate PR? Since the names are already in the implementation; this PR doesn’t add any new ones. If you come up with better names that the group approves, we can change them via your PR.

Based on the research I’ve done so far, .js as ESM is very popular and will likely remain so. I think users will appreciate a -m short flag.

[devsnek]

@GeoffreyBooth -m (sort of like --type, because this entire system is poorly designed) doesn't tell me anything about what it is doing. I don't think it's acceptable to add options that are this generic which also modify so much of how code is run.

[ljharb]

Assuming the OP is exhaustive about the changes in this PR, this seems good to land.

😢

[GeoffreyBooth]

Per meeting on 2019-02-27, landing on 2019-03-01

[MylesBorins]

Unfortunately this PR landed in a way that broke CI (both linting issues as well as broken tests on windows).

I've gone ahead and reverted modules-lkgr back to the state before this landed and backed up the original branch in https://github.com/nodejs/ecmascript-modules/tree/modules-lkgr-2019-03-05-backup

We will work in #49 to get things working again

[GeoffreyBooth]

That's weird, I ran the tests and make lint before submitting this. Is there a way to trigger a CI build before merging in?

[MylesBorins]

@GeoffreyBooth there were unfortunately linting errors that were still broken...

we have for other prs run ci.nodejs.org against it... although only node.js core collaborators have access. It isn't a big deal, we'll get this fixed quickly, just documenting