Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

Support esm in .js #91

Open
wants to merge 301 commits into
base: latest
Choose a base branch
from
Open

Support esm in .js #91

wants to merge 301 commits into from

Conversation

tbranyen
Copy link

@tbranyen tbranyen commented Oct 2, 2017

For your consideration:

This modifies an early ESM implementation bmeck/node@e47459c to not exclude .js files from ESM. This maintains complete backwards-compatibility with .mjs. It is created as a demonstration that .mjs should not be a hard requirement to use ESM, as it introduces needless friction to the existing environment.

I introduce two command line flags:

--esm - Parse the initial script as ESM
--cjs - Parse the initial script as CJS (default)

Use of the --esm flag will parse the script as ESM supporting import and allowing the user to import CJS files within the limitations of Node's implementation. Currently all of core is available for named imports. Users can require CJS scripts by importing the require function from the module core module.

import { require } from 'module';

Notes on Relative CJS from ESM:

Supports relative require calls when called from an ES Module. This was temporarily implemented using a hack where the require fn is called, then the parent caller is matched from the resulting error call stack, then a new require path is formed from the normalized. This is working with simple examples, but is obviously not a long-term solution. The fix is using import.meta to get a caller path and matching relative to that.

Limitations:

  • The aforementioned hack for relative path resolution
  • .mjs will allow arbitrary execution of node script.js, this
    approach requires the end user to specify what to start with, which can
    be problematic for CLIs, although they can use .mjs in this case to
    make the distinction.

Benefits:

  • Separation of module systems, import for ESM/or core, require for CJS
  • Greater compatibility with existing tooling and the web
  • Don't need to switch existing projects/tooling to support .mjs
  • Use of require for CJS in ESM, opposed to always using import
    despite its numerous limitations with CJS compatibility
  • Compatibility with .mjs, and still allows the benefits of .mjs
    without making it a hard requirement
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
  • module (Updated to include new require function in ESM)

jasnell and others added 30 commits August 24, 2017 13:25
Expand definition of Collaborator to include individuals
with commit access to any Node.js GitHub repository.

Clarify the kinds of things that should be considered when
considering inviting new collaborators

PR-URL: nodejs/node#14981
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Some errors in buffer module losed some arguments or received
wrong arguments when they were created. This PR added these
losing arguments and fixed the wrong arguments.

PR-URL: nodejs/node#14975
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs/node#14974
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#14971
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: David Cai <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#14928
Ref: nodejs/node#12376
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Original commit message:

    Work around glibc thread-local storage bug

    glibc before 2.17 has a bug that makes it impossible to execute
    binaries that have single-byte thread-local variables:

        % node --version
        node: error while loading shared libraries: cannot allocate
        memory in static TLS block

    Work around that by making the one instance in the V8 code base
    an int.

    See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898
    See: nodesource/distributions#513
    See: nodejs/build#809
    Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385
    Reviewed-on: https://chromium-review.googlesource.com/612351
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Eric Holk <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#47400}

PR-URL: nodejs/node#14913
Ref: nodejs/build#809
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Nikolai Vavilov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Promise is implemented as a pair of objects. `napi_create_promise()`
returns both a JavaScript promise and a newly allocated "deferred" in
its out-params. The deferred is linked to the promise such that the
deferred can be passed to `napi_resolve_deferred()` or
`napi_reject_deferred()` to reject/resolve the promise.

`napi_is_promise()` can be used to check if a `napi_value` is a native
promise - that is, a promise created by the underlying engine, rather
than a pure JS implementation of a promise.

PR-URL: nodejs/node#14365
Fixes: nodejs/abi-stable-node#242
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
It may not return random bytes right away, but when called
asynchronously it will not block.

PR-URL: nodejs/node#14993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs/node#14992
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Not knowing which APIs use libuv's threadpool can lead to surprising
performance problems. Document the APIs, and also document
UV_THREADPOOL_SIZE, which can be used to fix problems.

PR-URL: nodejs/node#14995
Reviewed-By: Brian White <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Preserve indentation for multiline strings, objects that span multiple
lines, etc.

also make groupIndent non-enumerable

Hide the internal `groupIndent` key a bit by making it non-enumerable
and non-configurable.

PR-URL: nodejs/node#14999
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
The returned chunk is *never* longer than `size`.

PR-URL: nodejs/node#15014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
This changes the error handling model of ServerHttp2Stream,
ServerHttp2Request and ServerHttp2Response.
An 'error' emitted on ServerHttp2Stream will not go to
'uncaughtException' anymore, but to the server 'streamError'.
On the stream 'error', ServerHttp2Request will emit 'abort', while
ServerHttp2Response would do nothing

See: nodejs/node#14963

PR-URL: nodejs/node#14991
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs/node#14885
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
The benchmark script for dns contained functions with args declared
but never used. This fix removes those arguments from the function
signatures.

No test existed for the dns benchmark so one was added to the
parallel suite.
To improve performance the tests are limited to 1 invocation to a
single endpoint.

PR-URL: nodejs/node#14936
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: nodejs/node#14997
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
This will allow `localAddress` to be properly set before writing
debug output.

PR-URL: nodejs/node#12616
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
This test now prints out some child error output if the npm child proc
fails, allowing us to debug easier.

PR-URL: nodejs/node#12490
Refs: nodejs/node#12480
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit 4ca8ff2.

That commit was landed without a green CI and is failing on Windows.

PR-URL: nodejs/node#15047
Reviewed-By: James M Snell <[email protected]>
Add `SIGTRAP` to allowed signals (seen on PPC machines in CI).

Improve message when assertion fails in test-abort-uncaught-exception by
providing the signal name that was not expected.

PR-URL: nodejs/node#14013
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Improve error message by showing output when frames output does not meet
expectations.

Since we can't tell at runtime if we have the correct libc for
backtraces, allow an empty backtrace and run the test on all platforms.

PR-URL: nodejs/node#14013
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Currently, tests in test/abort do not run in CI.

This change configures the test runner to not write core files for abort
tests and to run them.

PR-URL: nodejs/node#14013
Fixes: nodejs/node#14012
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
PR-URL: nodejs/node#15003
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#14807
Fixes: nodejs/node#12802
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs/node#14394
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Add -i option for release.sh that allows users to specify
non-default private key for ssh and scp commands.
Change argument parsing to getopts.

PR-URL: nodejs/node#14401
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
PR-URL: nodejs/node#14659
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
This changes the error handling model of ServerHttp2Stream,
ServerHttp2Request and ServerHttp2Response.
An 'error' emitted on ServerHttp2Stream will not go to
'uncaughtException' anymore, but to the server 'streamError'.
On the stream 'error', ServerHttp2Request will emit 'abort', while
ServerHttp2Response would do nothing.
It also updates respondWith* to the new error handling.

Fixes: nodejs/node#14963
PR-URL: nodejs/node#14991
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs/node#14716
Refs: nodejs/node#14332
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
PR-URL: nodejs/node#14204
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@tbranyen
Copy link
Author

tbranyen commented Oct 2, 2017

I am stating that it doesn't make sense, because it must be a new function that is strictly not equal for every file that imports it.

Not sure why you think that'll be the case, it isn't how my implementation works. There is a single require function for all ES modules. And they are strict equal.

@ljharb
Copy link

ljharb commented Oct 2, 2017

@tbranyen requires are relative based on the filename in which they're used; that's why each CJS file in node has a different require function. How do you suggest the same JS function - something that can be passed around as a first-class value - would be able to be called yet have different contextual behavior? (this is why import.meta is syntax; because it's impossible to do this cleanly with API)

@tbranyen
Copy link
Author

tbranyen commented Oct 2, 2017

@ljharb please read my PR and the hack I employ. It works "fine" for simple cases, require function that inspects the stack and makes relative requires work with a single function reference. I'm not proposing this lands as-is, but it's a starting point for discussion. Once import.meta lands we can revisit and figure out a better way to integrate with that, I expect you are correct that multiple require functions will be necessary then to ensure the require function can be deconstructed from the import.meta object and used detached.

@Qard
Copy link
Member

Qard commented Oct 3, 2017

I'm wondering if import { require } from "module" could just give you a special non-relative require rooted at the location of the entry file. I doubt many would be using a mix of ESM and CJS in their own code, they'd likely just be using the require to load stuff from node_modules.

@jkrems
Copy link
Contributor

jkrems commented Oct 3, 2017

The import-based alternative for import.meta discussed before was import * as meta from 'js:context'. In theory that could be used to get a module-relative require for the current module by "simulating" import.meta.require (import { require, url } from 'js:context' mapping to const { require, url } = import.meta). But anything like this risks poisoning the ecosystem with legacy code.

That being said, introducing an official require inside of ESM that works differently than import.meta.require might would also risk introducing bad expectations into the ecosystem.

@tbranyen
Copy link
Author

tbranyen commented Oct 3, 2017

If folks are willing to take a slight performance penalty, and the caveats around reading the calling stack to determine relative positioning, we technically already have a solution that works. I'm just a bit worried I went off the deep end and that this could break in ways I'm not forseeing.

Effectively this is what I did:

At process start:

  • makeRequireFunction() and attach to require('module')
  • Get the cwd and associate that as the process root

Inside an ES module:

  • Leverages the work @bmeck has done to expose it as a named export import { require } from 'module'

When invoked:

  • Set up the node paths to try and load from the process CWD or node_modules
  • If this fails (relative require) look at the exception trace and find which module request it
  • Craft a relative path based off this information and require again (this time it is cached in require.cache)

I think this is a pretty acceptable overhead given the fact it's a temporary hold over until more code becomes ESM.

@Qard
Copy link
Member

Qard commented Oct 3, 2017

I suspect there's going to be a lot of code that never makes that transition. People aren't going to rewrite their code unless they have a really solid reason to.

@bmeck
Copy link
Contributor

bmeck commented Oct 3, 2017

Leverages the work @bmeck has done to expose it as a named export import { require } from 'module'

I have not worked on the require named export, just the creation of the named export facade which it uses under the hook.

@tbranyen
Copy link
Author

tbranyen commented Oct 3, 2017

Sorry @bmeck, I only meant that the facade work allows for the syntax I've suggested, without your work this would not function.

@bakkot
Copy link

bakkot commented Oct 6, 2017

I don't entirely understand what this PR proposes to do. Would you mind going through of these scenarios and explaining what happens in each one? (There ended up being more than I expected, sorry.)

Throughout, please assume global is the global object no matter the parse goal.

Suppose

  • common.js contains console.log(require('module.js'));, and
  • module.js contains export default 0;.

What happens when you do node --cjs common.js? What about node --esm common.js?

Suppose

  • common.js contains import('module.js').then(ns => console.log(ns));, and
  • module.js contains export default 0;.

What happens when you do node --cjs common.js? What about node --esm common.js?

Suppose

  • module.js contains import { require } from 'module'; require('common.js'); console.log('global.common');
  • common.js contains global.common = typeof require === 'function';, and

What happens when you do node --esm module.js?

Suppose

  • module.js contains import 'common.js'; console.log('global.common');
  • common.js contains global.common = typeof require === 'function';, and

What happens when you do node --esm module.js?

Suppose

  • module.js contains import 'ambiguous.js'; console.log(global.strict);, and
  • ambiguous.js contains global.strict = this === void 0;.

What happens when you do node --esm module.js?

Suppose

  • common.js contains require('ambiguous.js'); console.log(global.strict);, and
  • ambiguous.js contains global.strict = this === void 0;

What happens when you do node --cjs common.js?

Suppose

  • common.js contains import('ambiguous.js').then(() => console.log(global.strict));, and
  • ambiguous.js contains global.strict = this === void 0;

What happens when you do node --cjs common.js? What about node --esm common.js?

Suppose

  • module.js contains import 'ambiguous.js'; import { require } from 'module'; require('common.js'); console.log(global.strict);,
  • common.js contains require('ambiguous.js'); console.log(global.strict);, and
  • ambiguous.js contains console.log('executed'); global.strict = this === void 0;.

What happens when you do node --esm module.js?

Suppose

  • module.js contains import { require } from 'module'; require('common.js'); import('ambiguous.js').then(() => console.log(global.strict));,
  • common.js contains require('ambiguous.js'); console.log(global.strict);, and
  • ambiguous.js contains console.log('executed'); global.strict = this === void 0;.

What happens when you do node --esm module.js?

Suppose

  • common.js contains require('ambiguous.js'); import('module.js').then(() => console.log(global.strict));, and
  • module.js contains import 'ambiguous.js'; console.log(global.strict);,
  • ambiguous.js contains console.log('executed'); global.strict = this === void 0;.

What happens when you do node --cjs common.js?

@WebReflection
Copy link
Contributor

WebReflection commented Oct 13, 2017

As a developer, I'd like to answer with my expectations about his PR. Since we've slightly discussed this already and we seem to be on the same page, I hope this answers for @tbranyen too.

I am assuming all questions are about local files in the same project, not npm published modules, and with an implicit ./ file descriptor (that's missing and might throw in ESM)

Point 1

What happens when you do node --cjs common.js? What about node --esm common.js?

Project published in npm after transpilation are 100% CommonJS and require based.
The first point is a non-real-world use case.

Being non existent issue, it's straight forward to solve:

  • if a CJS module uses import it should throw
  • if an ESM module use require without importing it from module, it should throw

The same goes for pretty much every other case.

If you use standrd ES2015 module features you are running ESM, otherwise you are running CJS wannabe code that need transpilation.

This is already covered by runtime/post-production tools such Babel so ...

Point 2

What happens when you do node --cjs common.js? What about node --esm common.js?

throws in the --cjs case, works as --esm.

Point 3

What happens when you do node --esm module.js?

There's probably a typo there, it should be console.log(global.common); with no quotes.
The result is true because the module is being loaded as CJS.

It will also have __filename and module and all other CJS things.

Point 4

It has probably the same console.log(global.common) with quotes typo but the result would be false.

Why? Because it's loaded as ESM. It does nothing to break standard ES so it just works as such.

Point 5

Again, it's very simple: you load as ESM, the file is imported as ESM. You load as CJS, the file is imported as CJS.

This is the exact same of having the file duplicated as both .js and .mjs and importing it with different extensions. Since nobody on earth will maintain two separate extensions manually, it's a non issue.

The result? true like an ESM would do.

Point 6

Same as above. Result is false.

Point 7

There's no import(...) in CJS. import(...) is an ES2015 standard so ESM works, CJS throws.

Point 8

Another non-existent use case. You are loading same project files, you are shooting your foot if you have both CJS and ESM source code. Nobody does that already today.

Regardless, the result should be true because ESM loads upfront and same module twice means only one load.

Arguably the CJS require for a module already loaded, hence valid, as ESM, should simply throw.
Thanks gosh this use case is nonexistent in the real-world.

Point 9

require is synchronous, import(...) is not. That's a false and a thrown error on import(...).

You shouldn't import the exact same file twice, once in CJS and once in ESM world, because the whole point of this issue is that these two environments are not equivalent/compatible.

Thanks gosh this won't even happen on the real world.

Point 10

There is no import(...) in CJS. import(...) is an ESM standard.

@bmeck
Copy link
Contributor

bmeck commented Oct 13, 2017

@WebReflection import() is in all grammars of ECMA262. Its placement is valid in any CallExpression location

@WebReflection
Copy link
Contributor

WebReflection commented Oct 13, 2017

@bmeck so is import ... it's part of the specifications.

I thought the point here is to clearly differentiate as explicit flag intent between CJS and ESM, and I've answered to that.

In CJS modules nobody needs or uses import, or at least that's what I've been told when I've proposed asynchronous module.import(...).then(...) which they instantly killed (remember? it's been long time since I've started warning the community there were issues with using new standards in non standards such CJS files)

screenshot from 2017-10-13 15-42-23

Accordingly, keep import to ESM world and you've solved everything like I've done answering those questions.

NodeJS should fade away from CJS, not work around its limitations, and in doing so, the .js is the extension NodeJS should adopt, instead of diverging from it.

Developers behind tooling won't need to do anything until they decide to embrace standards, developers that want to adopt ESM and still use legacy CJS via npm, where modules are already published as CJS, can finally do that without compromising the language with two unnecessary extensions: the "it's maybe ES compatible" and "it's totally ES compatible" one.

This PR is the best migration pattern out there, it should land in core ASAP behind flags, IMO.

@bmeck
Copy link
Contributor

bmeck commented Oct 13, 2017

@WebReflection the fact remains that import() is valid syntax in CJS (heck it is even valid in eval) but static import declarations are not valid syntax in CJS. I don't understand your initial statement. Nor do I understand your claim that nobody needs or uses a syntax that hasn't even landed into an environment yet but has been standardized in the ECMA262 spec.

Accordingly, keep import to ESM world and you've solved everything like I've done answering those questions.

This pull request continues to use import() within CJS as a means of loading ESM. import certainly exists in CJS for that point. I am not sure your statement is true nor your claim that you have answered all the questions. You have made statements yes, but you seem to quickly state that your statement is truth while we are talking about alternatives and collisions. There are many paths that can be taken and none invalidate all others. However, a choice still must be made to take a single path and standardize upon it.

import() also introduces a problem with this approach as valid ESM workflows have been expanded by it to dynamic behavior that is also valid in CJS:

async () => {
  const global = this;
  global.foo = 123;
  import('thing-using-foo');
}

Has 3 different evaluation results between Script, Module, and CJS. I suspect there will be increased situations where source code becomes ambiguous over time when reading other peoples files. In the case of the file above it would be necessary to state how to consume the file as it may not be obvious. This problem already exists with .js representing a multitude of goals, some of which can't be loaded in specific ways just as CJS cannot be the src of a <script> tag.

Continuing the pattern of deferring to how modules are consumed prevents authors form being able to reliably trust that their even module works. This means they will need to do feature detection at runtime if they wish to be consumed in multiple ways and avoid any syntax that is invalid between goals.

I do think you have an approach that is able to be implemented, but I do not agree with your conclusions about it being the right approach due to a variety of concerns between teaching, migration planning, and increased need for context when reading files.

@WebReflection
Copy link
Contributor

WebReflection commented Oct 13, 2017

This pull request continues to use import() within CJS as a means of loading ESM.

good, I'm OK with that, 'cause I've never used/needed import(...) in CJS.

Please find me a published NodeJS npm module that uses it.

There are many paths that can be taken and none invalidate all others.

Assuming CJS uses the mechanism provided by CJS to load CJS and not import(...) as either statement or callback, there's never ambiguity in any of my answers.

If you find an ambiguous solution please expose it to me. I am saying: import and export is to ESM what require and module are to CJS.

Keep it this way and you can answer, like I've done, to all those questions.

What is the purpose of using import(...) in NodeJS ?

If that's really needed you Promise.resolve(require(...)).then(...) and you have an asynchronous import, as suggested in my old PR about having module.import(...) async.

Has 3 different evaluation results between Script, Module, and CJS.

No. It has exactly what the flag you used expect. You node --esm ? that's ES2015. You node --cjs ? you have to refactor that import into a require or it should throw, or load always ESM.

I'm fine with import(...) loading always as ESM. I can rewrite/edit my answers if needed.
The default should be ESM, not CJS.

I suspect there will be increased situations where source code becomes ambiguous over time when reading other peoples files.

I suspect that's the case indeed with .mjs, nobody from a generic source code could tell anymore if that's supposed to be valid ES2015 or not.

Keeping .js though, you have tooling that solved already the burden for everyone, a single extension that communicates you are writing JavaScript in there, and a clear distinction between require for legacy modules, and import for ES2015+ files: zero ambiguity.

With .mjs there's ambiguity as soon as you encounter a perfectly valid JS file used in every other environment that will fail even if it's fully standard syntax as specified by ES2015.

This problem already exists with .js representing a multitude of goals

This proposal solves that issue: node --esm is a crystal clear goal

SpiderMonkey has the same via -m

JSC is so ahead of time it just goes standard and ESM imports as goal by default.

Continuing the pattern of deferring to how modules are consumed prevents authors form being able to reliably trust that their even module works.

That's the current status with .mjs. Authors omit the extension and let tooling or Node solve the issue, causing themselves problems in env they don't control. I've already shown an example about this pattern easily breaking. That's how today everyone behind tools writes JS.

I do think you have an approach that is able to be implemented

Zero ambiguity is my goal. Fade away CJS, embrace ES2015+. It's already possible, we just need to punish those that never cared 'cause tools solved for them.

The day they'll start caring about the standard is the day they'll refactor few requires here and there, not a big deal, maybe they'll learn the story.

but I do not agree with your conclusions about it being the right approach due to a variety of concerns between teaching, migration planning, and increased need for context when reading files.

nothing to teach, the migration plan is highlighted by your editor without needing to read the file extension so it's a win for reading files too.

Today? Everyone believes they are using standards and Babel fixes everything, that's it.

And that is IMO what we should fix in the JS community.

@bmeck
Copy link
Contributor

bmeck commented Oct 13, 2017

Please find me a published NodeJS npm module that uses it.

I can't because no environment supports it. Babel and Webpack based compilation do not implement it as spec so they also don't support it. However, people are using it in modules today.

This discussions are about impact on existing modules and future planning, stating that something is not present in the ecosystem today is not relevant. The purpose in developing specifications is to plan how they will be used in the future. That directly has a relation to backwards compatibility though.

Assuming CJS uses the mechanism provided by CJS to load CJS and not import(...) as either statement or callback, there's never ambiguity in any of my answers.

If you find an ambiguous solution please expose it to me. I am saying: import and export is to ESM what require and module are to CJS.

This is an attempt to state that mine is ambiguous compared to yours. We have different forms of ambiguity here. On the side of import() should be able to load anything, the consumer/caller does not know the format of the module be it WASM/CJS/etc.; on the side of import() only loads ESM and require() only loads CJS, the author of the dependency does not know if how it will be loaded will cause problems with ambiguous texts like the one I previously showed. Therefore they need to guard against those situations.

I am not sure stance you are trying to produce from these statements, but there certainly is ambiguity in both.

What is the purpose of using import(...) in NodeJS ?

To load modules using ECMA262 provided mechanics and produce a Module Namespace, there may be multiple types of modules that it can load.

If that's really needed you Promise.resolve(require(...)).then(...) and you have an asynchronous import, as suggested in my old PR about having module.import(...) async.

This has different mechanics and would not be going through the same pipeline as import(); I cannot think of a clean way to implement loader hooks across both CJS and ESM with them being separated like this.

No. It has exactly what the flag you used expect. You node --esm ? that's ES2015. You node --cjs ? you have to refactor that import into a require or it should throw, or load always ESM.

This is false and based upon unknown environment context. My example does not include a CLI flag, as the author of that text you do not know.

I'm fine with import(...) loading always as ESM. I can rewrite/edit my answers if needed.
The default should be ESM, not CJS.

I am not clear here on what is being discussed, but the default is ESM in the current --experimental-modules which prioritize .mjs.

I suspect that's the case indeed with .mjs, nobody from a generic source code could tell anymore if that's supposed to be valid ES2015 or not.

This is the point of the usage restriction in the internet draft.

With .mjs there's ambiguity as soon as you encounter a perfectly valid JS file used in every other environment that will fail even if it's fully standard syntax as specified by ES2015.

I can actively point them to the internet draft as being out of line. .js currently has CJS and Script as the main targets of it's content, and some people are using it for Module. We can never fully prevent language extension or tooling, but we can have our environment/standard state exactly how a module will be handled with a given extension. This lets tools unambiguously write files and have some sense of safety that the environment will load them correctly. The MIME update included goal as a parameter in part to allow this guard to trickle into the browser.

That's the current status with .mjs. Authors omit the extension and let tooling or Node solve the issue, causing themselves problems in env they don't control. I've already shown an example about this pattern easily breaking. That's how today everyone behind tools writes JS.

Correct to an extent. Like all new things, you need to upgrade your environments for support of features. We still haven't fully specced out how the web intends to handle path aliasing or bare imports. I am in no rush to make statements about their plans, but am curious how things will work out.

Zero ambiguity is my goal. Fade away CJS, embrace ES2015+. It's already possible, we just need to punish those that never cared 'cause tools solved for them.

I seem to disagree since ambiguity is present always in import since it can load multiple types of modules. The way this conversation has been going I think your goal is to use your solution without discussing other solutions or fixing problems with other solutions. There seems to be a hard set approach that you have decided upon, and I will happily help ensure that it works with loaders.

The day they'll start caring about the standard is the day they'll refactor few requires here and there, not a big deal, maybe they'll learn the story.

I don't understand this. It is claiming that the standard is not cared about today?

nothing to teach, the migration plan is highlighted by your editor without needing to read the file extension so it's a win for reading files too.

The migration plan keeps .js as both CJS and ESM. That leaves a lot to teach and perpetuates using both module systems without a clear way to state the intent of how to evaluate a .js file. It relies upon the consumer to learn the behavior of any given module and the author to guard their behavior against potentially being used improperly.

@WebReflection
Copy link
Contributor

However, people are using it in modules today.

of course, because Webpack split modules for browsers if you use import. So it's not NodeJS code that is involved, it's browser-land that will still use Webpack to split up bundles.

The Web will keep using bundlers, at least in production, with or without .mjs official support.
Bundlers solve every problems you are talking about upfront, so browsers using import(...) when they mean the ES2015 imprt(...) are just fine.

stating that something is not present in the ecosystem today is not relevant

it is. import(...) outside CJS is already possible. You said nobody uses it, because they use bundlers or tooling to solve a different issue. NodeJS does not need import(...) and if today is not using it, that's perfect because tomorrow, moving forward here, will simply use ES2015 which enables import(...) as always ESM, answering this other sentence:

The purpose in developing specifications is to plan how they will be used in the future.

node --esm is the future and import(...) works out of the box there.

the author of the dependency does not know if how it will be loaded will cause problems with ambiguous texts like the one I previously showed.

They have everything they need to avoid issues. They have samever based packages, tooling to help them use the right loader for the right package.

They need to know regardless what they are importing because CJS exported after ESM break in ESM once imported transpiled.

Can we plese starrt checking what exhaustively already discussed?

Because of automatic extension insertion, NodeJS is making imports ambiguous. Authors need to know what they are importing because they might need to explicitly opt in for .default.

We've talked about this already. .mjs does not solve this issue.

Therefore they need to guard against those situations.

.mjs does not guard against anything if imports are ambiguous or auto-resolved.

To load modules using ECMA262 provided mechanics

Excellent, then go full ES2015 if that's your intent. node --esm is the way.

I cannot think of a clean way to implement loader hooks across both CJS and ESM with them being separated like this.

Perfect, then don't. If I node --esm I won't need any require hook unless I explicitly opt in for it via import {require} from 'module';

If I use that, the only hook I need would be CJS one, 'cause the module is a CJS one.

You need to think inside-out. The intent is explicit in the source code. That's literally it. If the goal is not satisfied, it fails.

we can have our environment/standard state exactly how a module will be handled with a given extension.

Precisely. The NodeJS environment standard has been require and module for years. Why is NodeJS trying to appropriate of the ES2015 standard which is not its own specific domain?

Use require for CJS, import for ESM. Problem solved, nobody gets hurt.

The MIME update included goal as a parameter in part to allow this guard to trickle into the browser.

The goal should never be a file extension, rather a user intent.

If I double click on a file my OS tries to find the best program, or the instrumented program, to open such file. If I right click and open with another program though, that just happens: it's my explicit intent.

If I decide to register .js extension to open with Firefox instead of an editor, that's my specific intent.
The OS never steps over, it does what I tell it to do. Some default is OK, but explicit intents come from me, not from OS idea of what I expect. Do you follow?

We still haven't fully specced out how the web intends to handle path aliasing or bare imports

You don't have to. Every JavaScript valid MIME served file will do. That's .js in 100% of web servers out there already. The Web supports .js files as modules.

import since it can load multiple types of modules.

No. import should always load ESM. That's what it does in browsers, SpiderMonkey, and JSC too. Stick with ecosystem de-facto standards.

Who uses import(...) today uses tools. Tools have no issue, tools are OK.

NodeJS shouldn't act like a tool. It should just work in a never ambiguous way.
That's it: import for ES2015+, require for CJS.

No exceptions beside those that make sense like .json or .wasm. CJS shuold never be able to require("m.wasm") as example, or you promote non standards.

Remove CJS goal from anything defined in ES2015+ and you have solved all the ambiguities.

Let tools do their job. Let developers that setup those tools migrate with the pace they want.
Avoid magic in NodeJS imports and be always explicit with file names.
Enable ES2015+ and the ability to include require and not a single problem would exist anymore.

The migration plan keeps .js as both CJS and ESM.

to keep .js as JavaScript, a programming language defined by ECMAScript, not by NodeJS.

That leaves a lot to teach and perpetuates using both module systems without a clear way to state the intent of how to evaluate a .js file.

Which part is not clear?

CJS

// I WANT CJS
require('./file');

ESM

// I WANT ES2015
import app from './my-app.js';

What is not clear, if you remove CJS goals from any ES2015 explicit intent?

Drop CJS goal from any import, import(...) or export explicit developer intent when you start node --esm and forget about .mjs issues because none would ever exist.

The only missing bit is the following:

// I WANT ES2015
import app from './my-app.js';

// BUT I ALSO NEED CJS
import {require} from 'module';
require('./file');

@benjamingr
Copy link
Contributor

Hey, thanks for keeping this heated discussion civil.

Just a comment on Promise.resolve(require(...)) vs import - synchronous I/O is the issue here - import does the underlying loading asynchronously which means much better startup performance and old time for apps that load modules lazily - for example an app loading several term dictionaries based on how it is configured. I also know that js-dom wants it for performance related reasons. There was an issue about it - I can dig it up if the above didn’t clarify things.

@tbranyen
Copy link
Author

@benjamingr with this PR, and a v8 that supports it, import().then(...) would be supported in CJS and ESM environments. This would be the intended way to import ES modules asynchronously in either module system. The same is true with require in this pull request. It works synchronously to only load CJS modules in either environment.

A commonly repeated MJS argument is that it is simply a file extension and that the community will catch up to it, with tooling and modules. That is true, but there is more to the implementation in that it allows for importing CJS with ESM semantics. You will then hear about how import will load WASM and HTML Modules as justification for CJS. Those targets can be designed to work with ESM first-class. The same cannot be said of CJS yet, unless major concessions are made.

@bmeck
Copy link
Contributor

bmeck commented Oct 15, 2017

@tbranyen

@benjamingr with this PR, and a v8 that supports it, import().then(...) would be supported in CJS and ESM environments. This would be the intended way to import ES modules asynchronously in either module system. The same is true with require in this pull request. It works synchronously to only load CJS modules in either environment.

This encouragement to continue using non-standard syntax when writing new code is part of why I do not think it is a good idea.

A commonly repeated MJS argument is that it is simply a file extension and that the community will catch up to it, with tooling and modules. That is true, but there is more to the implementation in that it allows for importing CJS with ESM semantics. You will then hear about how import will load WASM and HTML Modules as justification for CJS. Those targets can be designed to work with ESM first-class. The same cannot be said of CJS yet, unless major concessions are made.

These module types are all looking to use MIME/file extension. This statement is misleading about there needing to be different concessions for what loading other file types means. "work with ESM first-class" in particular is misleading because every module type is just needing to define what to create a Module Namespace Object means when loading that module type. Defining a well known procedure for CJS is no different than anything listed here.

@WebReflection
Copy link
Contributor

WebReflection commented Oct 15, 2017

@benjamingr the import(...) is an ES2015 standard. It works well already on the web, it imports ES2015 modules, and here we all agree it should continue doing so.

Although, with --esm flag, import(...) should never be compatible with CJS, because otherwise we are doing exactly what @bmeck is trying to prevent:

This encouragement to continue using non-standard syntax when writing new code

This is why we should put a line for developers: import is for ES2015, require is for CJS.

In this way they'll migrate, in any other way they'll continue doing whatever they are doing: omitting the extension because .mjs and the current import type ambiguity is encouraging this.

These module types are all looking to use MIME/file extension.

OK then, tell me what's the difference between text/javascript for .mjs and text/javascript for .js ... 'cause I'm sure my browser doesn't change behavior a single bit with these two extensions.

"work with ESM first-class" in particular is misleading because every module type is just needing to define what to create a Module Namespace Object means when loading that module type.

The only module type allowed in ES2015+ is the one defined by ECMAScript.

Get CJS out of the equation or you'll inevitably bury all your good intents way before it'll ever disappear as module type for Node.

Go full ESM when required and see the migration happening.

Neither me nor you have crystal balls, and yet all the polls I've seen, all the voting on GitHub, and all the people I've talked to, would prefer keeping .js and explicitly opt in for ESM.

@bmeck
Copy link
Contributor

bmeck commented Oct 15, 2017 via email

@WebReflection
Copy link
Contributor

Browsers dont care about file names so .mjs and .js are equivalent in their mechanics.

same goes for SpiderMonkey and JSC

The goal parameter of text/javascript is defaulted to module when loading via a Module.

same should happen when the engine is asked to import a text/javascript file. It's like the right click on a file and the explicit open with mechanism. It worked well for every Operating System to date.

Scripts in browsers have no mandated MIME so with <script type=text/javascript> no MIME is comin from the attribute and it is defaulted to text/javascript with goal being defaulted to script.

But <script type=module> is a clear intent as much as import is when it comes to ESM.
This works already for the browser, it works also very well in node using this proposal.

Serving Script with MIME like text/html is perfectly valid.

If the intent is clear, it should be OK for any engine to load whatever file name as module too.

Being in JS the JS itself the only reasonable default target, if that file won't result into a valid JS code with goal module then it will fail as expected.

File extensions are, and always will be, a convention, nothing else.

Having one programming language under one file extension is what every other programming language has so please, let's not make JS a joke, it's been derided already for a way too long time.

@ljharb
Copy link

ljharb commented Oct 15, 2017

The language has two parse goals. Extensions determine parse goal for textual formats. The only way JS could retain one extension is if instead of two goals, it was two modes in one goal - i.e., unambiguous parsing. Browser reps on TC39 blocked that change (and presumably would have blocked it pre-ES6 too, for the same reasons which aren't relevant here), thus, two file extensions have always been inevitable.

(It's also worth noting that had python 3 had a different file extension than python 2, a "python 3 situation" might not have entered the lexicon)

@WebReflection
Copy link
Contributor

WebReflection commented Oct 15, 2017

unambiguous parsing

As already discussed in this PR, that's done here.

You require CJS and you import ESM.
It's 100% unambiguous in both developer intent and parsing goals dictated by standard syntax.

two file extensions have always been inevitable

As already discussed in this PR, that's not the case via node --esm.

It's also worth noting that had python 3 had a different file extension than python 2, a "python 3 situation" might not have entered the lexicon

Python 3 migration problems has nothing to do with file extensions, it's about syntax.

Here we are promoting exact same syntax for both .js and .mjs and the devil hides in the missed import extension detail that makes .mjs imports ambiguous.

Python 3 syntax breaks on Python 2 (and vice-versa)
But .mjs syntax won't break on .js (or vice-versa).

Nobody is proposing new syntax here so ... a new extension using Python as previous case, makes even less sense.

@ljharb
Copy link

ljharb commented Oct 16, 2017

Modules do have different syntax than Scripts, as well as different semantics.

An important requirement is that the developer not have to know the module format of the module being imported or required. Forcing one mechanism based off the module format violates this constraint.

@WebReflection
Copy link
Contributor

WebReflection commented Oct 16, 2017

Modules do have different syntax than Scripts, as well as different semantics.

absolutely no. A module can be a polyfill with side effects on the global scope.

CJS and ESM don't need to export a thing and use same syntax.

An important requirement is

who decided the requirement? if Node users, apparently most of them are not happy with the current status.

the developer not have to know the module format of the module being imported or required.

that's impractical in the real-world and, accordingly, a made-up problem that doesn't exist.

You need to know if you are importing an ESM module or a CJS one or a transpiled one, because you eventually need to address the .default if that's the case.

Node adding extension implicitly when omitted is the source of ambiguity.

This also has been discussed numerous times already.

Forcing one mechanism based off the module format violates this constraint.

.mjs forces the module format too so this is a made up problem.

Since I don't think anyone here wants me to repeat all the answers, to avoid repeating everything discussed already please read the conversation from the beginning.

Thank you

@jamesplease
Copy link
Contributor

jamesplease commented Jan 13, 2018

This PR is one piece of npm’s proposal to add ESM support in Node.

All I can say is if npm supports this then we must be onto something here 😉

npm’s Node branch with this change (among others): https://github.com/chrisdickinson/node-1/tree/you-cant-spell-ex-mea-sententia-without-e-s-m

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.