microsoft / TypeScript-Website

The Website and web infrastructure for learning TypeScript
https://www.typescriptlang.org
Creative Commons Attribution 4.0 International
2.22k stars 1.37k forks source link

Docs: documented use of function return type conditional on argument type is impractical #1931

Closed msbit closed 1 year ago

msbit commented 3 years ago

Page URL:

https://www.typescriptlang.org/docs/handbook/2/conditional-types.html

Issue:

The example showing use of return type conditional on argument type near https://github.com/microsoft/TypeScript-Website/blob/3cb94f4f302dba9df56656ae5fa0ab808ab3f1fc/packages/documentation/copy/en/handbook-v2/Type%20Manipulation/Conditional%20Types.md#L82 will compile, but as indicated by the trajectory of https://github.com/microsoft/TypeScript/issues/24929 a type safe function body is not possible.

Recommended Fix:

I can see a few options, either:

edokeh commented 2 years ago

+1

spent much time on this impossible function body

orta commented 2 years ago

Yeah, makes sense, we should probably swap it out with:

```ts twoslash
interface IdLabel {
  id: number /* some fields */;
}
interface NameLabel {
  name: string /* other fields */;
}
type NameOrId<T extends number | string> = T extends number
  ? IdLabel
  : NameLabel;
// ---cut---
function createLabel<T extends number | string>(idOrName: T): NameOrId<T> {
  if (typeof idOrName === "number" ) {
    return { id: idOrName } as NameOrId<T>
  } else {
    return { name: idOrName } as NameOrId<T>
  }
}

let a = createLabel("typescript");
//  ^?

let b = createLabel(2.8);
//  ^?

let c = createLabel(Math.random() ? "hello" : 42);
//  ^?
whschultz commented 2 years ago

Yeah, makes sense, we should probably swap it out with:

interface IdLabel {
  id: number /* some fields */;
}
interface NameLabel {
  name: string /* other fields */;
}
type NameOrId<T extends number | string> = T extends number
  ? IdLabel
  : NameLabel;
// ---cut---
function createLabel<T extends number | string>(idOrName: T): NameOrId<T> {
  if (typeof idOrName === "number" ) {
    return { id: idOrName } as NameOrId<T>
  } else {
    return { name: idOrName } as NameOrId<T>
  }
}

let a = createLabel("typescript");
//  ^?

let b = createLabel(2.8);
//  ^?

let c = createLabel(Math.random() ? "hello" : 42);
//  ^?

I find the following works just fine:

interface IdLabel {
    id: number /* some fields */;
}
interface NameLabel {
    name: string /* other fields */;
}

// If you pass a number, you get `IdLabel`
function createLabel(idOrName: number): IdLabel;
// If you pass a string, you get `NameLabel`
function createLabel(idOrName: string): NameLabel;
// If you pass an ambiguous type, you get an ambiguous type.
// Yes, this extra overload needs to be here, if you intend to be
// able to call it this way.  Otherwise, the implementation signature
// isn't externally visible.
function createLabel(idOrName: number | string): IdLabel | NameLabel;

// The implementation has to be able to handle all of the above
// defined possibilities.
function createLabel(idOrName: number | string): IdLabel | NameLabel {
    if (typeof idOrName === "number") {
        return { id: idOrName };
    } else {
        return { name: idOrName };
    }
}

const a: NameLabel = createLabel("typescript");
const b: IdLabel = createLabel(2.8);
const c: NameLabel | IdLabel = createLabel((Math.random() >= 0.5) ? "hello" : 42);
wbt commented 2 years ago

The prime example given in official example documentation for conditional types is not implementable. The simplest implementation of that Deferred Conditional Type example is:

const getID = function<T extends boolean>(fancy: T): T extends true ? string : number {
    if(fancy) {
        return 'StringID';
    } else {
        return 5;
    }
};

but that fails to compile, even though on lines 3 and 5 where the error occurs the true/false value of 'fancy' is known at compile-time and that should permit TypeScript to figure out the expected type of the return value.

24929 remaining closed suggest this won't be fixed and it's a big sore spot when trying to implement something like the example.

typescript-bot commented 1 year ago

Hello! As per #2804, we are automatically closing all open issues. Please see #2804 for a description of what issues and PRs can be accepted going forward.