-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
deleteCount
param to splice()
is not optional in es5 types
#32638
Comments
ES5 Array.prototype.splice (start, deleteCount, ...
|
Unless I'm confused about something, the function's behavior is well-predictable when given one argument. The spec is not authoritative as to argument optionality since it's not a well-defined concept in JS and For example |
Hmm interesting. The reason I opened this is because we had a The issue is the default value essentially changed between es5 and es6. Given the the default becomes something else in es6, and the param is not marked as optional in es5, do you think it makes sense for there to be an exception in this case? It's not safe to omit |
The wording used in the spec is interesting:
Based on this, and the fact that the behavior changed between ES5 and ES6 (despite "don't break the web" being the unofficial ES motto), the default value of 0 therefore seems to just be an accident of the integer coercion rules and not directly intended in the design. It definitely seems like the older spec wants to treat the second parameter as required, at any rate. That said, even with the reasoning above, the fact that in practice there is a "default" and it isn't consistent between ES5 and ES6 is quite frustrating. |
Yeah, after reading more, it seems like making the 2nd arg required for ES5 and optional for ES6 would be better in this particular case. I don't think I've ever called splice without a second arg, though, because it's not very useful. The ES6 spec explicitly mentions behaviour for 1 Arg being passed,
But it seems like the ES5 spec is hinting that if called with zero or one Arg, then the behavior is more or less undefined, even if the algorithm outlined defines a behavior |
OK, that sounds pretty annoying. Accepting PRs |
Thanks for the feedback on this BTW. When/if someone submits a PR it'd be nice if this issue had a clear and concrete description of what happens in ES5 vs ES6 with the various argument counts. |
Awesome. I will put together a PR. I know I have used |
I think this is what happens. Maybe someone can correct me if I made a mistake.
From this, the
|
In ES5 `deleteCount` is not an optional argument. If it is not provided it defaults to 0 as a side effect of `undefined` being converted to an integer. In ES6 `deleleteCount` is optional, and it defaults to the length of the array minus the start index. If you are targeting ES5 but don't provide `deleteCount` the behaviour will be different depending on the environment your build is running in. fixes microsoft#32638
In ES5 `deleteCount` is not an optional argument. If it is not provided it defaults to 0 as a side effect of `undefined` being converted to an integer. In ES6 `deleleteCount` is optional, and it defaults to the length of the array minus the start index. If you are targeting ES5 but don't provide `deleteCount` the behaviour will be different depending on the environment your build is running in. fixes microsoft#32638
PR: #32643 |
* make splice `deleteCount` required in es5.d.ts In ES5 `deleteCount` is not an optional argument. If it is not provided it defaults to 0 as a side effect of `undefined` being converted to an integer. In ES6 `deleleteCount` is optional, and it defaults to the length of the array minus the start index. If you are targeting ES5 but don't provide `deleteCount` the behaviour will be different depending on the environment your build is running in. fixes #32638 * update baselines
Hi! I've run into a related issue. I'm not sure if this is really a bug with TypeScript, the interpreters, or the spec itself. However, the typings are at the very least misleading. ES2022 deleteCount was clarified to be clearly optional, but if you pass While deleteCount may or may not be optional (your guess is as good as mine), it cannot be ⏯ Playground LinkI added several cases to test behaviour at the following: Playground link with relevant code 💻 Codeclass FailingWrapper{
arr = [1,2,3,4];
splice(start:number, deleteCount?: number){
// When deleteCount is 'undefined' method fails silently
return this.arr.splice(start, deleteCount);
}
}
const wrapper = new FailingWrapper();
const removed = wrapper.splice(2) // deleteCount is undefined
console.log(
`Case 4: Arr length is now ${wrapper.arr.length}, and removed length is ${removed.length}`
); 🙁 Actual behavior
splice() failed silently here because undefined was passed as deleteCount. 🙂 Expected behavior
According to the typings splice() should have worked. AddendumIt looks like the PR was reverted, but this edge case doesn't seem to be accounted for. @DanielRosenwasser and @RyanCavanaugh can you please take another look at this? It's easy enough to code around, but this is unexpected / undocumented behaviour, and was a serious pain to track down. |
That's annoying as heck. We'll have to make it two overloads instead of having an optional parameter. |
@RyanCavanaugh how would that work? I just tried adding a |
To be clear, as noted above, when deleteCount is assigned undefined it defaults to 0. It's just weird because: arr.splice(number) // deleteCount is length - start
arr.splice(number, undefined) // deleteCount is 0 Behave totally different, when you'd expect they would be the same. I don't know if this should be handled by the transpiler, or simply just documented and warn users of weird behaviour of splice. |
Even if it remains optional in ES5, could at least the documentation be clarified? The current documentation |
TypeScript Version: 3.5.1
Search Terms: splice deleteCount
Code
Set the target to ES5 then
Expected behavior:
[1, 2, 3].splice(0);
not allowedActual behavior:
[1, 2, 3].splice(0);
allowedPlayground Link: this
deleteCount
only become optional in ES6ES5: https://www.ecma-international.org/ecma-262/5.1/#sec-15.4.4.12
ES6: https://www.ecma-international.org/ecma-262/6.0/#sec-array.prototype.splice
The text was updated successfully, but these errors were encountered: