hybrids icon indicating copy to clipboard operation
hybrids copied to clipboard

Small Type Inferencing Issue with Property<E, V>

Open au-z opened this issue 10 months ago • 3 comments

Hi @smalluban!

I was working on some side project and noticed that the type inferencing when passing properties as arguments has a small problem.

When passing a computed getter function to a factory function, the property is inferred as a function:

function factory<E, V>(property: Property<E, V>) {
  return property;
}

define<CustomElement>({
  tag: 'custom-element',
  foo: '',
  // inferred incorrectly as Property<CustomElement, (host: CustomElement) => string>
  factoryProperty: factory((host: CustomElement) => host.foo),
})

I was able to reproduce the issue here and I show a possible fix: https://stackblitz.com/edit/vitejs-vite-xmqndb?file=package.json,src%2Fmain.ts,tsconfig.json&terminal=dev

In my experimenting, I found out that rearranging the union type for Property made a difference:

type Property<E, V> =
  | ((host: E & HTMLElement, lastValue: V) => V)
  | (V extends string | number | boolean | undefined ? V : never)
  | Descriptor<E, V>
  | undefined;

If you think re-arranging the union type wouldn't create any unforseen negative side-effects, I'm happy to author a quick PR. 👍

au-z avatar Mar 30 '24 21:03 au-z

First error:

You broke the Component<CustomElement> type by mapping in your interface:

image

Let's fix this:

image

Now we have arrived at the same problem the right way

The factory(() => string) function compares two types: Property<E, V = () => string> and Property<E, V = string> So let's simplify the task to...

type Property<T> =
  | (T extends string | number | boolean | undefined ? T : never)
  | ((lastValue: T) => T)

function factory<T>(value: Property<T>): Property<T> {
  return value;
}

// Type 'string' is not assignable to type '() => string'.ts(2322)
factory(() => "")

type ExtractOrigin<P> = P extends Property<infer Origin> ? Origin : never;


type SecondGeneric = ExtractOrigin<() => string>
// We expect `string` but `string | (() => string)` comes back!

It looks like using type unions along with conditional types breaks typescript.

And your solution actually works for this case, although it looks absurd. I assume that after the conditional type, all other elements of the type union break down. image Therefore, conditional types need to be moved to the end of type unions throughout the code. I found several more such cases in the /** Store **/ section.

Qsppl avatar Mar 31 '24 07:03 Qsppl

Sent a bug report: https://github.com/microsoft/TypeScript/issues/58016

Qsppl avatar Mar 31 '24 21:03 Qsppl

Typescript contributors say that this problem will not be solved in the near future, so we are solving it ourselves.

Qsppl avatar Apr 02 '24 17:04 Qsppl

Do you have a solution, which might be implemented on our side? If not, I would close the issue.

smalluban avatar Jun 25 '24 07:06 smalluban

The solution proposed by @au-z only works because if you have several incompatible candidates for output, then the first one is selected as the assumed type.

This solution fixes the problem in this specific use case, but may create typing errors in other use cases.

Typescript contributors will not fix this issue.

To completely solve the problem, we need to change the types significantly.

You can put this problem aside for later, I can solve it when more problems related to types appear.

Qsppl avatar Jun 26 '24 16:06 Qsppl

Thanks for clarification. Feel free to re-open if you think we can do something with it.

smalluban avatar Jun 26 '24 17:06 smalluban