Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set esModuleInterop to "true" by default #41139

Open
3 of 5 tasks
voxpelli opened this issue Oct 16, 2020 · 11 comments
Open
3 of 5 tasks

Set esModuleInterop to "true" by default #41139

voxpelli opened this issue Oct 16, 2020 · 11 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@voxpelli
Copy link

voxpelli commented Oct 16, 2020

Search Terms

  • esModuleInterop

Suggestion

Change esModuleInterop from being an opt-in to instead be an opt-out, taking one step further towards deprecating the non-esModuleInterop mode and make esModuleInterop the only supported mode going forward (which has to be the long term goal).

Use Cases

When Typescript 2.7 introduced esModuleInterop almost 3 years ago it emphasized clearly that:

We highly recommend applying it both to new and existing projects.

esModuleInterop is also a recommended option to set in tsconfig.json and when one runs tsc --init it gets set to true automatically.

However, the fact that it still is opt-in makes that non-esModuleInterop mode is still very prevalent, which kind of works against the very problems it was meant to fix – as rather than having just the pre-esModuleInterop functionality, modules now have to try to make things work across both options – and on top of that increasingly have to work with TS-validated JS.

All in all, that causes problems: fastify/env-schema#17

And is hard to wrap ones head around: https://github.com/fox1t/modules-playground

Even if one really tries: fox1t/modules-playground#3

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
    • It would for all code which haven't yet adopted esModuleInterop, but it would make it easier to get TypeScript and JavaScript code to stay compatible with one another
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
    • It would emit different JS for all code which haven't yet adopted esModuleInterop, but it would make it easier to get TypeScript and JavaScript code to stay compatible with one another
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
    • Apart from Do not cause substantial breaking changes from TypeScript 1.0.
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Oct 19, 2020
@RyanCavanaugh
Copy link
Member

I'll take this as a possibility but we have historically not changed commandline defaults unless the case for it was overwhelmingly strong. This might have the possibility to break people's programs and we would need some very good evidence that it was worth those people encountering that problem.

@voxpelli
Copy link
Author

@RyanCavanaugh Yeah, it's tricky, right now it's causing resentment among non-TS developers who are trying to cater TS users, so current situation is undesirable as well. (See eg also this Twitter thread with me and @mcollina on top of the above GitHub issues: https://twitter.com/matteocollina/status/1305953416217268226)

@voxpelli
Copy link
Author

An example where the current default causes confusion and makes people stick with the older non-recommended path: plantain-00/type-coverage#78

Also had this Twitter conversation with @Rich-Harris: https://twitter.com/rich_harris/status/1361839944453541892?s=21

@bmeck
Copy link

bmeck commented Mar 17, 2021

I'm a bit wary of using esModuleInterop since how it interacts with node's ESM implementation of default isn't quite compatible. This leads to various conversations about what to do usually ending up with "don't use default exports" as my go to response. Enabling it by default would remove some issues but it seems like a second migration would have to happen afterward if node compatibility was another configuration option.

@voxpelli
Copy link
Author

I'm a bit wary of using esModuleInterop since how it interacts with node's ESM implementation of default isn't quite compatible.

@bmeck Can you elaborate a bit on this or provide a link which describes it?

My impression was that esModuleInterop was meant to increase compatibility?

@bmeck
Copy link

bmeck commented Mar 30, 2021

@voxpelli

My impression was that esModuleInterop was meant to increase compatibility?

It kind of does, but not enough to be completely compatible. Particularly how "default" works still tries to do some magic and things start to get incompatible because of it.

Given:

// ./entry.ts
import * as asNamespace from './dep.js';
import asDefault from './dep.js';

console.log({
  asDefault,
  asNamespace
});
// ./dep.js
module.exports = {
  __esModule: true,
  default: 'prop named default'
};

Compiling to CommonJS will result in the following outputs:

Node (after renaming .ts to .mjs):

$ node ./entry.mjs
{
  asDefault: { __esModule: true, default: 'prop named default' },
  asNamespace: [Module: null prototype] {
    __esModule: true,
    default: { __esModule: true, default: 'prop named default' }
  }
}

After TS:

$ node ./out/entry.js
{ asDefault: 'prop named default',
  asNamespace: { __esModule: true, default: 'prop named default' } }

Namely, even with esModuleInterop it tries to do some hoisting of the default property if __esModule:true exists that Node won't ever do.

@voxpelli
Copy link
Author

The #44501 seems to partially implement this as both its module types imply esModuleInterop

So anyone setting the 'module' in their tsconfig.json to either node12 and nodenext will get esModuleInterop by default.

A good solution for changing the default without breaking backwards compatibility, but may complicate the uptake of those new types, see eg: sindresorhus/tsconfig#8

@voxpelli
Copy link
Author

@bmeck Looking at the documentation page for esModuleInterop

This mis-match causes these two issues:

  1. the ES6 modules spec states that a namespace import (import * as x) can only be an object, by having TypeScript treating it the same as = require("x") then TypeScript allowed for the import to be treated as a function and be callable. This breaks the spec’s recommendations.
  2. while accurate to the ES6 modules spec, most libraries with CommonJS/AMD/UMD modules didn’t conform as strictly as TypeScript’s implementation.

Turning on esModuleInterop will fix both of these problems in the code transpiled by TypeScript. The first changes the behavior in the compiler, the second is fixed by two new helper functions which provide a shim to ensure compatibility in the emitted JavaScript.

So while the compiler was fixed to comply with the first point, the ESM spec, as you say, two helper functions was also added to increase compatibility (I guess with older TS-code?)

Those two helpers seems to be these two ones in tslib:

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

__importDefault = function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};

And after compilation your code would roughly look like (assuming importHelpers would also be true):

// ./entry.ts
const asNamespace = tslib.__importStar(require('./dep.js'));
const asDefault = tslib.__importDefault(require('./dep.js'));

console.log({
  asDefault,
  asNamespace
});

So I guess, what you're getting at is that you think these two helpers should not be there?

@bmeck
Copy link

bmeck commented Aug 27, 2021

So I guess, what you're getting at is that you think these two helpers should not be there?

I have no opinion on implementation, just that the example above shouldn't behave differently from Node pre/post running TS when trying to match Node. I do not think esModuleInterop preserves behavior as shown above and shouldn't be enabled by default if a 2nd pass of enabling something would create another breakage. I'd rather just have 2 modes of default operation with 2 understood behaviors rather than a long tail mode of legacy prior with esModuleInterop:false default, a second with esModuleInterop:true, and a third with esModuleInterop:true + some flag to make the default export work like Node.

@voxpelli
Copy link
Author

The #44501 seems to partially implement this as both its module types imply esModuleInterop

So anyone setting the 'module' in their tsconfig.json to either node12 and nodenext will get esModuleInterop by default.

Double checked whether this actually shipped, and it did:

export function getESModuleInterop(compilerOptions: CompilerOptions) {
if (compilerOptions.esModuleInterop !== undefined) {
return compilerOptions.esModuleInterop;
}
switch (getEmitModuleKind(compilerOptions)) {
case ModuleKind.Node16:
case ModuleKind.NodeNext:
return true;
}
return undefined;
}

So anyone using "module": "node16" or "module": "nodenext" will have their esModuleInterop default to true (but can override it by actively setting it to false).

Though: This isn't at all mentioned in the docs for esModuleInterop 🤔

@remcohaszing
Copy link

I am strongly opposed to using esModuleInterop and I was shocked to find out this is default behaviour when module is set to node16.

Many people don’t understand the difference between export default / import identifier from 'module' and export = / import identifier = require('module'). Many people don’t even know the latter exists, though it is often the correct one for their use case.

esModuleInterop hides this difference for them by generating some interop code. For end user products such as websites or CLI applications, this is not a big deal. For libraries however, this means TypeScript will generate invalid type definitions (#52779).

I believe users need to be more aware of the code they ship (not write) and its correctness. esModuleInterop opposes this view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants