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

Infer arrow function type guard type for specific simple cases #38390

Closed
4 of 5 tasks
iliazeus opened this issue May 7, 2020 · 14 comments · Fixed by #57465
Closed
4 of 5 tasks

Infer arrow function type guard type for specific simple cases #38390

iliazeus opened this issue May 7, 2020 · 14 comments · Fixed by #57465
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@iliazeus
Copy link

iliazeus commented May 7, 2020

Search Terms

arrow function type guard

Suggestion

Specific simple cases of arrow functions of a single argument should be considered type guards:

const f1 = x => x typeof "string"; // immediately returns a typeof expression with a literal rhs
const f2 = x => x instanceof Foo; // immediately returns an instanceof expression
const f3 = x => x === "foo"; // immediately returns a strict (non-)equality comparison with a literal
const f4 = x => x == null; // immediately returns a non-strict (non-)equality comparison with a null literal
const f5 = x => isFoo(x); // immediately returns another type guard

There are more general requests like #16069, but I beleive this one should be easier to implement, and it still covers a lot of use cases.

Use Cases

filter with heterogenous arrrays:

// currently: (string | number)[]
// would be: string[]
const a = [1, "foo", 2, "bar"].filter(x => x instanceof "string");

Specifically, filtering out null or undefined elements:

// f: (x: string) => Promise<string | undefined>
const a = await Promise.all(["foo", "bar"].map(f));
const b = a.filter(x => x !== undefined);

Limiting the number of arguments of a more complex type guard:

// isFoo: (x: any, strict?: boolean) => x is Foo
const b1 = a.filter(x => isFoo(x));
const b2 = a.filter(x => isFoo(x, true));

// compare:
const b3 = a.filter(isFoo); // No overload matches this call.

Examples

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels May 8, 2020
@RyanCavanaugh
Copy link
Member

Notably, this would be a breaking change

const a = [1, "foo", 2, "bar"].filter(x => x instanceof "string");
// Currently legal
a.push(10);

@iliazeus
Copy link
Author

iliazeus commented May 8, 2020

Yes, looks like it would, as would any suggestion about type guard inference.

Neither do I really have any ideas of how to avoid that. The most sane one would probably be some kind of a syntactical marker for "treat this function as a type guard, but infer the type if possible, and error if not". Or maybe just an "as narrow a type as you can infer" assertion for expressions, similar to (a part of) what as const does.

const f1 = x => typeof x === "string"; // (x: any) => boolean;
const f2 = (x => typeof x === "string") as narrow; // (x: any) => x is string
// or maybe even
const a = [1, "foo", 2, "bar"].filter(x => x instanceof "string") as narrow; // string[]

But this goes way out of scope of this feature request.

So yeah, this request was probably a bad idea then.

@robertmassaioli
Copy link

The only way to avoid this being a breaking change would be to introduce a not-in-javascript operator that changes the behaviour of the type inference OR to add in a typescript compiler --strictInference opt-in flag. I don't know what the Typescript appetite for either would be. Or what the priority of this would be.

@vultix
Copy link

vultix commented Jun 1, 2020

@iliazeus What about this syntax meaning inferred type predicate?

// a is a string[]
const a = [1, "foo", 2, "bar"].filter((x): x is ? => x instanceof "string");

// b is a (string | number)[]
const b = [1, "foo", 2, "bar"].filter(x => x instanceof "string");

The only new syntax is the x is ? predicate, which would fail to compile if the predicate type cannot be inferred. This makes it opt-in and a non-breaking change.

We can optionally use something other than the ?. I also like _.

Thoughts @iliazeus @RyanCavanaugh

@isaiahtaylorhh
Copy link

This would be a very valuable step to take, but I agree with the inference cautions. There needs to be something syntactical that marks that you're intending predicate inference.

As far as x is ?, I don't think that communicates what's happening very clearly. I would advocate a keyword that is more explicit and clear:

// a is a string[]
const a = [1, "foo", 2, "bar"].filter(infer (x) => x instanceof "string");

// OR...
// a is a string[]
const a = [1, "foo", 2, "bar"].filter(predicate (x) => x instanceof "string");

// b is a (string | number)[]
const b = [1, "foo", 2, "bar"].filter(x => x instanceof "string");

@acutmore
Copy link
Contributor

acutmore commented Nov 18, 2020

Personally I don't feel the desire for type hint syntax.

A new --stictXXXX flag added seems inline with other breaking changes to TS's type inference. It would be interesting if someone could produce some stats on the number of times this would trigger a breaking change when sampled on a group of open-sources repos, also if the error actually seemed to catch a potential bug.

Anecdotally I have come across code like this a few times:

declare let someArray: (string | null)[];
declare function useString(s: string): void;

someArray
  .filter(v => v !== null)
  .forEach(v => {
     useString(v!); // unnessesary null assertion to stop strictNullError
  });

With added syntax required I can see people still being inclined to continue to just add the null assertion, this new 'infer type guard' syntax will possibly be seen as 'advanced syntax' that not everyone is aware of.

When code is written in a structured style TS infers the type guard without syntax:

for (const v of someArray) {
    if (v !== null) {
        useString(v); // value is inferred correctly without type syntax
    }
}

This all said, I do feel that the infererence should only be done for inline callbacks where the function lists a type guard as one of the possible parameters. Otherwise I could see it being a performance issue on large code bases.

// no type guard infered
const f = (v: any) => typeof v === 'string;

// type guard infered because of filter's type being:
//   filter<S extends T>(cb: (value: T) => value is S): S[];
arr.filter(v => typeof v === 'string) 

@haltman-at
Copy link

Hm, so now that TS 4.4 added #44730, could this happen too? They seem at least vaguely related to this non-expert...

@ffMathy
Copy link

ffMathy commented Sep 15, 2021

I can't wait for this <3

I am curious, what kind of feedback is needed?

@asktree
Copy link

asktree commented Mar 13, 2022

re @RyanCavanaugh 's breaking change example:
it seems to me that if a is readonly then there would be no breaking change. Maybe we could simply infer / refine only in that case then?

Likewise, as somebody who never mutates data in their code, a --strictInference flag sounds great, as I tend to only ever wish for more inference.

@Tiedye
Copy link

Tiedye commented Jun 2, 2022

If implemented would this flag also cover this case? #14826

@PMLP-novo
Copy link

I know that this is blocked by introducing breaking changes.

But has anybody a real world example of production code, that would break? It seems like a close to unreachable corner case.

Secondly if somebody where to reach this corner case they could very quickly resolve it and would notice it right away when they upgrade typescript.

This question has been asked so many times and so many people spend time discussing it and having a bad typescript experience.

So I argue that this is a case where the right solution is to introduce a breaking change.

@jcalz
Copy link
Contributor

jcalz commented Sep 17, 2022

There's a lot of instanceof "string" in the example code here and it gave me a brief feeling of vertigo. "Oh no, is this the Mandela Effect? Does instanceof "string" work in some new universe in which I've found myself?" But no, I just checked, I'm in the right place; "abc" instanceof "string" produces a TypeError at runtime. Maybe this issue has been transported from another plane of existence?

@manniL
Copy link

manniL commented Jul 11, 2023

IMO thoughts regarding a breaking changes are valid, but options like a compiler flag (+ gracefully defaulting to it in a few more major versions) looking like a fair tradeoff, especially given the improved DX.

@msakrejda
Copy link

I've also run into this a number of times. In the past, I just used a ! assertion to work around it. Now I write a user type predicate. But it'd be nice not to have to do that for simple cases.

The breaking changes don't seem like they would cause problems for many valid cases, and a feature flag seems like a reasonable workaround (though to be fair, I'm not familiar with TS breaking change or feature flag policies—anyone know where I can learn more?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.