Skip to content

Logical successive type guards #37258

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

Closed
toddriley opened this issue Mar 6, 2020 · 4 comments
Closed

Logical successive type guards #37258

toddriley opened this issue Mar 6, 2020 · 4 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

@toddriley
Copy link

TypeScript Version: 3.9.0-dev.20200306

Search Terms:
successive type guards, conditional type guards, type guards

Code

function isNull(a: string | null): a is null {
  return a == null;
}

function doStuff(a: string | null, b: string | null) {
  if (a == null && b == null) {
    return;
  }

  if (a == null && b != null) {
    return;
  }

  if (a != null && b == null) {
    return;
  }

  a; // logically must be `string`, but inferred as `string | null`
  b; // logically must be `string`, but inferred as `string | null`
  a.length; // Object is possibly 'null'
}

Expected behavior:
a and b cannot logically be null, therefore it should be inferred that these are strings.

Actual behavior:
It is inferred that these variables are possibly null.

Playground Link:
https://www.typescriptlang.org/play/?ts=3.9.0-dev.20200305&ssl=1&ssc=1&pln=21&pc=2#code/GYVwdgxgLglg9mABDAzgORAG0wCgIYBciKUATjGAOaIA+iYWmAlEXsivY4gN4BQiiUgFMoIUkjYBeSZ2wBuXgF9evUJFgJEAEzgBlUcGD4iJclVqzMAGkQAjE2QrU6DbEx79kwRPkTTLiABkgXZ+Mq7MHgICwqLiCgLKnjDevv4RQSG2iACE4YzufNGCImJgCYhJAik+bHkBwaHpBVHRsWUVVYh4cogA9H2ImHCUMBB42ACeiAC2ICR2QogABqZOyza2IFDIYMBCpMJa3RyrjuYujMuetr0DQyNjE5jTcwu2S2dmlBt227v7Q5CY54U5rC6Wa4CPAAOkwQioUAAFndBgB5WwAKyE0HYiAADnAUCgYLYXogAOQRClKIA

Related Issues:
#9016

@nmain
Copy link

nmain commented Mar 9, 2020

Making the compiler reason about this sort of thing sounds really cool until you realize how slow it would be to implement. Even in this toy baby case, in order to narrow a the compiler needs to

  1. realize that it needs to narrow b at the same time
  2. pick up all previous flow control statements in the function that involve a or b
  3. Exhaustively analyze the product of union possibilities for a and b at the same time through all of those statements

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds labels Mar 12, 2020
@RyanCavanaugh
Copy link
Member

As mentioned this would be very complex, plus there are relatively few type domains that can be exhausted by one or two checks. Structuring this code in a way that's more compiler-friendly will also make it more human-friendly so I don't think it's too bad if this doesn't work.

@toddriley
Copy link
Author

toddriley commented Mar 12, 2020

@RyanCavanaugh how would you restructure this code to be more compiler/human friendly? It seems pretty straightforward. I don't disagree with closing this ticket due the complexity of the solution, but just curious about how else you'd structure this.

2 new functions for context:

function main() {
  let previousResult: string | null = null;
  setTimeout(async () => {
    const currentResult = await fetchAThingFromApi();
    doStuff(previousResult, currentResult);
    previousResult = currentResult;
  }, 30000);
}

function fetchAThingFromApi(): Promise<string | null> {
  // fetch stuff
}


// Below code is initial issue with comments added
function isNull(a: string | null): a is null {
  return a == null;
}

function doStuff(a: string | null, b: string | null) {
  if (a == null && b == null) {
    // both are null, do nothing
    return;
  }

  if (a == null && b != null) {
    // previous was null, current is not, show new thing alert
    return;
  }

  if (a != null && b == null) {
    // current is null, previous was not, show deleted thing alert
    return;
  }

  // thing changed, do other things
  a.length; // Object is possibly 'null'
}

@toddriley
Copy link
Author

For the record, I don't appreciate the issue-closing comment to be along the lines of "write better code". Feels like a cop-out.

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

3 participants