microsoft / TypeScript

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

JS: Name resolution of classes is broken #32993

Closed sandersn closed 5 years ago

sandersn commented 5 years ago

TypeScript Version: 3.4.0-dev.201xxxxx

Search Terms:

Code

Run this:

$ npm install @types/node

Have a tsconfig like this:

{
    "compilerOptions": {
        "noEmit": true,
        "strict": true,
        "target": "esnext",
        "lib": ["es5", "es2015", "es2016", "es2017", "esnext", "dom"],
        "allowJs": true,
        "checkJs": true,
        "outDir": "out",
        "jsx": "preserve",
        "esModuleInterop": true,
        "module": "commonjs",
    },
    "files": [
        "welove.ts",
        "mod1.js"
    ]
}

In mod1.js:

const EventEmitter = require('events')
class Worker extends EventEmitter {
}

module.exports = {Worker};

In welove.ts:

import { Worker } from './mod1'
function f(w: Worker) {
}
var wp = new Worker()
f(wp)

Expected behavior: type reference and value reference to Worker refer to the class defined in mod1.js.

Actual behavior: type reference Worker refers to the one in lib.dom.d.ts.

Two variants:

  1. If you convert mod1.js to typescript with the equivalent export { Worker }, the error goes away.
  2. If you use the syntax module.exports.Worker = class Worker extends EventEmitter, the error goes away, but goto-def on the value Worker throws an assertion:
Error: Debug Failure. Expected declaration to have at least one class-like declaration
    at getConstructSignatureDefinition (/home/nathansa/ts/built/local/tsserver.js:107575:89)
    at getDefinitionFromSymbol (/home/nathansa/ts/built/local/tsserver.js:107570:20)
    at Object.getDefinitionAtPosition (/home/nathansa/ts/built/local/tsserver.js:107439:20)
    at Object.getDefinitionAtPosition (/home/nathansa/ts/built/local/tsserver.js:125773:38)
sandersn commented 5 years ago

@weswigham I'm assigning to you because I'm pretty sure this is a result of https://github.com/microsoft/TypeScript/pull/32947. If it's actually a result of contructor-functions-as-classes, re-assign to me.

weswigham commented 5 years ago

@sandersn this was broken in TS 3.5, too (and probably stretches back even farther)~

image

On inspection, yeah - also broken in 3.1.

sandersn commented 5 years ago

It's a duplicate of commonjs-exports-should-be-aliases, #25533, although this is a particularly good example of it.