Synthesize namespace records for proper esm interop

[weswigham]

Fixes #16093

This PR updates our commonJS and AMD (thus by extension UMD) module emit to synthesize namespace records based on the presence of an __esModule indicator, or lack thereof, bringing our emit in line with babel's. (Modules which are not babel-compiled will finally be uncallable and unconstructable unless you use the synthetic-default-import version of the module.)

This changes:

// foo.js
module.exports = class Foo {}

// foo.d.ts
class Foo {}
namespace Foo {}
export = Foo;

// index.ts
import * as Foo from "./foo";
new Foo();

Will break. The correct way to write this code (with allowSyntheticDefaultImports to handle the type changes) is:

// index.ts
import Foo from "./foo";
new Foo();

This also:

• These changes are behind a flag: --esModuleInterop, which will default to false for a release before to becomes true making it the default behavior - it will be set to true for config files generated with tsc --init
• Set allowSyntheticDefaultExports to true by default for commonjs and AMD modules
• Disable allowSyntheticDefaultExports for javascript files and automatically infer if the module will be default or not based on the presence or lack thereof of an __esModule indicator.
• Refines the behavior of allowSyntheticDefaultExports for TS files to never create a synthetic default, unless the file uses export =.
• Expands the error on imports from Module '"name"' resolves to a non-module entity and cannot be imported using this construct. by removing call and construct signatures from namespace imports and closing the namespace-merging hack gap (this should break everyone broken by the emit update in a very loud way).
  • There's a quickfix to correct the import whenever it is used improperly!
• Add new helpers to tslib once their implementation is reviewed

[cspotcode]

Can we opt-out of this change in emit via a compiler flag?

[weswigham]

@cspotcode: Opting out of the emit change should be as simple as using noEmitHelpers and providing noop functions for the new helpers (minimally, we need to discuss if we want to offer more). The change to make * as Foo uncallable from the type system's perspective... It's a spec bug we've had for awhile and we've tried to discourage it. We'll probably offer a quick fix, but an opt-out flag we need to discuss.

[weswigham]

@DanielRosenwasser As you suggested, I've added the same changes to the AMD emit (babel does also do this for their AMD emit, so it certainly makes us more consistent).

[weswigham]

After a bit of discussion, we've decided that even with the quickfix available, we'd like this to be behind a flag for a release, then made into the default but still opt-out, then finally made the only option in a 3rd release (also known as a proper deprecation cycle). As such, I've gone ahead and updated the checker behavior - allowSyntheticDefaultImports is now the default behavior (though you can still force it off if you're overwriting the emit helpers), and it now has much safer behavior which aligns with the emit more often (at the cost of greater complexity). For example:

• JS files never have a synthetic default if they have __esModule set or use ES6+ module syntax, otherwise they have one
• TS files never have a synthetic default unless the file uses export=
• Declaration file and ambient modules are still a bit ambiguous, however:
  • If the module has an explicit default, it does not have a synthetic default (this was the only condition on the old behavior)
  • If the module has an explicit __esModule (because the user wrote it in their code prior to emit or explicitly in the .d.ts), then it does not have a synthetic default
  • Otherwise we really can't be sure, so we allow a synthetic default - it might not exist at runtime, so it is up to users to check (which is most of our current behavior).
• Additionally, synthetic defaults on dynamic imports are now handled with a spread instead of an intersection type, avoiding a class of issues where an overridden default from a JS file mangle the JS file's types.

Right now everything in this PR is still on by default, however I should have it behind a flag in short order - without that it's still likely ready for initial review and iteration.

[nojvek]

Thanks for working on this. Excited for babel & typescript to behave in similar fashion when importing non es6 modules.

[mhegazy]

This is not a strict option per se.. this is an interop flag.. i would not use strict prefix since we have already given it a different meaning with --strict.

[weswigham]

🚲 🏠 🕐 Alright, --standardESM then?

[weswigham]

Spoke with daniel, gunna try --ESMInterop

[weswigham]

@mhegazy Changed the option to the name we discussed at today's design meeting, and set it to output as true in the --init default object, also as discussed.

[demurgos]

Hi,
It seems that the last commit introduced a regression.

I created a demo repository for ESM using your module-nodejs branch.
It uses your version of the Typescript repo as submodule and compiles the commonjs output with the command node node_modules/typescript/built/local/tsc --rootDir src/lib --outDir build/cjs --moduleResolution node --ESMInterop src/lib/*.ts.

When using weswigham@7ff11bb I get the correct output for cjs:

"use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
}
exports.__esModule = true;
var hybrid_1 = require("hybrid");
var path_1 = __importDefault(require("path"));
function run() {
    console.log("ESM is getting real");
    console.log(path_1["default"].posix.join("i", "can", "use", "cjs", "modules"));
    console.log("And hybrid modules:");
    console.log(hybrid_1.sayHello());
}
run();

But when I use weswigham@bcafdba with ESModuleInterop I get the following (invalid) code when building cjs:

"use strict";
exports.__esModule = true;
var hybrid_1 = require("hybrid");
var path_1 = require("path");
function run() {
    console.log("ESM is getting real");
    console.log(path_1["default"].posix.join("i", "can", "use", "cjs", "modules"));
    console.log("And hybrid modules:");
    console.log(hybrid_1.sayHello());
}
run();

It seems that the rename caused the option to no longer be applied.

[mhegazy]

@weswigham can you also update https://github.com/Microsoft/TypeScript-Handbook/blob/master/pages/Compiler%20Options.md

[weswigham]

@mhegazy Done!

[felixfbecker]

Having read the description, it's not fully clear to me whether this is a breaking change or not. If it is, it probably has a very large impact. Could someone clarify?

[weswigham]

This does not affect TS without the esModuleInterop flag specified beyond small enhancements to check if a JS file has an explicit __esModule marker while under allowSyntheticDefailtImports. Without setting esModuleInterop you shouldn't notice any changes at present.

But we would like to make this behavior the default in the future (to align our default emit with babel), so you should check if this flag breaks you. If it does, you have a future problem that needs fixing - likely invalid usage of the namespace import hack that you should probably use a default import for instead (which was not possible prior to the addition of the emit helpers this flag enables). There is a quick fix to that effect bundled with this PR, so finding and fixing all those instances shouldn't be hard. 🌞

[bluelovers]

how to work with when "esModuleInterop": true,?
or this is bug?

globby is a export = function like module

import * as globby from 'globby';

var __importStar = (this && this.__importStar) || function (mod) {
    if (mod && mod.__esModule) return mod;
    var result = {};
    if (mod != null) for (var k in mod) if (Object.hasOwnProperty.call(mod, k)) result[k] = mod[k];
    result["default"] = mod;
    return result;
}
Object.defineProperty(exports, "__esModule", { value: true });
const globby = __importStar(require("globby"));

TypeError: globby is not a function

[weswigham]

Use a default import for export= modules that aren't namespace like: import globby from 'globby'

[bluelovers]

no option disable importStar only?

[weswigham]

I don't think so, no. But a default import aught to work for you in this case; since it's not a module it's top level exported object becomes it's default. You shouldn't need to wrap the library or anything; simply using the other import syntax should suffice.

[bluelovers]

thx, feel sad, looks like esModuleInterop is not good for me

i hope can import * is old way ( make sure everything is expected )
, but import default is esModuleInterop way by ts

lookes like i should keep use old option lol

[simonbuchan]

@bluelovers as far as I can tell, you shouldn't see bad runtime behavior without getting a build time error anyway, and import star is less correct: import default is not really correct either, but import star is not used to get a commonjs export object outside of the typescript world.

@weswigham TS has a logically correct import name = require("id") form, but it doesn't allow using this when targeting ES2015 modules. Any chance this restriction could get lifted? Targeting esmodules for your own code for bundling/tree-shaking purposes while still using commonjs modules makes sense.

[bluelovers]

anthor question why we need __importStar not just require when esModuleInterop open? just confused
is there something need this?

[weswigham]

@simonbuchan

@weswigham TS has a logically correct import name = require("id") form, but it doesn't allow using this when targeting ES2015 modules. Any chance this restriction could get lifted?

In both node's current esm environment and in the browser's esm environment, there's no concept for an import name = require("id") statement to transpile to. Without a real definition for what the target environment is, saying that we can transpile it is a dicey prospect (esp. since how you expect it to "behave" in your case is entire based on how your toolchain is composed). You could always transform away imports of that style yourself in a before transform as part of your toolchain and ignore the error saying it's forbidden.

@bluelovers

anthor question why we need __importStar not just require when esModuleInterop open? just confused
is there something need this?

If you didn't need it, than you wouldn't have to use the esModuleInterop flag. All it does is alter the emit and typechecking to make commonjs packages available as a "default" import, which is how babel does commonjs interop.

[simonbuchan]

@weswigham Pretty sure node esm can require() commonjs modules? It's just a function, after all. They call out the other way around, not being able to require a mjs file, which makes some sense.

And you do have a definition for your environment: if @types/node or @types/react-native or whatever isn't referenced, you already get an error about require not being defined, so you can look at the modules option the same as Babel's module transforms, and this import just being a version of const that pulls in the module typings.

The use would mostly be clarity about behavior, but with this it also allows skipping the interop cost.

[weswigham]

It can't. require isn't available in node's esm mode.

[weswigham]

Not that node's esm mode is stable in the least, so that's still got the potential to change.

[bluelovers]

@weswigham

oh I thought esModuleInterop is for import default
but why need make a __importStar when import *

there has something we need use ?

import * as a form 'a';
//.... ?
a.default

[weswigham]

That same default import needs to be available on the namespace and named imports, too.