8000 Array.shift() should return defined for consistency with random access Β· Issue #51035 Β· microsoft/TypeScript Β· GitHub
[go: up one dir, main page]

Skip to content
Array.shift() should return defined for consistency with random accessΒ #51035
@cuuupid

Description

@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+MNIADBIA3pQwzzA1sABe9Rraegb7n11rD1k 8BA7 v1PAthkEAO6lABWfEUwC0wmAAGs+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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Awaiting More FeedbackThis means we'd like to hear from more people who would be helped by this featureSuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      0