Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small Type Inferencing Issue with Property<E, V> #249

Open
au-z opened this issue Mar 30, 2024 · 3 comments
Open

Small Type Inferencing Issue with Property<E, V> #249

au-z opened this issue Mar 30, 2024 · 3 comments

Comments

@au-z
Copy link
Contributor

au-z commented Mar 30, 2024

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 au-z changed the title Small Typing Inferencing Issue with Property<E, V> Small Type Inferencing Issue with Property<E, V> Mar 30, 2024
@Qsppl
Copy link
Contributor

Qsppl commented Mar 31, 2024

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
Copy link
Contributor

Qsppl commented Mar 31, 2024

Sent a bug report: microsoft/TypeScript#58016

@Qsppl
Copy link
Contributor

Qsppl commented Apr 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants