knockout / tko

🥊 Technical Knockout – The Monorepo for Knockout.js (4.0+)
http://www.tko.io
Other
273 stars 34 forks source link

Fix typing of the Observable<boolean> #168

Open brianmhunt opened 2 years ago

brianmhunt commented 2 years ago

See:

// Broken
function observable<T>(value: T): Observable<T>
{
  return undefined as any;  // the implementation is not important
}

// Fixed
function observable<T>(value: T extends infer U ? U: never): Observable<T>
{
  return undefined as any;  // the implementation is not important
}
maskmaster commented 2 years ago

I added an answer in the stackoverflow question. We need to make sure that these two lines remain valid after any fix.

const x = observable<false | undefined>(false);
const y = observable(false);
tomhanax commented 2 years ago

For now the best I can offer is:

// Fixed
function observable<T, U = any>(value?: T extends infer R ? R : U): unknown extends T ? Observable<U> : Observable<T>
{
  return undefined as any;  // the implementation is not important
}

The playground contains many examples and seems OK, of course anybody can check and correct if needed, I am no expert in this.

Playground Link

brianmhunt commented 2 years ago

Thanks @maskmaster , @tomhanax the answer & playground are very helpful.

direct link to the SO question

chuanqisun commented 2 years ago

Hi, I believe I'm hitting the same issue when upgrading to typescript 4.6 but I might need some help understanding the workaround. Here is a minimum repro.

image

The ExtendedType is clearly assignable to the BaseType, but typescript wouldn't allow. Also, the error suggests that typescript is trying to assign BaseType to ExtendedType, which is opposite to the desired direction.

Should I manually patch the type definitely of KnockoutObservable? I'm not sure how that might work. Thank you and let me know if I should cross post to other repos.

mbest commented 1 year ago

This was fixed in TypeScript: https://github.com/microsoft/TypeScript/pull/48380