-
Notifications
You must be signed in to change notification settings - Fork 13k
Description
Bug Report
When a package is typed using module.exports =
, the correct way to type it is export =
. The correct way to import this, in either .ts
or .d.ts
files, is import identifier = require('module')
. For example import React = require('react')
.
If a user enables esModuleInterop
, they are allowed to use import React from 'react'
. When compiling to CJS, this will output JavaScript that used either module.exports.default
or module.exports
.
"use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
const react_1 = __importDefault(require("react"));
IMO This isn’t great, but it works.
What is bad, is that it generates a default import in the type definitions.
import React from 'react';
Whereas it should be typed as this:
import React = require('react');
If this is library code, that means the user of the library now is also enforced to use esModuleInterop
, whether they like it or not.
🔎 Search Terms
esModuleInterop
🕗 Version & Regression Information
This is the behavior in every version I tried, and I reviewed the FAQ for entries about Common "Bugs" That Aren't Bugs
⏯ Playground Link
Playground link with relevant code (Look at the .d.ts
output)
💻 Code
The following code requires esModuleInterop
to be enabled.
import React from 'react'
export const element = React.createElement('div')
🙁 Actual behavior
This is the generated type definition:
import React from 'react';
export declare const element: React.DetailedReactHTMLElement<React.HTMLAttributes<HTMLElement>, HTMLElement>;
🙂 Expected behavior
This is the correct type definition:
import React = require('react');
export declare const element: React.DetailedReactHTMLElement<React.HTMLAttributes<HTMLElement>, HTMLElement>;
Activity
andrewbranch commentedon Feb 17, 2023
This is correct only if type checking assumes that the target module has a synthetic default (i.e., does not have the
__esModule
marker).allowSyntheticDefaultImports
(implied byesModuleInterop
) means that the type of a default import from one CJS module to another varies depending on whether the target module was originally written in ESM syntax. If it was, the default import accesses the target’smodule.exports.default
. If it wasn’t, the default import accesses the target’smodule.exports
. The former case can’t be represented byimport x = require("./y")
, but the latter case can. Heurisitics are used during type checking to guess which of these cases will happen at runtime (i.e., which branch of the conditionalreturn (mod && mod.__esModule) ? mod : { "default": mod }
will be taken). We could pull this same heuristic check during declaration emit, and emit an import assignment when the false branch is taken and a default import when the true branch is taken. This would improve the portability of the declaration file produced, but it would be a problem for #47947.remcohaszing commentedon Feb 20, 2023
Once this problem exists in one library, this is likely to “infect” all its dependants too. I think this is partly because TypeScript gives improper hints on how to fix it. Let’s say someone writes a library
my-component
, and they don’t enableesModuleInterop
. TypeScript will show the following error.As stated in the OP, the correct fix for this is to use either
import React = require('react')
orimport * as React from 'react'
. However, TypeScript suggests to enableesModuleInterop
.If the author of
my-component
would follow TypeScript’s suggestion and enableesModuleInterop
, this library can’t be consumed without usingesModuleInterop
, so consumers ofmy-component
are prompted with the same error and suggestion.The consumers of
my-component
have these options:esModuleInterop
, delegating the issue to their dependants.skipLibCheck
, meaning usage ofmy-component
results in implicitany
types.patch-package
to fix the types ofmy-component
in theirnode_modules
.my-component
and hope its author accepts the fix.I’m not familiar with the details from #47947, but it introduces two new compiler options. As far as I understand, it makes sense to make those options mutually exclusive with
allowSyntheticDefaultImports
(and thereforesModuleInterop
).andrewbranch commentedon May 10, 2023
I discussed another solution to this in #54212