Array.shift() should return defined for consistency with random access

[cuuupid]

Suggestion

🔍 Search Terms

in:title array shift
#13778

✅ Viability 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. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Whether an array element is defined/undefined is inconsistent. If you use direct access it's always defined even when the control flow clearly indicates it couldn't possibly be; if you use method-based access it's always possibly undefined even when the control flow clearly indicates it must be defined.

The discussion for random access in #13778 ended in keeping it defined by default and adding a pedantic compiler flag for making it all undefined. I'm proposing an extension of that to shift and other Array methods that should, for consistency, follow the same behaviour.

📃 Motivating Example

https://www.typescriptlang.org/play?#code/MYewdgzgLgBAZiEMC8MDaBGANAJiwZiwBYsBWAXQChRJYAjAQwCcUYAKAfQC4YwBXALZ0ApkwCUKAHwwOlauGgwAHqwQgAdBAAWASzhQ2EgPRHlMHRBh8wAE2FwdYYTZjCAbsLAwoWkHwDmWvCIMFoMlsIANsICnlCWdg5OLgx0IB6UjExsSsamUACeAA7CrkxMIExyNIoFqohoAJyN5DAmMHUWMImOzq4eXj5+gcFINiDClmAgsGEe3mGwAgxgdVExcRCZzGwFefC9cnrsaurRYP4+MNIADBIA3pQwzzA1sABe9Rraegb7n11rD1kv1PAthkEAO6lABWfEUwC0wmAAGs+qdzpcgrcni8smx3vtCiUyhUqgBfI5wdgAQjYGM8WLEDzapkqozOjKuyB5MBuMEhi3congkRADHiHT8LDSEtxzzeME+qDUTRarKV5gS9l6LmFg18AShsPhsERyLRLh0Zr8kRsYAA5LAiiAIBAdHRInURPKYPjCRqksJKJSgA

const foo = [1,2,3,4,5]
const bar = (_: number) => _

const x = foo.shift() // x is undefined even though foo has elements defined above
bar(x) // type error

const y = foo[99] // y is defined even though foo does not have that many elements
bar(y) // fine

if (foo.length > 0) {
 const z = foo.shift() // z is undefined even though we just checked foo.length > 0
 bar(z) // type error
}

if (!(foo.length)) { // or foo.length === 0 whatever floats your boat
 const z = foo[99] // z is defined even though we just checked it couldn't possibly be
 bar(z) // fine
}

💻 Use Case

I can understand that it always could be undefined, e.g. in this case:

const x: number[] = [1, 2, 3]

;(async () => {
 await longAsyncTask();
 while (x.length > 0) x.shift()
})()

;(async () => {
 await longAsyncTask();
 console.log(x.shift()) // could be undefined if the previous async block executes first
})()

However, this is a rarer case than x[n] which is much more commonly undefined. Given that x[n] is by default assumed defined unless the --noUncheckedIndexedAccess compiler flag is provided, .shift() should follow the same pattern.

Currently we use .shift() and a queue-based structure rather than random access, but TypeScript actually penalizes this. The only ways around this are assertions ! or adding an extra call for random access.

Workaround 1 (!):

const foo = [1,2,3,4,5]
const bar = (n: number) => n

bar(foo[99]) // somehow fine
bar(foo.shift()) // error?
bar(foo.shift()!) // workaround

Consider the following case as why this is a problematic anti-pattern:

const foo = myArray.map(myFunc) // I write this thinking the signature of foo is number[]
// however due to a bug in myFunc, signature is actually (number | undefined)[]

bar(foo.shift()!) // I type this as my workaround to behaviour described above
// now no type checking occurs above and I have a much more obscure bug in my code!

Workaround 2 is more type-safe (extra call for random access):

bar(foo[0])
foo.shift();

However, this is an anti-pattern at best.

[fatcerberus]

There’s no way to have the return type of a method depend on a compiler flag, so the stricter option was chosen.

[cuuupid]

Given the less strict option was chosen for random access until that flag was added, wouldn't it be more consistent to assume defined in this case?

[MartinJohns]

Consistency is a really bad argument when the proposed change would effectively reduce type safety.

Instead of while (x.length > 0) { x.shift() } you can just do

let nextItem: number | undefined;
while ((nextItem = x.shift()) !== undefined) nextItem