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

allow storing results of narrowing in booleans for further narrowing #24865

Closed
zpdDG4gta8XKpMCd opened this issue Jun 11, 2018 · 11 comments · Fixed by #44730
Closed

allow storing results of narrowing in booleans for further narrowing #24865

zpdDG4gta8XKpMCd opened this issue Jun 11, 2018 · 11 comments · Fixed by #44730
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Jun 11, 2018

Search Terms

https://github.com/Microsoft/TypeScript/search?q=type+guards+constants+narrowing&type=Issues

Suggestion

There is no way to use a saved result of a type guard evaluation for narrowing. Allow narrowing by

  • storing assertions (booleans) of narrowing into constants
  • impose narrowing based on the values carried by those constants

Use Cases

  • reuse for narrowing evaluations, in case there are two places that narrow using the same guard on the same value
  • declutter "if" expressions
  • give names to narrowing facts for better readability

Examples

// now
            if (
                isLoneAvailableCapacity(feed) &&
                (isAvailableCapacitySubbeamPath(link) || isAvailableCapacityConnectivityLegPath(link)) &&
                toAvailableCapacityKey(feed.availableCapacity) === link.availableCapacityKey
            ) {


// could be
            const isAvailableCapacityPairdWithPath = isLoneAvailableCapacity(feed) &&
                (isAvailableCapacitySubbeamPath(link) || isAvailableCapacityConnectivityLegPath(link)) &&
                toAvailableCapacityKey(feed.availableCapacity) === link.availableCapacityKey

            if (isAvailableCapacityPairdWithPath) {
            }

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. new expression-level syntax)
@mhegazy mhegazy added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Jun 11, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jun 11, 2018

looks like you want the compiler to automatically infer dependent types for you :)

@zpdDG4gta8XKpMCd
Copy link
Author

i merely ask to extend booleans (in the local scope) with knowledge of what's been narrowed

@hraban
Copy link

hraban commented Jun 15, 2018

If I understand it correctly this is similar to #24983 ? I don't see a typeof keyword here but is that what this is about?

like this? :

function twostep(x: symbol | string) {
    const isSymbol = typeof x === "symbol";
    if (!isSymbol) {
        console.log(`Type of x is still string | symbol here: ${x}`);
    }
}

function onestep(x: symbol | string) {
    if (typeof x !== "symbol") {
        console.log(`Type of x is now just string: ${x}`);
    }
}

@ajafff
Copy link
Contributor

ajafff commented Jun 15, 2018

Instead of inferring boolean for the intermediate variable, the TypeChecker could infer a type predicate. That could look similar to type predicates in type guard functions. Type guard types would be a subtype of boolean and can therefore be used like any other boolean, but with the added benefit of narrowing another variable.

declare function isNumber(v: any): v is number;

function infer(x: string | number) {
  // currently inferred as 'boolean'
  // now inferred as 'x is number';
  const intermediate = isNumber(x);
  if (intermediate) {
    x.toFixed();
  } else {
    x.substr();
  }
}

// maybe also allow this syntax in type position
function syntactic(x: string | number) {
  const intermediate: x is number = doSomething();
  if (intermediate) {
    x.toFixed();
  } else {
    x.substr();
  }
}

Caveat: the inferred type predicate needs to be invalidated at the next assignment of the narrowed variable (x). When dealing with object types this becomes more difficult, because assigning a property could also invalidate the type predicate.

@johnnyreilly
Copy link

I'd love to see this worked on. The motivation for me is this:

give names to narrowing facts for better readability

Code readability is very important; at present the behaviour of type narrowing enforces a code style which doesn't allow for meaningful variable names to drive control flow whilst maintaining narrowed types. I super miss that.

@Kotarski
Copy link

Kotarski commented Sep 14, 2018

Completely agree, I've found myself wanting to do this a lot as I like keeping the main body of logic as simple and human readable as possible.
A simple example is (where add6 causes an error):

   function add4(n: unknown): number | undefined {
      if (typeof n === "number") {
         return n + 4;
      } else {
         return undefined;
      }
   }

   function add6(n: unknown): number | undefined {
      const nIsNumber = typeof n === "number";
      if (nIsNumber) {
         return n + 6;
      } else {
         return undefined;
      }
   }

The way I can see of resolving it is (as @ajafff said) to make type predicates (x is y) into proper types, so in add6 "nIsNumber" would be of type "n is number" rather than type "boolean".

Note that this example is simplified to illustrate the problem but isn't a case where it would be particularly necessary.

@justingrant
Copy link
Contributor

It'd be great if TS solved this problem. I strongly agree with @johnnyreilly that this limitation causes less-readable code.

Note that the problem is broader than TypeScript-specific constructs like typeof and user-defined type guards. It also complicates migrating existing JavaScript code to TS when using common JS idioms like (example 2 below) preventing property access on a may-be-null object. It even breaks switch statements (example 3 below) where there's no intermediate local variables at all!

Also, the problem doesn't only apply to boolean values. It applies to values of any type that may be used in a truthy/falsy context, like examples 2 and 3 below.

function repro (a: string[] | null) { 

  // example 1 (no error)
  if (a && a.length===1) {
    console.log (a.length); // no compiler error
  }

  // example 2 (error)
  const test = a && a.length===1;
  if (test) {
    console.log (a.length); // compiler error: Object is possibly null
  }

  // example 3 (error)
  switch (a && a.length) {
    case 1: 
      console.log (a.length); // compiler error: Object is possibly null
    default: 
      return null;
  }

}

@justingrant
Copy link
Contributor

BTW, I think this is a dupe of #12184.

@jwdinker
Copy link

I find myself coming back here every few weeks and reading this issue and every related issue in hopes this will be implemented. I like typescript, but this is the most frustrating thing I've encountered using it.

@Alecell
Copy link

Alecell commented Jun 12, 2021

I would love to see this working, sadly it don't seem to come soon.

As a work around I cast the type when the error pop's up, but this isn't a good approach and I don't recommend it very much since if the if where removed the compiler wont show any errors either because of the casting. I only use this work around on very complex logic that need a more readable code than usual.

BTW I think is good to ask, how to make a good proposal?

@forresthopkinsa
Copy link

Hell yeah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants