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

SUGGESTION: Set and Map '.has' method as type guard #18781

Closed
jdavidls opened this issue Sep 26, 2017 · 8 comments
Closed

SUGGESTION: Set and Map '.has' method as type guard #18781

jdavidls opened this issue Sep 26, 2017 · 8 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@jdavidls
Copy link

This is a small suggestion, but I think it can be interesting to bring this type of characteristics to the spirit of the typescript language.

Code

function actual(value: string|number) { 
    let numbers = new Set<number>();

    if (numbers.has(value))  // Error: Type 'string' is not assignable to type 'number'.
        value  
         // we need to do
    if (numbers.has(value as number)) 
        value = value as number 
}

declare class MySet<T> {
    has(elem: any): elem is T;
}

function suggested(value: string|number) 
    let numbers = new MySet<number>();

    if (numbers.has(value)) // OK, we are doing a legal checking
        value // <= value is a 'number'
}

thank you for your attention.

@jcalz
Copy link
Contributor

jcalz commented Sep 27, 2017

I don't think this is a great idea, for a few reasons. The most obvious one is this:

declare let numbers: Set<number>;
declare let value: string | number; 
if (numbers.has(value)) {
  value + 2 // narrowed to number, okay
} else {
  value.charAt(0); // narrowed to string?!
}

If a Set<T> doesn't contain a value, then the value isn't of type T?

Less obvious but possibly more important is that such a declaration subverts the point of type checking. Mistakes like this will no longer be caught:

let nums = [0, 1, 2]; // array of numbers
numbers.has(nums); // oops, no error!

So I wouldn't want to see this added to the standard libraries.


If these objections don't sway you and you still find this change useful for your own work, you could always modify or augment your local declaration of Set:

// put this in your own code to reap its benefits
interface Set<T> {
    has(elem: any): elem is T;
}

Cheers!

@mhegazy
Copy link
Contributor

mhegazy commented Sep 28, 2017

Duplicate of #13086

@mhegazy mhegazy marked this as a duplicate of #13086 Sep 28, 2017
@mhegazy mhegazy added the Duplicate An existing issue was already created label Sep 28, 2017
@jcalz
Copy link
Contributor

jcalz commented Sep 28, 2017

It's not a duplicate; #13086 is about performing a get() after a has(). This is... something else.

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript and removed Duplicate An existing issue was already created labels Sep 28, 2017
@DanielRosenwasser
Copy link
Member

Is there a very compelling scenario for this? I get the idea, but like @jcalz said, this means you could easily screw up and pass anything accidentally which lead to some frustrating behavior.

There's nothing super wrong with adding your own overload if you feel it'd help you out a lot.

@fasiha
Copy link

fasiha commented Aug 9, 2018

Wouldn't the narrower use-case, where has is just used just as non-undefined typeguard, still be really useful? I often have situations along the lines of:

function foo(map: Map<String, String>, key: String): String {
  if (map.has(key)) {
    return map.get(key); // Type 'String | undefined' is not assignable to type 'String'.
  }
  return "";
}

It feels odd and incomplete that TypeScript doesn't know that map.get(key) in the if's clause must return not-undefined thanks to that has.

@jcalz
Copy link
Contributor

jcalz commented Aug 9, 2018

@fasiha you are looking for #13086

@RyanCavanaugh RyanCavanaugh added the Too Complex An issue which adding support for may be too complex for the value it adds label Aug 13, 2018
@RyanCavanaugh
Copy link
Member

Agree with @jcalz this doesn't seem like a good solution (to say nothing of the problem)

@avin-kavish
Copy link

avin-kavish commented Aug 24, 2022

interface Set<T> {
    has<U extends T>(elem: U): elem is U;
    has(elem: any): boolean; // wider case passes through
}

Wouldn't this prevent the else case problem that @jcalz is describing?

Edit: I actually tested it and it prevents wider type narrowing, but it also causes the narrow type to resolve to never in the else case.

declare let numbers: Set<number>
declare let value: boolean | number
declare let value2: number

if (numbers.has(value)) {
  value + 2 // no narrowing
} else {
  value.charAt(0)  // no narrowing
}
if (numbers.has(value2)) {
  value2 + 2 // number as is.
} else {
  value2.charAt(0) // narrowed to never
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

7 participants