-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Description
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
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.