Unexpected error: TS2322: Type 'string | undefined' is not assignable to type 'string'

[britalmeida]

Bug Report

🔎 Search Terms

TS2322 ; "undefined is not assignable to type" ; array iteration ; noUncheckedIndexedAccess

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries

⏯ Playground Link

Playground link with relevant code

💻 Code

With noUncheckedIndexedAccess=true

function testfunc(urls: string[]): void {
  for (let i = 0; i < urls.length; i++) {
    const image = new Image();
    image.src = urls[i]; // Error: 'undefined' is not assignable to type 'string'
  }
}

🙂 Expected behavior

urls is declared as an array of string, not string | undefined, therefore I expect the assignment inside the function to be safe and an error on the calling side of the function if someone tries to pass in an array with potentially undefined things in it.
I don't understand why the for loop indexing makes it unsafe? Or how I should write my code then?

I have looked at the option documentation, at stack overflow and at the internet for far too long :))
I don't understand the documentation example (I don't know about 'unknown properties').

• Could it be that noUncheckedIndexedAccess is incorrectly adding 'undefined' for this case?
• Or is it doing the correct thing? It's my confusion then, but if so, is it possible to improve the documentation so I can understand why the option is useful and how it applies to my case which has no unknown properties or fields in the type? (and how I should be writing that code snippet correctly then).

Another minimal example that I would not expect to cause an error:

const names = ['apple', 'banana'];
let s : string;
s = names[0]; // Error: 'undefined' is not assignable to type 'string'

Background context:
I'm not a JS or TS expert. I have recently started using TypeScript and porting a JS project to it. I incrementally enabled all error reporting options and strictness that I found :)
Obviously it must have been my error and messy enabling of options, but I'm pretty sure that everything worked at some point and then I noticed this error days later. It was quite time consuming to search for it and track it to the for loop and specifically to noUncheckedIndexedAccess.

[MartinJohns]

Could it be that noUncheckedIndexedAccess is incorrectly adding 'undefined' for this case?

It is adding the undefined, and that's the very purpose of that flag. It's working as intended. See the related section in the linked release notes:

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-1.html#checked-indexed-accesses---nouncheckedindexedaccess

One consequence of using noUncheckedIndexedAccess is that indexing into an array is also more strictly checked, even in a bounds-checked loop.

The flag forces you to make sure you actually got a valid value when using indexed access.

[britalmeida]

@MartinJohns with that link, I can understand the purpose of the flag better. ty!

Indeed, it seems helpful that it flags accesses such as myObject["yada yada 3"], as most likely, it won't have a field named like that.
However, the consequence of getting an error also in bounds-checked arrays with a numerical index doesn't feel helpful to me. In my project, I don't have any (intentional :) optimistic access to fields that may or not exist, but I have indexing in arrays a couple dozen times, some of them in tight loops for performance.

What would be the ways to write code that accesses arrays? This is my thinking:

1. Use a for...of. This would be the nicest option for an iterable... unless the i is really needed. In that case, there'll be some awkward fumbling with the i declared outside of the scope it's used in.
   Playground link

function testfunc(urls: string[]): void {
  let i = 0;
  for (const url of urls) {
    const image = new Image();
    image.src = url;
    // Use 'i': Load the img to a GPU array texture with slice 'i'.
    i++;
  }
}

2. Explicitly check for undefined. (doesn't work!)
   I don't know why this doesn't work or if there'd be a nicer way to type it, but ultimately it trades of a static check for a runtime one and it adds control flow exceptions to something that should really really never happen, so it doesn't feel right to add this type of code.
   Playground link

    if (urls[i] !== undefined)
      image.src = urls[i];
    else
      throw new Error('Should never happen');

3. Add the '!' image.src = urls[i]!;
   Which I'd rather not as I would like the tooling to verify if I'm doing the correct thing and not the other way around. But still better than 2. :)

Any better suggestions?

As for action, does it make sense and is it possible to refine the option so it still allows indexing with numbers or take the range checking control flow into account? (again, not a TS expert here)

I feel the option is triggering on code that is quite common and demonstrably safe. I either have to disable it, or it's leading me to change my code into making other kinds of errors (like typing a for...in instead of for...of, have variable scope leak or add !).

At a minimum, can the option documentation be updated to include the information from the release notes and guide people on how to write code that doesn't trigger an error?

[fatcerberus]

However, the consequence of getting an error also in bounds-checked arrays with a numerical index doesn't feel helpful to me. In my project, I don't have any (intentional :) optimistic access to fields that may or not exist, but I have indexing in arrays a couple dozen times, some of them in tight loops for performance.

…which is exactly why the flag is disabled by default, even under strict. 😉 The compiler isn’t quite sophisticated enough to be able to tell the difference between arr[someRandomInt] and arr[forLoopIndexer], so you have to opt in if you’re willing to trade off some extra inconvenience for stricter checks on random access.

[Jamesernator]

1. This would be the nicest option for an iterable... unless the i is really needed. In that case, there'll be some awkward fumbling with the i declared outside of the scope it's used in.

Any better suggestions?

The .entries() method already exists for this exact purpose:

for (const [i, url] of urls.entries()) {
    // Can use both the index and url here
}

[typescript-bot]

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

[a-non-a-mouse]

The compiler isn’t quite sophisticated enough to be able to tell the difference between arr[someRandomInt] and arr[forLoopIndexer]

So shouldn't this be a feature request instead of closed?