Declaration file for `@implements` of type specified in `.d.ts` file includes broken syntax

[TimvdLippe]

TypeScript Version: 4.0.0-dev.20200512

Search Terms: implements, jsdoc, declaration

Code

file.js:

/**
 * @implements {Global.Dispatcher}
 */
export class Dispatcher {
  foobar() {
    return 42;
  };
}

defs.d.ts:

declare namespace Global {
  export interface Dispatcher {
    foobar(): number;
  }
}

tsconfig.json:

{
  "compilerOptions": {
    "module": "esnext",
    "target": "esnext",
    "outDir": "lib",
    "declaration": true,
    "allowJs": true,
    "checkJs": true
  },
  "files": [
    "file.js",
    "defs.d.ts"
  ]
}

Expected behavior:
Output declaration file file.d.ts:

/**
 * @implements {Global.Dispatcher}
 */
export class Dispatcher implements Global.Dispatcher {
    foobar(): number;
}
export {};

Actual behavior:

declare const Dispatcher_base: undefined;
/**
 * @implements {Global.Dispatcher}
 */
export class Dispatcher implements Dispatcher_base {
    foobar(): number;
}
export {};

The syntax in this declaration file is broken and shows the following error:

'Dispatcher_base' refers to a value, but is being used as a type here.ts(2749)

Playground Link: n/a

Related Issues: #35629 implemented in #36292 CC Nathan Shively-Sanders (@sandersn) Titian Cernicova-Dragomir (@dragomirtitian)

[TimvdLippe]

I think #36292 missed implementing the declaration part, as the relevant code appears to be https://github.com/microsoft/TypeScript/blame/d07e866a28879f38d8cf7aea2a1b030de26613cc/src/compiler/transformers/declarations.ts#L1365-L1371

I am going to take a stab at that tomorrow, as this issue is blocking various files from being typechecked by TypeScript in DevTools.

[TimvdLippe]

The above code link is incorrect. The _base type shows up because of the logic in serializeBaseType where it can't resolve the implements as a type-reference. Thus it starts using the tempName instead.

I haven't been able to figure out why trySerializeAsTypeReference does not return a type, but I also found out that staticType === undefined, because a class that @implements does not have an extends clause. Thus the logic in serializeAsClass that maps over the implementsTypes uses a staticBaseType that does not exist.

[TimvdLippe]

I only now found out that there is an existing test added in #36292 that already tests this behavior: https://github.com/microsoft/TypeScript/pull/36292/files#diff-06896a35c64292ec693e38ae9d4691a9R7

I am now trying to figure out what the difference is between these two test cases and why one fails and the other one doesn't.

[TimvdLippe]

The difference appears to be the fact that the interface in our example is defined in a namespace. The existing test tests with a global interface.

E.g. this works:

// @filename: defs.d.ts
interface Global_Dispatcher {
    foobar(): number;
}

// @filename: main.js
/**
 * @implements {Global_Dispatcher}
 */
export class DispatcherImpl {
    foobar() {
        return 42;
    };
}

And this fails:

// @filename: defs.d.ts
declare namespace Global {
    export interface Dispatcher {
        foobar(): number;
    }
}

// @filename: main.js
/**
 * @implements {Global.Dispatcher}
 */
export class DispatcherImpl {
    foobar() {
        return 42;
    };
}

[ExE-Boss]

It’s probably related to the fact that @implements doesn’t currently support all the TypeScript type identifiers, case in point:

Seems like type import is not allowed

Originally posted by sKopheK in #35629 (comment)

[TimvdLippe]

Over lunch, I realized that we can take advantage of the subtle difference in the previous comment.

Essentially the following compiles successfully:

// @filename: defs.d.ts
declare namespace Global {
    export interface Dispatcher {
        foobar(): number;
    }
}
interface GlobalWorkaround_Dispatcher extends Global.Dispatcher {}

// @filename: main.js
/**
 * @implements {GlobalWorkaround_Dispatcher}
 */
export class DispatcherImpl {
    foobar() {
        return 42;
    };
}

Solution implemented at https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2208978/

[TimvdLippe]

ExE Boss (@ExE-Boss) They could be related yes. However, TSC correctly checks the actual definition. E.g. it will warn when methods mismatch or when the type doesn't exist. It only goes wrong when it tries to write the actual declaration file.

[sandersn]

ExE Boss (@ExE-Boss) like the implements clause, @implements only supports dotted names, not arbitrary type references.

Not sure if that's related to Tim van der Lippe (@TimvdLippe)'s bug though.

[sandersn]

I have a preliminary fix up at #38688, but it still has a couple of open questions.