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

Property type update not continued inside next block after checking #48183

Closed
silverlight513 opened this issue Mar 9, 2022 · 10 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@silverlight513
Copy link

silverlight513 commented Mar 9, 2022

Bug Report

When an object contains an optional property that could either be an array or undefined. Checking that property exists as an array changes its type to only an array instead of optional. Later on, inside a .some of another array, the first array goes back to being possibly undefined. If I destructure the item before checking it's value, ts works perfectly but the destructure would be unnecessary in this case.

🔎 Search Terms

"Object is possibly 'undefined'."
"type guard not applied"
"possibly undefined inside method"

🕗 Version & Regression Information

This is the behavior in every version of the playground and could not find anything in the FAQ's that related.
I did find this issue but I'm unsure if it's related - #10530

⏯ Playground Link

Playground link with relevant code

💻 Code

interface User {
  permissions?: string[];
}

function checkPermissions(
  user: User,
  requiredPermissions?: string[],
) {
  if (!requiredPermissions?.length) return true;

  // destructure user.permissions here fixes the errors
  // const { permissions } = user; 

  if(!Array.isArray(user.permissions) || !user.permissions.length) 
    return false;

  return requiredPermissions.some(requiredPermission => {
    if (requiredPermission === 'test') {
      return user.permissions.includes(requiredPermission);
    }

    return false;
  });
}

🙁 Actual behavior

Error: Object is possibly 'undefined'.
Above error is shown for user.permissions inside the some function.

🙂 Expected behavior

The user.permissions object should be an array inside the some. It is an array in the second condition of the if statement above it.

@MartinJohns
Copy link
Contributor

MartinJohns commented Mar 9, 2022

Another duplicate of #9998.

Narrowing is not preserved in closures. You can assign the narrowed value to a const variable and use that one instead.

@silverlight513
Copy link
Author

@MartinJohns Thanks for tagging as a dupe so quick. I tried my best to make sure I wasn't raising a dupe but issues use very domain specific terminology making it hard to do this (obviously makes it easier for maintainers though).

Would it make sense having a note in the FAQ's about narrowing not preserved in closures? It appears as a fundamental part of TS that I'd expect to work. I checked in with my team who were also shocked by this issue.

@MartinJohns
Copy link
Contributor

The issue comes up quite often. Often enough for me to remember the issue number, and I'm terribly bad at remembering numbers.

Would it make sense having a note in the FAQ's about narrowing not preserved in closures?

I absolutely agree, and I would expect the TypeScript team to agree as well. It's likely that just no one did it yet. Why don't you go ahead? :-)

image
https://github.com/Microsoft/TypeScript-wiki

@jcalz
Copy link
Contributor

jcalz commented Mar 9, 2022

Yeah, the Common "Bugs" That Aren't Bugs page linked from the bug report template hasn't been touched since 2019.

I kind of wish something would keep track of the top ten most duplicated issues, and then when you file a report/suggestion you are asked to make sure that your issue isn't already covered by one of them. If I had to guess I'd say that #9998 is probably the most common one, so even something in the template that just briefly summarizes #9998 might work? Not that extra stuff in the template necessarily helps; at some point additional info starts to overwhelm/annoy people and then they are less likely to comply. Maybe even the current level is too much... it's not very often that people say "lol that template was too confusing or tedious for me so I ignored it completely yolo" but it does happen. And it's quite common for people to ignore/skip at least some of the template. 🤷‍♂️

@MartinJohns
Copy link
Contributor

so even something in the template that just briefly summarizes #9998 might work?

Like this?

image

@RyanCavanaugh
Copy link
Member

😅
image

We're not really bothered by the duplicate inflow on this one since it's so easy to triage (ironic I guess?). I've thought about making a bot responder but it doesn't feel worth the effort

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Mar 9, 2022
@MartinJohns
Copy link
Contributor

We're not really bothered by the duplicate inflow on this

But it'd still be nice to point people to this issue before they spend time on creating another issue and filling out the issue template.

@fatcerberus
Copy link

before they spend time on creating another issue and filling out the issue template.

Then again... #48148 🚎

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@MartinJohns
Copy link
Contributor

Related: #48261

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants