typescript-go icon indicating copy to clipboard operation
typescript-go copied to clipboard

difference in typecheck with 5.8 (broken type inference)

Open dartess opened this issue 8 months ago • 1 comments

Same errors, locations, and messages as TS5.8

I got 2 "new" errors on my huge project. I consider it an incredible luck, but I think I should report on the remaining two. here is the first one:

(This is the minimum example I could reproduce with the original error. As soon as I simplify it more I either get another error or they disappear)

type Message = { options?: Record<string, unknown> };

type Shared<T> = { handler: (message: T) => void };
type Static<T> = Shared<T> & { message: T };
type Dynamic<T> = Shared<T> & { message: () => T };

declare function subscribe<TMessage extends Message>(
  payload: (Dynamic<TMessage> | Static<TMessage>) & {},
): void;

subscribe({
  message: () => ({
    options: { market: '' },
  }),
  handler: (message) => message.options.market,
});

export {};

error TS18048: 'message.options' is possibly 'undefined'.

15   handler: (message) => message.options.market,

TS 5.8.3 is fine with this: playground

The easiest way to "fix" this error is to remove the intersection & {} (but in my code there is a type with some fields and I can't)

dartess avatar Apr 15 '25 14:04 dartess

This is an interesting one having to do with our internal representation of union types. Unions are structural types and the order of their constituent types doesn't matter (or at least it isn't supposed to). In its internal representation, the old compiler orders union types by their "type ID", a simple increasing serial number that's assigned when a type is first created, and then shares representation of union types with the same constituents. This works well in general and is key to efficiencies around analyzing union types.

Unfortunately, there are a few situations where the constituent ordering becomes semantically observable. In particular, type inference to a union type proceeds in the order of the constituents of the union type, and when inference produces multiple equally good candidates, we sometimes need to pick the "first" candidate. That in turn means we're depending on union type ordering.

Your repro above is an example of that situation. When inferring to type (Dynamic<TMessage> | Static<TMessage>) & {} we end inferring from type message: { options: { market: string } } first to { message: () => T } and then to { message: T }. That produces two equally good (from the compiler's point of view) inferences for T:

  • { options: { market: string } } and
  • () => { options: { market: string } }.

We then pick the first one, which so happens to be the only good choice. But a simple reordering of the types in your repro causes us to pick the other candidate: Just change the type of payload to (Static<TMessage> | Dynamic<TMessage>) & {} and you now get the same error that you see with tsgo.

And that brings me to the difference between tsc and tsgo. The old scheme of ordering types by their type ID ends up being non-deterministic when we bring concurrency into the picture. Therefore, tsgo introduces a new "total type ordering" that doesn't depend on type creation order. Instead, we depend on inherent characteristics of the types, e.g. their kind, their name, their textual declaration position, etc. The total type ordering is deterministic, but doesn't otherwise improve on the type inference algorithm. And, as (bad) luck would have it, it ends up reversing the inference order in your repro.

Ideally, we'd recognize that only one of the inference candidates are viable, and hopefully some day we can improve the inference algorithm to do so. Meanwhile, you can fix the issue by ensuring that you infer to { message: T | (() => T) }, a situation where the compiler does recognize that one inference is better than the other. For example, with these declarations the issue goes away:

type Payload<T> = Shared<T> & { message: T | (() => T) };

declare function subscribe<TMessage extends Message>(
  payload: Payload<TMessage>,
): void;

This ended up being a longer explanation than I intended, but hopefully it's helpful.

ahejlsberg avatar Apr 25 '25 17:04 ahejlsberg

I just ran into a similar issue when using parameter properties and I think (hopefully) it is relevant for this issue. In package A, I have this class definition:

export class SomeClass {
	// "timestamp" is a parameter property
	constructor(readonly timestamp = new Date()) {}
}

And in package B, I'll get this (false positive?) error:

const o = new SomeClass()
console.log(o.timestamp.getTime()) // <-- error TS18048: 'o.timestamp' is possibly 'undefined'.

However if I change the class definition to:

export class SomeClass {
	// property is no longer declared by a parameter property
	readonly timestamp: Date
	constructor(timestamp = new Date()) {
		this.timestamp = timestamp
	}
}

... the error goes away.

For what it's worth I'm on version 7.0.0-dev.20250704.1

It seems that the error only pops up when the class definition is in a different package 🤔

birgersp avatar Jul 30 '25 11:07 birgersp

Fiddling around with it some more, I see that this code:

export class SomeClass {
	constructor(readonly timestamp = new Date()) {}
}

Is compiled to two different things between tsc (5.8.3) and tsgo (7.0.0-dev.20250704.1).

tsc result (fiddle):

export declare class SomeClass {
    readonly timestamp: Date;
    constructor(timestamp?: Date);
}

tsgo result:

export declare class SomeClass {
    readonly timestamp: Date | undefined;
    constructor(timestamp?: Date | undefined);
}

So, perhaps not the same issue(?)

birgersp avatar Jul 30 '25 11:07 birgersp

Not the same issue, no. You're correct to file separately in #1478.

jakebailey avatar Jul 30 '25 12:07 jakebailey