Skip to content

strictNullCheck Errors caused by accessing array attributes like .pop in a while loop #23403

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
TheLarkInn opened this issue Apr 13, 2018 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@TheLarkInn
Copy link
Member

TheLarkInn commented Apr 13, 2018

Edit by @DanielRosenwasser: This issue is about JavaScript files.


TypeScript Version: 2.9.0-dev.20180412

Search Terms: .pop, strictNullChecks while(), loop, array, possibly, undefined

Code

const queue = [
    () => { console.log("hello") }
];

while (queue.length) {
    queue.pop()();
}

Expected behavior:
That because the while loop is checking the length of an iterable is first > 0, that array methods are callable and valid with out errors.
Actual behavior:
Error message: Cannot invoke an object which is possibly 'undefined'.
Playground Link:
https://www.typescriptlang.org/play/#src=const%20queue%20%3D%20%5B%0D%0A%20%20%20%20()%20%3D%3E%20%7B%20console.log(%22hello%22)%20%7D%0D%0A%5D%3B%0D%0A%0D%0Awhile%20(queue.length)%20%7B%0D%0A%20%20%20%20queue.pop()()%3B%0D%0A%7D

Relevant Information
This is a pretty common practice in webpack to use a queue to perform breadth first search traversals of our dependency graph in the fastest means possible. So it would be nice to not have to @ts-ignore this line

@jack-williams
Copy link
Collaborator

jack-williams commented Apr 13, 2018

Seems like a good use-case for a definite assignment assertion.

const queue = [
    () => { console.log("hello") }
];

while (queue.length) {
    queue.pop()!(); // <--- !
}

I think applying narrowing to this case might be tricky because of possible side effects.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 13, 2018

Sean didn't mention it, but this was in a JavaScript file, and we don't have non-null assertion syntax there.

@DanielRosenwasser
Copy link
Member

Just opened up #23405.

@jack-williams
Copy link
Collaborator

Ah sorry. Then my suggestion is redundant.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 16, 2018

Seems like there are two ways to go here, either change the definition of pop to be "optimistic" and not return undefined, that will be inline with array indexing not containing undefined, or go with the more explicit option in #23405. The first approach seems rather targeted, and i am sure this will come up again. so #23405 seems like a better investment here.

The underlying issue is discussed in #9998.

Closing in favor of #23405.

@mhegazy mhegazy added the Duplicate An existing issue was already created label Apr 16, 2018
@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants