nodejs / node-eps

Node.js Enhancement Proposals for discussion on future API additions/changes to Node core
442 stars 66 forks source link

.mjs extension trade-offs revision #57

Closed YurySolovyov closed 6 years ago

YurySolovyov commented 7 years ago

I obviously can't speak for the whole community, but it seems like a lot of people are not happy with .mjs.

One of the main arguments to keep .js is that if we can detect 99% of cases where we CAN tell if is it CJS or ESM (or where we just know what to do), we may just call rest 1% edge cases and deal with it.

We can even come up with some linter rules and/or workarounds to simply teach people to do the right thing.

pakastin commented 7 years ago

Couldn't main in package.json stay to reference to the commonjs version and module to the ES2015 version? And you could still call require to import cjs modules like it used to be and import to import the new modules?

That's how we do it in client-side as well, so why not in node? https://github.com/redom/redom/blob/master/package.json#L5-L6

ljharb commented 7 years ago

@pakastin require('esm') and import 'cjs' needs to work transparently, so that refactoring a module from one format to the other isn't necessarily a breaking change.

YurySolovyov commented 7 years ago

What about https://github.com/standard-things/esm ? It claims to be spec-compatible. I wonder how many real-world use-cases it covers...

ljharb commented 7 years ago

@YurySolovyov that's intended to be a loader for .mjs in node versions that don't support it yet - iow, a polyfill (which is great!). It has some extra options, sure, but that's not what node's planning to implement.

mrksbnch commented 7 years ago

I don't know if this was already brought up but what about introducing a flag similar to "use strict"? It could be called "use modules" and whenever this is used, all imports are treated as ES2015 module imports? As far as I can tell, this should also work in combination with the .mjs approach (in files where "use modules" is not set).

ljharb commented 7 years ago

@mrksbnch it's been brought up a lot; see #60 for the most recent discussion.

bellbind commented 7 years ago

In the current pull/14369/head branch which enabled import/export features, the loader resolver locates the last resolveRequestUrl function at lib/internal/loader/resolveRequestUrl.js.

My suggestion (in 13 May) of loading local js files as import "./foo.js" with es6 module (not cjs script) is modifying the:

  if (ext === '.mjs') {
    return new StandardModuleRequest(url);
  }

as:

  if (ext === '.mjs' || specifier.match(/^\.\.?\/.+\.js$/)) {
    return new StandardModuleRequest(url);
  }

It can accept ./main.js as es6 module from ./main.mjs as:

// main.mjs
import "./main.js";
// export * from "./main.js"; // otherwise

with command line $ node main.mjs

chee commented 6 years ago

This sounds so silly, but what about if it was .es instead of .mjs

The m is meant to stand for module, right? But node .js files are already CommonJS modules. .mjs doesn't make the distinction clear that these are in the ecmascript module format the way .es does.

We refer to them as ES modules, we have eslint, @std/esm, we regularly use the term es to discuss modern javascript. People are comfortable with the terms es6, es7, es2017.

I truly feel that a .es extension would be accepted without any kind of the same emotional reaction people are having to .mjs

Maybe emotional reactions don't seem important, but they are important. More decisions than we could ever be comfortable with are made from the feelings a person has in their belly

ljharb commented 6 years ago

@chee "ES" is short for "ECMAScript", which (I'm led to believe) is a name that was chosen intentionally to be so ugly as to never be used as the colloquial name for anything, to avoid copyright/trademark issues over "JavaScript".

If the issue were over which file extension was chosen, then I doubt we'd be seeing suggestions for non-file-extension parsing goal differentiation methods; I'd expect extension suggestions, but we've largely seen none.

YurySolovyov commented 6 years ago

I saw this and thought: Why can't this be the default mode?

ljharb commented 6 years ago

@YurySolovyov because it'd break a ton of code. Deviation from established practices have to be opt-in.

bmeck commented 6 years ago

@YurySolovyov that loader is opt-in so it is safe, it also does not change the behavior of other packages.

YurySolovyov commented 6 years ago

image

Then again, what it means:

?

that's intended to be a loader for .mjs in node versions that don't support it yet - iow, a polyfill (which is great!)

I'm not sure I understand this. Polyfill? Ok, tho it seems it solved all the ergonomics problems introduced by .mjs and ESM vs. CJS duality

bmeck commented 6 years ago

@YurySolovyov it achieves the "unlockables" by breaking spec, which we can't do.

bmeck commented 6 years ago

In default config it doesn't do any of those things.

YurySolovyov commented 6 years ago

Ok, then the question is how bad the breakage is, cause if it breaks some silly stuff like var arguments, then it ok to break (and adjust) the spec.

ljharb commented 6 years ago

@YurySolovyov no, if it breaks anything then it is not OK. The only amount of breakage that's acceptable is zero.

bmeck commented 6 years ago

I'm on the side of zero breakage. Various things like named imports from core are doable with Proxies (see branch https://github.com/bmeck/node/tree/named-export-core) but its going to take time to work out those kinks. For userland, you will still need a pragma to stay in spec and have the async timing of parse, link, and eval being separate.

I've been in meetings around changing the spec, but we will work with what we have since browsers have already shipped.

YurySolovyov commented 6 years ago

But using pragma won't break browsers I guess.

ljharb commented 6 years ago

@YurySolovyov if code only worked properly when in Module mode, and node used a pragma but browsers didn't, then that code would silently break in browsers, or worse, do the wrong thing.

bmeck commented 6 years ago

@YurySolovyov if the pragma only exists to live in CJS files, it does not break browsers. I expect it to look something like "use export {default, foo}"; which sets up a Proxy and locks down module.exports so that it can safely emulate a Module Environment Record for those properties.

pakastin commented 6 years ago

I can’t understand why couldn’t we just use require to ”import” commonjs and import for ES2015 modules? There would be no need for confusing .mjs suffix.

If you’d use the wrong kind, you’d get an error message – that’s it!

Why do we need to support importing commonjs with import?

loganfsmyth commented 6 years ago

ES6 modules don't have a require function because that's a feature of CommonJS. If there was no way to import a CommonJS module, your app would essentially have to be 100% CommonJS or 100% ES6, with no way to load one type from the other or transition your codebase to ES6 over time.

chyzwar commented 6 years ago

ES6 modules don't have a require function because that's a feature of CommonJS. If there was no way to import a CommonJS module, your app would essentially have to be 100% CommonJS or 100% ES6, with no way to load one type from the other or transition your codebase to ES6 over time.

Require is just global function in node. There is no reason why it cannot work in ESM. Using import in CommonJS is a problem, but this would not be a common case.

Why do we need to support importing commonjs with import?

Because Node.js CTC believes it is important. It seems that having compatibility with legacy cruft is more important than being spec compliant with JS.

ljharb commented 6 years ago

@chyzwar there is no incompliance with the spec here, and it's not " cruft", it's "basically the entire JS ecosystem".

loganfsmyth commented 6 years ago

@chyzwar

Require is just global function in node.

That is not true. CommonJS modules are automatically wrapped with this function

(function (exports, require, module, __filename, __dirname) { ... })

and executed with their own copy of the require function. This is done so that each require function is aware of the path to the module calling require. If require were global, there's be no way to use relative paths to load modules.

chyzwar commented 6 years ago

@chyzwar there is no incompliance with the spec here,

Node ESM implementation break ECMAScript spec in following ways:

This might break even further if ECMAScript add for example optional typing or any other feature that leverage ESM .

and it's not " cruft", it's "basically the entire JS ecosystem".

CommonJS is only used in the node.js ecosystem. JS ecosystem contains node but it is the order of magnitude larger than just node.js. CommonJS is a just cute hack that was created because JS did not have and module system.

That is not true. CommonJS modules are automatically wrapped with this function

You are describing implementation details of require. It does not change the fact that requires is a global function.

loganfsmyth commented 6 years ago

Node ESM implementation break ECMAScript spec in following ways:

That is not true. These places are defined in the spec explicitly for extension.

By introducing the ability to import CommonJS modules using import

The spec explicitly allows this behavior. The HostResolveImportedModule function, which is implementation-defined, controls translation of a given imported location string into an instance of a module. This can be anything that implements the defined Abstract Module Record interface. The spec defined Source Text Module Record as the implementation of this interface for standard ES6 module syntax, but implementations are entirely welcome to implement alternatives, which is what Node does. This is all well within the spec.

By adding index.js/index.mjs into path resolution algorithm

The ECMAScript spec doesn't define any behavior for path resolution at all. This is 100% performed in the previously-mentioned HostResolveImportedModule function that is implementation-defined.

By creating a custom extension for module type detection.

The ECMA spec does not include any description of file extensions, so nothing about the spec declares .js to exist either. The spec does however describe two independent and incompatible root grammars, which necessitates some way to tell them apart.

By adding the ability to mix strict and non-strict modules.

I'm not sure what you mean with this point. Some elaboration might help?

Possibly breaking in case of cyclic dependencies (not checked)

This one I don't have the background to respond to. Not familiar enough with the gritty details of what Node does here.

This might break even further if ECMAScript add for example optional typing or any other feature that leverage ESM .

Given my description above, as far as I can tell Node is doing it's upmost to conform to the spec.

CommonJS is only used in the node.js ecosystem.

A massive portion of existing npm packages are consumed for client-side use via Webpack and Browserify. The modern JS ecosystem is very built around CommonJS.

You are describing implementation details of require. It does not change the fact that requires is a global function.

Being a global in JS has a very specific meaning from a scoping perspective. The fact that require is injected into a module's scope via a wrapper means it is not functionally global. For it to be useful, it has to be injected into a module's individual scope, because a call to require must resolve relative to the file doing the require-ing, which means it needs to know capture metadata about the require-ing module. If require were a generic global, it would already work in ES6 modules, but it would also have no way of loading things relative to the calling module, which makes it useless. If it were injected into each ES6 module's scope to gain the file-based relative resolving, that would in fact be non-compliant with the spec.

Bamieh commented 6 years ago

why not only require non-es6 modules and only import es6 modules? this way nothing will break, and it is more semantic as the way the file is imported indicates whether it is es6 or commonjs

ljharb commented 6 years ago

@Bamieh this has been answered multiple times in the thread; because then it would prevent authors from being able to transparently change the module format, and it would force consumers to know the module format of the package they're importing.

Bamieh commented 6 years ago

@ljharb i dont see this as a convincing explanation, everyone uses versioning by the habit of npm, (semver or random versioning).

Bumping a breaking change (major version) when the authors upgrades (and they will have to eventually) will solve this issue. The users who want to update the packages' major version will understand that it is now not a commonjs module.

This will also force the users to migrate from commonJS to the spec import as they want to use newer versions of their packages eventually.

ljharb commented 6 years ago

That creates friction; the end result will be many modules never migrating to ESM at all.

The best path towards everybody migrating to ESM, is making that migration not be breaking.

Bamieh commented 6 years ago

can you elaborate on the "friction" part?

I believe who ever wants to migrate will do so, regardless if it was a breaking change or not. Most of the community i am exposed to uses babel to use the import syntax, I dont see community resistance to the change, on the contrary, es6 modules are one of the most anticipated features to come since es5.

demurgos commented 6 years ago

The friction part is that I'd have to know at every moment and for every one of my dependencies which module format it uses, have mixed requires and imports through my own source code because of this and have to keep changing my imports in all my code base every few weeks as the libraries get migrated. There's also the risk that now some libraries would use a major bump to both change their module format (so I have a boring but easy find and replace) and introduce some other changes in one bump (so now I have to seriously review my codebase if I want ES modules). To top it off, we would get in a Python-like situation were the transition would leave sequels for years because some libraries will want to support older Node versions and keep commonjs and they'd have to hack some ES-wrappers on top of it for those wanting ES syntax (I'd have either to stick with require("express") for years or use a spurious wrapper import * as express from "express-es").

As @ljharb mentioned it, the Node situation regarding ES modules was discussed at large for months. Please make some research before posting: it was most likely already proposed. mjs may not be ideal but from a technical point of view it's the best current solution to solve this issue.

ljharb commented 6 years ago

The import syntax as currently used is largely just sugar for require; its not ESM until it's native.

The only way to get to "everyone is on native ESM" is if there is as smooth a migration path as possible from all other things to ESM - using a file extension helps provide it in a very large way.

(The previous comment is right except that this has been discussed for years, not months)

WebReflection commented 6 years ago

how about explicitly opting in via .m.js instead?

currently Node is diverging from every other environment.

As of today there's basically no way to write a module that works seamlessly in both browsers, NodeJS, SpiderMonkey, and JSC, without risking issues due transpilers now mandatory.

Every browser, SpiderMonkey, or JSC, is capable of loading valid ESM modules right away.

If we use the current Node "magic" approach, every other environment is doomed:

import test from './module';

There's also no guaranteed result. If that code runs after Bable transpilation, and it's included from a pure ESM .mjs based env, it will fail because test won't point to the test.default.

If we have to write test.default it means that .mjs didn't solve anything.

About .m.js

If the file pointer is a fully valid path, NodeJS should be able to load it regularly like any other browser would when it comes to importing an ESM module.

import test from './module.m.js';
test();

Transpilers can at this point fallback to .c.js files and publishers can decide at that point if they should put in npm either index.m.js to let bundlers do their job, or as already transpiled CommonJS for NodeJS backward compatibility sake: it's up to modules authors.

Meanwhile, every browser could use the project as is without requiring mandatory transpilation, NodeJS doesn't need to inject any magic as extension, the JavaScript ecosystem based on .js files will still be 100% compatible.

An example I've just tested and worked well for me is majinbuu, now published as .m.js that would require @std/esm to be loaded properly (but that's inevitable with the current status) and it has a transpiled .c.js counterpart and a pre-bundled min.js for browsers/projects that don't use bundlers.

It's a 360 solution that misses only one thing: NodeJS consensus in treating .m.js files as always ESM or throw errors.

Thanks in advance for any sort of outcome/consideration.

pakastin commented 6 years ago

In the front end we bundle both UMD and ES2015 modules. UMD is defined with main and ES2015 with module. If you use browserify (require), main will be used, and if rollup (import), module will be used. I don't see why that couldn't be the case with also node.js.

For example: https://github.com/redom/redom/blob/872c91b320f53f09ca1487868bddcfd4de3f2e7f/package.json#L5-L6

pakastin commented 6 years ago

Even browsers do it that way: <script type="module"> defines the future version, and <script nomodule> the legacy one.

ljharb commented 6 years ago

@pakastin because node has use cases that involve no package.json at all - a file extension, or in-file content, are the only acceptable ways that don't exclude some use cases. (also please don't thumbs-up your own posts; that's tacky)

WebReflection commented 6 years ago

not quite on the same page @pakastin ... look at this code:

<!doctype html>
<script type=module>window.global=window;</script>
<script type=module>
import majinbuu from 'https://unpkg.com/majinbuu@latest/index.m.js';
console.log(majinbuu);
</script>

that's it ... my 100% valid JavaScript works out of the box as ES2015 module. I don't need to bundle anything and I've also kept the global reference in the module on purpose, for the time being, as that's basically the only thing that might still be tricky to get right until it's solved in ECMAScript.

JavaScript doesn't run only on Node or browsers, this .mjs path which is forcing people to omit the file extension in NodeJS causing portability disasters should be fixed before it lands!

pakastin commented 6 years ago

You can also import files in the browser-side as well. If you try to import commonjs, it would break. I don't see that as an issue? If you require ES2015 module that should break as well. That's the deal when you do an error: you get an error message, right?

WebReflection commented 6 years ago

yes @pakastin we agree on that. If you require that's irrefutably a CommonJS intent.

If you import though, as a developer I expect that to be also irrefutably an ESM intent.

I've no idea why NodeJS decided to promote early adopter of the import syntax as CommonJS overlay, but I understand it's now a whole new mess to solve.

My proposal fixes the intent and the portability hazard current NodeJS ecosystem is promoting: using modules paths without extensions and auto-magic, non-controllable, CJS or ESM switch.

That is bad, and a lock-in behind transpilers.

Please reconsider this approach, thank you.

WebReflection commented 6 years ago

a file extension, or in-file content, are the only acceptable ways

what is the real difference between .mjs and .m.js ?

everything .mjs supposed to solve would be solved as well, except you'll have the entire world already compatible with latter solution.

ljharb commented 6 years ago

@WebReflection one difference is that Sprockets, Rails' asset engine, operates on single extensions at a time - so .m.js would mean .m is one format and .js is another; whereas .mjs would be its own.

In other words, an extension is a single thing - I think code in tons of software would have to change to easily work with a double-extension like .m.js.

WebReflection commented 6 years ago

@ljharb Rails doesn't have to do anything different than it does already. .js is a perfectly valid extension for a JavaScript module. I am not sure what kind of issue you are talking about.

Can you please provide a real-world example of what could go wrong?

I'm pretty sure my proposal works out of the box everywhere.

.mjs on the other side, fails in Apache, Python 2, Python 3, and the entirity of the world that doesn't know, and maybe never will, about this new .mjs thing.

The .js is an ubiquitous extension: it works for browsers, it can work for NodeJS with exact same semantics used right now for .mjs.

pakastin commented 6 years ago

There’s also .min.js wildly in use, so I don’t see the ”double” suffix as an issue..

ljharb commented 6 years ago

.js only works for browsers as a Module with a separate channel - type="module". Sprockets has no other channel, and neither does node - only the file name/extension, and its contents.

@pakastin nothing looks at the .min tho - it's for humans, not computers.

WebReflection commented 6 years ago

.js only works for browsers

SpiderMonkey runs modules via .js without any issue, just an -m flag to start with, JSC has enabled ES2015 modules by default and it works out of the box with .js modules.

Sprockets has no other channel

I'm not familiar with Sprockets but it looks like a Ruby thing to bundle? Webpack and Rollup have no issues with figuring out modules, same goes for @std/esm.

Why should a bundler that is not even written in NodeJS affect anything about the future of JS?

and neither does node

the same way node can understand a new extension is the same way it could understand a path that ends with .m.js ... node already opts in/out from ESM so I wonder if this is really something it couldn't do. I might dig into the source code though ...

WebReflection commented 6 years ago

maybe I'm missing something, but it looks like it's relatively straight forward to enable my proposal.

screenshot from 2017-10-12 18-44-36

vkurchatkin commented 6 years ago

same goes for @std/esm

It just uses a different non-standard convention.