microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.58k stars 12.43k forks source link

Synthesize namespace records on CommonJS imports if necessary #16093

Closed DanielRosenwasser closed 6 years ago

DanielRosenwasser commented 7 years ago

TL;DR

Flags like --allowSyntheticDefaultImports will just work without extra tools like Webpack, Babel, or SystemJS.

TypeScript will just work with the --ESModuleInterop flag without extra tools like Webpack, Babel, or SystemJS.

See the PR at #19675 for more details.

Background

TypeScript has successfully delivered ES modules for quite some time now. Unfortunately, the implementation of ES/CommonJS interop that Babel and TypeScript delivered differs in ways that make migration difficult between the two.

TypeScript treats a namespace import (i.e. import * as foo from "foo") as equivalent to const foo = require("foo"). Things are simple here, but they don't work out if the primary object being imported is a primitive or a value with call/construct signatures. ECMAScript basically says a namespace record is a plain object.

Babel first requires in the module, and checks for a property named __esModule. If __esModule is set to true, then the behavior is the same as that of TypeScript, but otherwise, it synthesizes a namespace record where:

  1. All properties are plucked off of the require'd module and made available as named imports.
  2. The originally require'd module is made available as a default import.

This looks something like the following transform:

// Input:
import * as foo from "foo";

// Output:
var foo = __importStar(require("foo"));

function __importStar(mod) {
    if (mod && mod.__esModule) return mod;
    var result = {};
    if (mod != null) {
        for (var k in mod) {
            if (Object.prototype.hasOwnProperty.call(mod, key)) {
                result[k] = mod[k]
            }
        }
    }
    result.default = mod;
    return result;
}

As an optimization, the following are performed:

  1. If only named properties are ever used on an import, *the require'd object is used in place of creating a namespace record.

    Note that this is already TypeScript's behavior!

  2. If

    1. a default import is used at all in an import statement and
    2. if the __esModule flag is not set on the require'd value, then

    A fresh namespace record whose default export refers to the require'd value will be created in place of the require'd value. In other words:

    // This TypeScript code...
    import { default as d } from "foo";
    d.member;
    
    // Would become this CommonJS JavaScript code...
    var foo = require("foo");
    var foo_2 = foo && foo.__esModule ? foo : {default: foo};
    foo_2.default.member;

Note that there is no optimization for never using the default. This is probably because you'd need alias analysis and could never be sure that someone else would use the default.

Proposal

I believe that we'd serve both communities well if we decided to adopt the aforementioned emit.

Drawbacks

Performance & Size Impact

This certainly impacts the size and readability of TypeScript's output. Furthermore, for large CommonJS modules, there could be a speed impact to keep in mind - notice that these synthesized namespace records are not cached at all.

Tools already do this for us

Those who've been using tools like Webpack 2 and SystemJS have been getting this functionality for a few months now. For example, if you target ES modules, Webpack 2 will already perform this behavior. Taking this strategy makes it an opt-in for users who care about the interop behavior.

I mainly bring this up because I want to immediately bring up the counterpoint. While these tools are definitely central to many modern web stacks, they won't cover

  1. Vanilla Node.JS users
  2. React Native Packager users
  3. Browserify users (duh)
  4. Users whose test code doesn't run through these tools
DanielRosenwasser commented 7 years ago

cc @TheLarkInn @hzoo @guybedford

aluanhaddad commented 7 years ago

This is fantastic. I really want to see this happen. Thank you very much.

I want to add that one benefit of this is that it simplifies documentation, tutorials, and other learning materials that currently either have long explanations regarding these differences or, as is more often the case, only show an import usage example that works for either the TypeScript community or for the Babel community but not for both.

guybedford commented 7 years ago

This is great to hear and I'm really glad TypeScript is looking to support this interop.

@DanielRosenwasser do you perhaps mean to correct if the __esModule flag is set on the require'd value, then to read if the __esModule flag is not set on the require'd value, then above?

Just to share the current shift SystemJS has had to make wrt interop - I know this has probably been discussed ad nauseam, but just to voice my opinion again...

Namespace lifting for CommonJS

NodeJS doesn't seem to have any plans of treating CommonJS modules as namespaces, and instead simply providing them as default exports, due to all the edge case concerns here of calling getters etc. While I know this is gambling on a possibility, I think it is worse to gamble that NodeJS will get over this restriction any time soon in its adoption of ES modules. SystemJS took this hit already (while using __esModule as a fallback method), and it has definitely affected users.

Just thought it is worth mentioning as I do think it would benefit TypeScript to double down on the syntax import x = for CJS as this allows a great way to syntactically separate CJS requires and their named exports, without assuming that CommonJS modules can be magically converted into ES module namespaces. import x from 'x' for CJS can still be allowed, while gradually deprecating import {x} from 'cjs'.

Edit: and perhaps Node core modules can be treated as exceptions here due to the expectation that in the conversion to ES modules, NodeJS core would permit named exports for these.

aluanhaddad commented 7 years ago

Just thought it is worth mentioning as I do think it would benefit TypeScript to double down on the syntax import x = for CJS as this allows a great way to syntactically separate CJS requires and their named exports, without assuming that CommonJS modules can be magically converted into ES module namespaces. import x from 'x' for CJS can still be allowed, while gradually deprecating import {x} from 'cjs'.

Just a thought, but I wonder if it might be worth adding another --module target to TypeScript, say CommonJSStrict, that only supports the set of operations that Node currently plans to support for interop. I realize this would add complexity to TypeScript, but it might be easier than deprecating named imports. If Node adopts named imports, then that feature could be added to the new module target.

I've been recommending the import = syntax to people targeting CommonJS for a while now.

Also, CC @unional, @Kovensky

unional commented 7 years ago

Thanks @aluanhaddad, for notifying me about this wonderful issue. Why can't we add more than one 👍 to the OP? 🌷

I have also been recommending the import = syntax for a while. It works nicely in my use cases.

However, I want to bring up that unfortunately, it cannot be a long-term solution. The issue is browsers' eventual support of ESM.

When user target es6, the import = needs to be interop to import default as the require() function is not available in the browser without the help of any loader.

This means CJS -> ESM interop and a unify view of CJS <-> ESM is still needed. And it also means magically converting CJS -> ESM (for the dependencies of the compiling package) is still needed to be performed during compilation.

(edit: we should point this out to the nodejs folks as their decision on this prohibits code sharing on server and browser unless all dependencies are ESM)

Another major issue needs to be dealt with is typings and direct ts source code distribution. As mentioned in #7398, the implicit capturing of the tsconfig in the typings is very problematic and could greatly complicate the implementation of this work and the adaptation of the new mode CommonJSStrict if introduced. We should also look at that one to see if there is a solution to that or a migration path on updating all typings available. Speaking of which, making typings to respect scope would be desirable, but that's another issue. 🌷

unional commented 7 years ago

cc @blakeembrey @basarat @felixfbecker

DanielRosenwasser commented 7 years ago

@unional I think that the .d.ts issue is something we'll have to deal with separately down the line. I've already been pushing new PRs in DefinitelyTyped that describe value-exported entities to use the export = syntax, and use import x = require('...') syntax.

@DanielRosenwasser do you perhaps mean to correct if the __esModule flag is set on the require'd value, then to read if the __esModule flag is not set on the require'd value, then above?

@guybedford Yes, thanks, I corrected my post.

Just thought it is worth mentioning as I do think it would benefit TypeScript to double down on the syntax import x = for CJS as this allows a great way to syntactically separate CJS requires and their named exports

@guybedford I agree, but unfortunately, a lot of people are extremely turned off by that idea for philosophical reasons. The existence of import foo = require('...') makes complete sense and is actually useful, but my experience is people don't want to hear it.

Just a thought, but I wonder if it might be worth adding another --module target to TypeScript, say CommonJSStrict

@aluanhaddad that's something @mhegazy and I have discussed at some point, and I think it'll be on the table, but since nobody really knows what will happen in 6 months time, I think we should wait. At the very least, adopting this behavior will mean that there is a smooth migration path for users (as opposed to now where users can't get ready for a change like that at all without an external tool).

DanielRosenwasser commented 7 years ago

After our design meeting today, I don't think this is 2.4-bound, but what we're looking for is the best migration path we can conceive, along with a story that synchronizes with type declarations on DefinitelyTyped.

Current concerns are that the Babel emit does not allow namespace imports to be callable, which actually does break a lot of (technically invalid) code, such as the following:

import * as padLeft from "pad-left";
padLeft(10, "zup doug");

So a potential modification could be the following

function __importStar(mod) {
    if (mod && mod.__esModule) return mod;

    // For callable/constructable things, tack properties onto a function record
    // that passes arguments through to the actual import
    var result = !(mod instanceof Function) ?
        {} :
        function() { return mod.apply(this, arguments); };

    if (mod != null) {
        for (var k in mod) {
            if (Object.prototype.hasOwnProperty.call(mod, key)) {
                result[k] = mod[k]
            }
        }
    }
    result.default = mod;
    return result;
}

But we might also just be willing to have a new mode that goes with Babel's emit and breaks away from the old behavior in that respect.

Jessidhia commented 7 years ago

The behavior of import { x, y, z } from 'foo' is only correct if foo is an ES module. If it is a commonjs-exported object, it should look like a namespace with only a default export (i.e. x, y and z should be undefined regardless of whether foo's module.exports has those keys, and ideally be build-time errors).

All non-star-import emits should behave as if a hidden import * as _ref from 'cjs' was done; with the this value erased if the direct imports are called as a function; e.g. importedDefault() -> (0, _ref.default)(). The star import is basically imported.__esModule ? imported : Object.freeze({ __proto__: null, [Symbol.toStringTag]: 'Module', default: imported }).

Any of the changes to make TypeScript any closer to ESM/CJS interoperation spec compliance will be breaking because of how off the TypeScript implementation is from the actual proposed interoperation semantics. This would also need stricter semantics than --allowSyntheticDefaults -- a star import from CJS is a namespace with a default export, not the CJS exports with a self-referencing default.

felixfbecker commented 7 years ago

A lot of libraries have started to do stuff like this:

module.exports = BigNumber
module.exports.BigNumber = BigNumber
module.exports.default = BigNumber

To support nicer ES6 imports à la

import { BigNumber } from 'bignumber.js'
import BigNumber from 'bignumber.js'

the majority of libraries is not using a transpiler and will have to support CommonJS/Node imports, so it would be very sad if this is broken

guybedford commented 7 years ago

As an update on https://github.com/Microsoft/TypeScript/issues/16093#issuecomment-304263496, note that NodeJS has an ES module implementation PR at https://github.com/nodejs/node/pull/14369 with no plans to support synthetic exports at all. This implementation will likely be released early next year sometime. TypeScript has the ability to start preparing for the impending break of losing named exports for CJS here already.

guybedford commented 7 years ago

As a follow up, has it been considered before to support a destructuring variation of the import x = require('x') pattern in TypeScript?

Something like:

import { destructuredExport } = require('pkg');

as sugar for const { destructuredExport } = require('pkg') in CommonJS and import _pkg from 'pkg'; const { destructuredExport } = _pkg in ES modules.

That could complete the syntax on this form making it a syntax specific way of indicating this interop pattern.

Come on TS team, bite this bullet before it bites you :)

frankwallis commented 7 years ago

In the interim would it not be possible to remove the TS1202 warning

TS1202: Import assignment cannot be used when targeting ECMAScript 2015 modules. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.

And output

const mymod = require('some-module');

as that is in fact what the user asked for?

Currently it is not possible to target both es2015 and commonjs when importing a library which exports itself as a commonjs module. This is a common scenario when developing UI code which is unit tested in nodejs and bundled for the browser using webpack/rollup. In order to support nodejs it is necessary to compile to commonjs, but in order to take advantage of tree-shaking and module hoisting when bundling it is necessary to compile to es2015 modules.

It is not possible to use import mymod from 'some-module' as this code will fail at runtime in nodejs due to the lack of interopRequireDefault.

The previous workaround of using import * as add from 'some-commonjs-module' and adding a namespace to the declaration file is now being disallowed in DefinitelyTyped which means that the only solution is to use const mymod = require('some-module') and lose all the typing.

DanielRosenwasser commented 7 years ago

as that is in fact what the user asked for?

@frankwallis Yes, if it was known that the user expected that to emit to CommonJS, but not if they were asking for AMD or SystemJS.

frankwallis commented 7 years ago

@DanielRosenwasser thanks I see the problem now. Although I am yet to be convinced that there is a real-world use-case for mixing ES2015 imports and amd/commonjs...

It seems to me that resolving ES2015/CommonJS interop could be a long drawn-out process, and in the meantime this is causing real problems. Perhaps a better interim solution could be to output import * as mymod from 'some-module' when compiling to ES2015 as both Webpack and SystemJS do handle this (according to the initial post on this thread)

guybedford commented 7 years ago

A module written with import x = require('x') should become import x from 'x' in ES 2015 output and this should really be the recommended way to ensure correct interop. I'm not sure what I'm missing here?

frankwallis commented 7 years ago

@DanielRosenwasser is there anything to prevent typescript from compiling

import mymod = require('mymodule')

to

import mymod from 'mymodule'

when outputting ES2015 modules?

[Optionally this behaviour could be controlled by the allowSyntheticDefaultImports flag]

felixfbecker commented 7 years ago

@frankwallis @guybedford Imo that doesn't make sense. CommonJS has a default export, ES6 has a default export. If you do

import mymod = require('mymodule').default

it should become

import mymod from 'mymodule'

as that is only shorthand for

import {default as mymod} from 'mymodule'

if you do

import mymod = require('mymodule')

it should be become

import * as mymod from 'mymodule'

as it is today.

Jessidhia commented 7 years ago

And that is why this whole situation is a mess. It should not as (in the actual commonjs interop spec that was not written at the time TS implemented modules) the module.exports value is only visible as the default export, no matter what keys are in that object (or not object; exports can be number, string or undefined too).

The mymod in your * as mymod example, in real modules, would be a frozen object (similar, but not quite the same, as Object.freeze’s effect) that looks like { [Symbol.toStringTag]: “Module”, default: require(“mymodule”) }. import mymod from would then correctly import the module.exports value.

guybedford commented 7 years ago

The lack of native support for the __esModule pattern is exactly why I'm suggesting making import x = require('x') the primary recommended means of loading CommonJS in TypeScript.

Alternatively the __esModule flag check on all imports could just be directly adopted. We created it exactly for this purpose.

Note that checking imports for __esModule is fully orthogonal to the question of synthetic named exports extracted from a CommonJS - which probably isn't recommended since NodeJS is not going to be supporting this as things stand in the current implementation. Since TypeScript currently allows this, I thought it may be a backwards compatible way in TypeScript to just encourage the import x = require('x') pattern perhaps with an import { readFile } = require('fs') destructuring sugar for ease in allowing these while remaining compatible with Node.

aluanhaddad commented 7 years ago

@guybedford

Since TypeScript currently allows this, I thought it may be a backwards compatible way in TypeScript to just encourage the import x = require('x') pattern perhaps with an import { readFile } = require('fs') destructuring sugar for ease in allowing these while remaining compatible with Node.

That feels natural. It aligns with existing JavaScript usage patterns that already combine ES2015 and require. Also, I imagine that syntax would be intuitive for TypeScript users and, critically, it would be sharply different from ES Module syntax and thus not be misleading.

As far as CommonJS to ESM

import fs = require('fs');
import {readFile} = require('fs');

an emit of

import fs from 'fs';
let {readFile} = fs; // not hoisted.

also seems desirable for backwards compatibility. That it wouldn't behave like an ES Module named import would, given the source, not be surprising.

guybedford commented 7 years ago

I've created a separate proposal for the suggestion in https://github.com/Microsoft/TypeScript/issues/16093#issuecomment-320250943 at https://github.com/Microsoft/TypeScript/issues/18307. @aluanhaddad your suggestion seems like it is now covered by https://github.com/Microsoft/TypeScript/issues/17556.

guybedford commented 7 years ago

Has there been discussion over considering analysing which import statements correspond to CommonJS modules (treated as a default export on the namespace) and those that are local ES modules in the application?

Such an approach with full knowledge of resolution would be ideal, and TypeScript is in this position better than Babel ever was so can make this kind of a consideration.

It would be really interesting to hear what current thoughts are around this.

nojvek commented 7 years ago

I understand that Typescript wants to stick as close as possible to ES Spec

Since webpack and Babel support this, tons of libraries and modules have been written with kind of interop usage. It’s what an end user implicitly expects.

So just as some thing doesn’t work on Edge but works in chrome and we blame Edge for being incompatible. This kind of feels like that.

unional commented 6 years ago

if you do

import mymod = require('mymodule') it should be become

import * as mymod from 'mymodule' as it is today.

@felixfbecker the problem is that doesn't work correctly as mymod in import * as mymod syntax is not supposed to be callable. It is a module namespace.

So when mymodule exports a function...

Maybe I missed something? I know that you know these topics quite at heart. 🌷

guybedford commented 6 years ago

There may be some movement in named exports support in NodeJS happening at https://github.com/nodejs/node/pull/16675. The current proposal would be to support both __esModule as indicating named exports and a custom Symbol.for('esModuleInterop') on exports.

Would be interesting to see if this may or may not align with TypeScript expectations.

Edit: corrected the link

weswigham commented 6 years ago

@guybedford It looks like it'd be nice (not just for us, but for people who transpile with babel, too), since it'd enable interop with the existing ecosystem (which should be a priority, I imagine?) except it seems like it's now DOA, due to spec-nonconformance with respect to execution order. You guys have bent the spec to support node-style module names already (or at least departed from browsers); is there any reason you can't bend it more and report name binding errors late in order to get better interop behavior? It seems like that'd be technically-spec-breaking, but mostly transparent to actual language users, while also enabling the desired behavior (getting named exports from commonjs modules).

ljharb commented 6 years ago

@guybedford As you can see from the comments on https://github.com/nodejs/node/pull/16675, nothing runtime can be used to determine CJS-exported names since that would violate spec requirements for evaluation order.

What the ES spec does require, which TypeScript seems to be violating here (correct me if I'm wrong), is that import Foo from 'path'; import * as Module from 'path'; Foo !== Module && Module.default === Foo. Additionally, Module, being a Module Record, must be a frozen null object - not a function, not a primitive, and certainly not what the module exports - that has string keys corresponding to the imports from the package: "default" if present (which is always present in CJS), and the individual string names if they are present.

In other words, import * as React from 'react' is in violation of the language spec.

weswigham commented 6 years ago

import * as React from 'react' (the line, taken as its own) isn't in violation of any spec, as the module loader is unspecified. Thus, you could conceivably write a module loader, loaded by the runtime, which takes in some package, analyzes it prior to execution (or executes it once, then throws away the results of that execution other than the exports shape), and constructs an appropriate namespace object. This capability is even theoretically true with node's experimental esm support as it currently exists, using the dynamic instantiate and resolve hooks it exposes.

The issue is that the default loader in node is looking like it's going to have trouble emulating the semantics that the community expects based on how it was presented by community leaders and implemented in babel and now tsc.

On the other hand, the reason the community has had no issues polyfilling the an esm loader with hoisting behavior for commonjs modules is that we've given up early erroring on bad destructuring imports and instead have late errors when the destructuring fails on execution (which is pretty normal for a polyfill, as enforcing early/late error distinctions in a polyfill is costly at runtime and usually not relevant), thus enabling the behavior - that technically is spec-noncompliant behavior, but is mostly unobservable at runtime.

ljharb commented 6 years ago

@weswigham regardless of loader, the namespace object there must be a Module Record, which must follow the spec with regard to its prototype and its keys and its frozen-ness. In fact, if properly implemented, I believe a loader that provides a nonconforming Module Record would cause the engine to throw an exception.

(in my example, the assumption there is that React always refers to the namespace for the react library that, eg, contains a Component constructor function)

weswigham commented 6 years ago

(in my example, the assumption there is that React always refers to the namespace for the react library that, eg, contains a Component constructor function)

And that assumption is based on the behavior of an unspecified loader. Nothing says how the things pointed at by the react module reference becomes a namespace record. Nothing. Weather the namespace record is frozen/null prototype/whatever other exoticness is specified doesn't really matter; the loader should be capable of providing the correct properties in that regard - the issue here in the interop story is that the loader being written for node right now doesn't provide as many capabilities as people are accustomed to compared to the loaders polyfills have been providing, since those capabilities rely on looser error semantics on invalid imports than are formally specified.

ljharb commented 6 years ago

@weswigham that's fair; all I'm saying is that whatever the loader does, import Foo from 'path'; import * as Module from 'path'; Foo !== Module && Module.default === Foo && Object.isFrozen(Module) && typeof Module === 'object' && Object.getPrototypeOf(Module) === null && Object.prototype.toString.call(Module) === '[object Module]' must hold; what the values of Foo, or the string keys and values of Module are, is of course up to the loader (although humans will probably already have an intuition about that for importing from CJS packages).

What does TypeScript do right now in the above code if I replace 'path' with 'react'?

weswigham commented 6 years ago

@weswigham that's fair; all I'm saying is that whatever the loader does, import Foo from 'path'; import * as Module from 'path'; Foo !== Module && Module.default === Foo && Object.isFrozen(Module) && typeof Module === 'object' must hold; what the values of Foo, or the string keys and values of Module are, is up to the loader.

First, on the behavior of our downlevel esm loader polyfill: With the linked PR and its associated flag on (to make us do same thing babel does), Foo !== Module would be true, Module.default === Foo would be true, and typeof Module === 'object' would be true, while Object.isFrozen(Module) would be false because neither babel nor us use Object.freeze by default, since the runtime cost of polyfilling it is enormous, and even on runtimes that support it, userland objects which are frozen are usually slower. (Naturally, we've always said that if you absolutely must have the freeze behavior at runtime, you can overwrite our runtime helper with one which calls freeze on the result, and in babel there's a flag to use such a helper in place of the normal one)

Secondly, neither Foo !== Module nor Module.default === Foo are required invariants per spec. The spec only says that, in this case, Module must be a namespace record - it does not, however, preclude Foo from also being an object of the same namespace record, therefore under a given module loader it is very possible for both of those to be false, thus is not a good assumption to make. While there is not syntactic way to receive a self-reference within a module, the loader itself has no such restriction, and it is free to warp the shape of what it imports (including adding a self-reference as the default) at will.

What does TypeScript do right now in the above code if I replace 'path' with 'react'?

Assuming the context of the linked PR where we align with babel, none of the above, since react's top-level export will have an __esModule member at runtime, thanks to this PR and this root file. Meaning it will have no default, as at runtime we will pretend it is an ESM (meaning we assume that the loaded JS has already been transpiled into something approximating a namespace record). Incidentally, this means that import React from "react"; would be a runtime error (unless React is somehow actually exporting a self reference as their default for back-compat since they've changed to using esm internally), while import * as React from "react"; would be correct. Again, this only works because all major real polyfill implementations of an ESM loader defer module errors, as it is impractical not to do so.

ljharb commented 6 years ago

@weswigham

it does not, however, preclude Foo from also being an object of the same namespace record, therefore under a given module loader it is very possible for both of those to be false, thus is not a good assumption to make.

Can you provide some code that would make the ModuleRecord for a module be === to its own default export? If that code uses ESM, could you provide it with CJS?

Separately, this file actually ensures that it will not have an __esModule member at runtime.

guybedford commented 6 years ago

@ljharb this is possible to do with live bindings let module = createDynamicModule(['default'], function execute () { module.reflect.exports.default.set(module) }). Alternatively just import * as x from 'own-module-name-self-reference'; export default x.

It may be worth React including the __esModule attribute for named export support in interop.

ljharb commented 6 years ago

Fair enough; but that's still not up to the loader - it's up to the module.

Jessidhia commented 6 years ago
// self.mjs
import * as namespace from './self'
export { namespace }

namespace.namespace === namespace

This looks like a highly specific corner case, though, and most certainly does not imply that namespace[keyname] !== namespace is false unless you purposely go out of your way to make it so by setting it to be the namespace itself.

namespace[keyname] === namespace is a highly specific exception and a code smell, and not something you should consider as valid or desirable.

(I don't even know if this would work or if it would throw at runtime)

weswigham commented 6 years ago

The point is simply that that is not an invariant any spec asserts, nor one anyone should rely on, as it is simply untrue.

And @ljharb, the line let module = createDynamicModule(['default'], function execute () { module.reflect.exports.default.set(module) }) is part of the source of a theoretical node custom loader, using the currently exposed API - not the source of an esm itself. So yes, you can construct a loader which does odd things like restructures imports (custom loaders wouldn't really be useful for node if they couldn't).

My point is that no consumer can make sane assumptions about es6 imports without also making assumptions about the underlying loader's semantics. Babel and TS have a loader polyfill with certain loose semantics people have come to expect, while browsers have a very strict loader with ties to the document model and its execution goals, while node (experimental) has its own loader, and none of these are the same right now. ES6 modules are looking like they may become the least portable modern js you can write, since there's a good chance that an import you write for one runtime won't work in another, at least not without a transpilation tool or custom loader to translate. My only goal right now is to align with babel, since it's loader polyfill is a community winner (I mean, we half supported it two years ago already) in order to remove one instance of the competing standards problem from the equation. If node also manages to get its ducks in a row and find a solution that allows it to adopt similar semantics, then everyone but browsers will at least be using the same or similar loader semantics, and hopefully we cut the number of common variations to two.

Re @guybedford:

It may be worth React including the __esModule attribute for named export support in interop

Their index require's an esm-transpiled module and sets it as the module exports object. That import will have an esModule marker as transpiled esm usually gets one, ergo when that import is assigned as the module exports object, the module will have an esModule marker. So they should already be good. Probably. Not sure what that conditional .default thing is working around, so I'm not sure if it'd have the marker or not.

ljharb commented 6 years ago

@weswigham i will concede that “Foo !== Module” isn’t a guarantee provided by the spec. However, the characteristics of the Modue Record object (Module) are guaranteed, and no loader is permitted to violate them - if node has a proposed loader that does, then that won’t end up being able to ship.

The conditional default thing is explicitly to unwrap any Babel interop if it exists, so that CJS can require the object directly.

guybedford commented 6 years ago

@ljharb the characteristics are guaranteed by v8 - Node can't override them even if it wants to.

ljharb commented 6 years ago

So then the question is, what happens now in typescript when you import the module record and the default export from ‘react’? What are the values of Module and Foo?

guybedford commented 6 years ago

@ljharb it depends if NodeJS will support esModule (and accept CJS executes early in the pipeline) or not, as it has the ability to set the precedent here. But if React does not export esModule, then React is only equal to ModuleNamespace({ default: React }).

aluanhaddad commented 6 years ago

But if React does not export __esModule, then React is only equal to ModuleNamespace({ default: React }).

Based on react/src/React.js#L34 and react/src/React.js#L75

That seems to be the intent of the library.

ljharb commented 6 years ago

@guybedford node will not support that; runtime determination doesn't provide the ordering guarantees that the committee intends (and if needed, the spec will be reworded after this month's meeting to ensure that this is mandated).

guybedford commented 6 years ago

@ljharb CommonJS is a separate parse goal whose execution semantics are not defined by the specification for interop. There is a TC39 agenda item to discuss this at the coming meeting - it is not black and white by any means. You will get a chance to discuss this further at the end of the month, and I hope you will think a little more about what your stance is here. I personally believe this sort of a tradeoff would be a worthy one in the name of the health of the ecosystem.

ljharb commented 6 years ago

@guybedford the discussion we had at TC39 around CJS was effectively that ordering must be maintained; as far as I'm aware, the situation with what node will do with CJS, and thus what TypeScript should do, has not changed (and I believe, will not).

guybedford commented 6 years ago

@ljharb yup agreed this is the future now. I hope this gives TypeScript the confirmation it needs now to properly fix default exports in CommonJS interop.

guybedford commented 6 years ago

Just a heads up - I will likely be opening a new issue in the coming months to discuss native NodeJS default export form interop support in TypeScript. If there's an issue already tracking this please let me know too.

weswigham commented 6 years ago

@guybedford It'd just be the new esModuleInterop mode with namespace and named-member imports forbidden on modules which have a "synthetic" default, right? AFAIK every node proposal to actually interpret the __esModule marker and create named imports for CJS in ESM officially has fallen through.

guybedford commented 6 years ago

@weswigham exactly I guess it is to use synthetic defaults on CommonJS modules and namespaces on ES modules, where the format for which way to go on this is determined by the NodeJS resolver.