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

TypeScript generated invalid types if esModuleInterop is enabled #52779

Open
remcohaszing opened this issue Feb 15, 2023 · 3 comments
Open

TypeScript generated invalid types if esModuleInterop is enabled #52779

remcohaszing opened this issue Feb 15, 2023 · 3 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@remcohaszing
Copy link

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>;
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 17, 2023
@andrewbranch
Copy link
Member

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 by esModuleInterop) 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’s module.exports.default. If it wasn’t, the default import accesses the target’s module.exports. The former case can’t be represented by import 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 conditional return (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
Copy link
Author

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 enable esModuleInterop. TypeScript will show the following error.

MyComponent.tsx:1:8 - error TS1259: Module '"node_modules/@types/react/index"' can only be default-imported using the 'esModuleInterop' flag

1 import React from 'react'
         ~~~~~

  node_modules/@types/react/index.d.ts:63:1
    63 export = React;
       ~~~~~~~~~~~~~~~
    This module is declared with 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.

As stated in the OP, the correct fix for this is to use either import React = require('react') or import * as React from 'react'. However, TypeScript suggests to enable esModuleInterop.

If the author of my-component would follow TypeScript’s suggestion and enable esModuleInterop, this library can’t be consumed without using esModuleInterop, so consumers of my-component are prompted with the same error and suggestion.

The consumers of my-component have these options:

  1. Follow the suggestion to enable esModuleInterop, delegating the issue to their dependants.
  2. Enable skipLibCheck, meaning usage of my-component results in implicit any types.
  3. Use something like patch-package to fix the types of my-component in their node_modules.
  4. Fix the bug upstream in 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 therefor esModuleInterop).

@andrewbranch
Copy link
Member

I discussed another solution to this in #54212

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

3 participants