Inconsistent errors reported between tsc and VSCode (Unions across objects with method overload definitions)

[frank-weindel]

Bug Report

This is similar to #52813, in spirit. However the errors, how it's reproduced, and the code causing it are pretty different.

Basically I have a code snippet that appears 100% error-free* (most of the time, see note below) in the VSCode IDE. Logically in my head the code should be error free too. However, when I run tsc I get an error inside the snippet.

Here's the code:

export function createWebGLContext(
  canvas: HTMLCanvasElement | OffscreenCanvas,
): WebGLRenderingContext | null {
  return (
    canvas.getContext('webgl')
  )
}

tsc reports this error:

createWebGLContext.ts:4:3 - error TS2322: Type 'RenderingContext | null' is not assignable to type 'WebGLRenderingContext | null'.
  Type 'CanvasRenderingContext2D' is missing the following properties from type 'WebGLRenderingContext': drawingBufferHeight, drawingBufferWidth, activeTexture, attachShader, and 428 more.

4   return (
    ~~~~~~~~
5     canvas.getContext('webgl')
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
6   )
  ~~~

* In the larger project that I originally discovered this issue, I actually do sometimes see the error in the IDE. I haven't figured out how to consistently cause it, but restarting the TS server or making a small change and saving the file generally clears the error.

🔎 Search Terms

inconsistency, inconsistent, errors, ts language server, tsc

🕗 Version & Regression Information

• Attempted on 5.0.3 and latest dev build (typescript@5.1.0-dev.20230331)

⏯ Playground Link

With the Playground, since it uses elements of VSCode under the hood, the snippet appears valid with no errors.
Playground link with relevant code

You can also clone this repo or start a Github CodeSpace on it to reproduce the errors when running tsc:
https://github.com/frank-weindel/ts-53614-ide-tsc-inconsistency

🙁 Actual behavior

• tsc considers the above code to have an error.
• VSCode normally and properly does not show that error, but it occasionally can.

🙂 Expected behavior

• tsc should not consider above code to have an error.
• VSCode should be consistent with tsc.

Analysis

I'm not sure why but this seems related to the use of the union HTMLCanvasElement | OffscreenCanvas for the variable canvas.

Both interfaces in the union contain a set of method overloads for getContext...

lib.dom.d.ts

// ...
interface HTMLCanvasElement extends HTMLElement {
    // ...
    getContext(contextId: "2d", options?: CanvasRenderingContext2DSettings): CanvasRenderingContext2D | null;
    getContext(contextId: "bitmaprenderer", options?: ImageBitmapRenderingContextSettings): ImageBitmapRenderingContext | null;
    getContext(contextId: "webgl", options?: WebGLContextAttributes): WebGLRenderingContext | null;
    getContext(contextId: "webgl2", options?: WebGLContextAttributes): WebGL2RenderingContext | null;
    getContext(contextId: string, options?: any): RenderingContext | null;
    // ...
}
// ...
interface OffscreenCanvas extends EventTarget {
    // ...
    getContext(contextId: "2d", options?: any): OffscreenCanvasRenderingContext2D | null;
    getContext(contextId: "bitmaprenderer", options?: any): ImageBitmapRenderingContext | null;
    getContext(contextId: "webgl", options?: any): WebGLRenderingContext | null;
    getContext(contextId: "webgl2", options?: any): WebGL2RenderingContext | null;
    getContext(contextId: OffscreenRenderingContextId, options?: any): OffscreenRenderingContext | null;
    // ...
}
// ...

Based on these method overload signatures I'd expect the statement canvas.getContext('webgl') to use the union of these two overrloads:

getContext(contextId: "webgl", options?: any): WebGLRenderingContext | null; // from HTMLCanvasElement
getContext(contextId: "webgl", options?: WebGLContextAttributes): WebGLRenderingContext | null; // from OffscreenCanvas

Which I believe would reduce down to look like this:

getContext(contextId: "webgl", options?: any): WebGLRenderingContext | null;

Where the return type is WebGLRenderingContext | null. Which is what we see if we hover over the method name in the Playground or VSCode.

However, tsc inferred the return type of the method to be RenderingContext | null.

RenderingContext | null is returned by the base method signature for getContext() in HTMLCanvasElement:

getContext(contextId: string, options?: any): RenderingContext | null;

So it would seem tsc somehow is inferring the wrong override while TS Language Server / VSCode is inferring the correct one.

[RyanCavanaugh]

It's worse than this; there's a tsc ordering sensitivity as well originating in the same problem

declare let cv: HTMLCanvasElement | OffscreenCanvas;
declare let oc: OffscreenCanvas;

// Swapping these two unrelated lines changes errorness
oc.getContext('webgl');
const g: WebGLRenderingContext | null = cv.getContext('webgl');

[RyanCavanaugh]

Here's a reduced version that doesn't need the DOM types

interface Alpha {
    foo(type: "num", opts?: unknown): number;
    foo(type: "str", opts?: unknown): string;
    foo(type: string, opts?: unknown): number | string;
}

interface Beta {
    foo(type: "num", opts?: { a?: string }): number;
    foo(type: "str", opts?: { b?: string }): string;
    foo(type: "num" | "str", opts?: unknown): number | string;
}

declare let b: Beta;
// Commenting this line out causes an error on the bottom line
const b_res = b.foo("str");

declare let ab: Alpha | Beta;
const x: string = ab.foo("str");

[gabritto]

The problem in the minimal repro is a combination of two things: first, that overloaded signatures are order-dependent, so depending on the order of the component types Alpha and Beta in the internal representation of the union Alpha | Beta, we get a different order for the resulting signatures of foo on type Alpha | Beta.

Looking at the example, if when computing the type of ab.foo, signature foo(type: "str", opts?: { b?: string }): string occurs before signature foo(type: "str", opts?: unknown): string in the union type, then everything works correctly and the error doesn't happen, because we end up with the following signature list for ab.foo (after processing each union component's signatures):

type CallSignatures2 = {
    (type: "num", opts?: { a?: string | undefined; } | undefined): number
    (type: "str", opts?: { b?: string | undefined; } | undefined): string
    (type: "num" | "str", opts?: unknown): string | number
    (type: "num", opts?: unknown): string | number
    (type: "str", opts?: unknown): string | number
}

So the ab.foo("str") call matches signature (type: "str", opts?: { b?: string | undefined; } | undefined): string and the return type is string, and everything is fine.

However, if the opposite happens, i.e. signature foo(type: "str", opts?: unknown): string occurs before foo(type: "str", opts?: { b?: string }): string in the union type Alpha | Beta, then we have an issue. We end up with the following signatures for foo:

type CallSignatures = {
    (type: "num", opts?: unknown): string | number // Notice the return type is now `string | number`
    (type: "str", opts?: unknown): string | number // Notice the return type is now `string | number`
    (type: "num", opts?: { a?: string | undefined; } | undefined): number
    (type: "str", opts?: { b?: string | undefined; } | undefined): string
    (type: "num" | "str", opts?: unknown): string | number
}

Here the second factor comes into play: how we process each component type's signatures in a union type to try and get a signature for the whole union.
When you have ab of type Alpha | Beta, and we need to figure out the type of ab.foo, we need to combine Alpha's signatures for foo with Beta's signatures for foo.
The way we do this for a union type, in general, is that:
For each signature s of each component type, we try to find a signature that matches s in each component type in the union. If we find those matching signatures, we combine them (by, among other things, unioning their return types together) into a new signature, and that new signature is what is present in the union type's signatures. If we don't find matching signatures in every component in the union, we discard that signature from the result.

In the case of signature foo(type: "str", opts?: unknown): string from type Alpha, we match it to itself in component Alpha of the union Alpha | Beta, and we match it to signature foo(type: "num" | "str", opts?: unknown): string | number. So as a result, we combine those two matches into signature foo(type: "str", opts?: unknown): string | number and put this result into the foo signatures on type Alpha | Beta. This is because, if you call foo on type Alpha and pass in an opt, you get something of type string, and if you do it on type Beta, you get something of type string | number. However, if you call foo on type Alpha and don't pass an opt argument (because it is optional), then you get type string, and if you call foo on type Beta and don't pass an opt argument, then you'd get type string as a result. But the way we combine signatures in a union isn't precise enough to capture that distinction. (If we split the signature with an optional parameter into two separate signatures, then the issue is fixed. See code here)

So tl;dr I think there are two issues here: (1) when combining signatures from a union type's component, we don't try to order them relative to other components' signatures by how specific their parameter types are, so in the resulting union type's signatures, a less specific signature can appear first, and (2) when finding signature matches for a signature with optional parameters to get a resulting signature in a union, we don't distinguish between that parameter being optional or not, resulting in a less precise signature.

[gabritto]

Re: how having b.foo("str") changes things, it switches the order of Alpha and Beta in the internal representation of union Alpha | Beta, because it affects the order of the signatures in the union. The same thing happens if you comment that off but switch the order of the declarations Alpha and Beta: the error goes away.

[RyanCavanaugh]

Further reduced. Will provide more commentary later

interface Alpha {
    foo(n: "str", opt?: boolean): string;
}
// Comment/uncomment to change errorness
type B = Beta['foo'];

interface Beta {
    foo(n: "str", opt?: true): string;
    foo(n: string): number | string;
//  ^^^ <- not the one we should pick to unify
//         with Alpha#foo, sort of.
}

declare let ab: Alpha | Beta;
const x: string = ab.foo("str");

[RyanCavanaugh]

I was going to try to simplify @gabritto's comment but I think it already is as simple as it gets, modulo the slightly simpler example above.

There are basically only two possible fixes here:

• Change the signature merging algorithm to be fully order-agnostic. We don't really have any ideas on what that might look like; the current algorithm is sound in both cases but generates different completeness depending on which signature set comes first
• Change the union ordering algorithm to be more stable. We don't really have any ideas on what that might look like either; any sorting order is liable to introduce other observable instabilities (e.g. if we sorted alphabetically, then renaming a type might cause a type error when there previously wasn't one). Given that the signature merging algorithm is already order-sensitive, enforcing a new sort order here is also liable to be a breaking change in existing code since there may well be hidden dependencies on current behavior

If someone can come up with a good order-agnostic signature merging algorithm that has reasonable performance, we'd welcome a PR.

But given that this has been the behavior since at least 3.3 and probably earlier, and this is the first report, and we don't have any good solutions on the table, this will have to go to the Backlog.

Aside: A much more approachable fix is to just adjust the lib DOM types to make them not repro this problem, possibly by splitting out the optional parameters into separate signatures as suggested above. Also accepting PRs for that!

[dylangrandmont]

But given that this has been the behavior since at least 3.3 and probably earlier, and this is the first report, and we don't have any good solutions on the table, this will have to go to the Backlog.

I don't mean to spam but I want to add some visibility to this issue. I'm surprised this hasn't been reported more often since it's quite common for rendering operations in TS to pass around a union like HTMLCanvasElement | OffscreenCanvas.

I upgraded a project from TS 4.3.4 (where I've never encountered this issue) to 5.0.4 where I encountered an equivalent issue from the following snippet

function createCanvas(width: number, height: number) {
  return supportsOffscreenCanvas() ? new OffscreenCanvas(width, height) : document.createElement('canvas');
}

const c = createCanvas();
c.getContext('2d')?.drawImage(img, 0, 0);

Type error: Property 'drawImage' does not exist on type 'OffscreenCanvasRenderingContext2D | RenderingContext'.
  Property 'drawImage' does not exist on type 'ImageBitmapRenderingContext'.

Is there a suggested work-around for this issue at this time?

[controversial]

Like @dylangrandmont, I’ve been encountering this issue pretty frequently in my own code in the case of a HTMLCanvasElement | OffscreenCanvas union.

I use classes of the form:

export default class Renderer {
  ctx: CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D;

  constructor(
    public canvas: HTMLCanvasElement | OffscreenCanvas,
  ) {
    const ctx = canvas.getContext('2d');
    if (!ctx) throw new Error('Could not get 2D context from canvas');
    // fails unpredictably
    this.ctx = ctx;
  }

  draw() {
    this.ctx.fillStyle = 'red';
    this.ctx.fillRect(0, 0, 100, 100);
  }
}

throughout several work projects. Since, like the original commenter, the issue doesn’t always appear in my editor (vscode), my experience ends up usually being that a build fails basically at random and without warning, and it’s hard for me to predict whether TypeScript will infer the type of ctx

• correctly, as CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D | null or
• too broadly, as OffscreenCanvasRenderingContext2D | RenderingContext | null

The best workaround I have is just to use type assertion:

// https://github.com/microsoft/TypeScript/issues/53614
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const ctx = canvas.getContext('2d') as CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D | null;