make splice deleteCount required in es5.d.ts

[Tom Jenkinson (tjenkinson)]

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.
https://www.ecma-international.org/ecma-262/5.1/#sec-15.4.4.12

In ES6 deleleteCount is optional, and it defaults to the length of the array minus the start index.
https://www.ecma-international.org/ecma-262/6.0/#sec-array.prototype.splice

If you are targeting ES5 but don't provide deleteCount the behaviour will be different depending on the environment your build is running in.

This PR makes the deleteCount required when targeting ES5 but should allow it to be optional for ES6.

fixes #32638

The Issue

When running in ES5

const input = [1, 2, 3];
const out = in.splice(0);
console.log(input); // [1, 2, 3]
console.log(out); // []

When running in ES6

const input = [1, 2, 3];
const out = in.splice(0);
console.log(input); // []
console.log(out); // [1, 2, 3]

[Microsoft Contribution License Agreements (msftclas)]

All CLA requirements met.

[Ryan Cavanaugh (RyanCavanaugh)]

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) user test this

[TypeScript Bot (typescript-bot)]

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the parallelized community code test suite on this PR at 368d007. You can monitor the build here. It should now contribute to this PR's status checks.

[TypeScript Bot (typescript-bot)]

Heya Ryan Cavanaugh (@RyanCavanaugh), I'm starting to run the extended test suite on this PR at 368d007. Hold tight - I'll update this comment with the log link once the build has been queued.

[TypeScript Bot (typescript-bot)]

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

[Ryan Cavanaugh (RyanCavanaugh)]

Wesley Wigham (@weswigham) looks like some noise in the user suite diff - can you check in on it?

[Wesley Wigham (weswigham)]

I opened #32610 to fix the chrome devtools changes awhile ago, the other changes (to the azure sdk which is happily updating rush and such under us, and to zone.js which seems to be broken slightly (no ides if that's our fault)) just need to be accepted.

[Tom Jenkinson (tjenkinson)]

Anything I can do to help here?

[Tom Jenkinson (tjenkinson)]

Ryan Cavanaugh (@RyanCavanaugh) Wesley Wigham (@weswigham) any update?

[Tom Jenkinson (tjenkinson)]

I brought this back up to date with master

[Nathan Shively-Sanders (sandersn)]

Thanks for your patience on this!

[Ryan Cavanaugh (RyanCavanaugh)]

This caused a break

const arr: string[] = [];
const boundSplice = arr.splice.bind(arr, 0, 0);
const items: string[] = [];
// Used to be OK, now an error
boundSplice.apply(arr, items);

[Jordan Harband (ljharb)]

Ryan Cavanaugh (@RyanCavanaugh) it's OK in ES6, it's not OK in ES5 (altho there may have been pre-ES6 browsers that considered it optional). is that a type error when targeting ES6?

[Ryan Cavanaugh (RyanCavanaugh)]

It's incorrectly a type error right now against ES6

[Tom Jenkinson (tjenkinson)]

Shame this is getting reverted. We had an outage on our TV app which this would have caught if it had existed. A few TV’s are still way back in the past.

[Tom Jenkinson (tjenkinson)]

Do you know why it caused bind and apply to break?

[Jordan Harband (ljharb)]

Ryan Cavanaugh (@RyanCavanaugh) so does that mean this PR needed to also override it in ES6 to make it optional?

[Daniel Rosenwasser (DanielRosenwasser)]

Tom Jenkinson (@tjenkinson) I believe it's because under strictBindCallApply, we try to infer the types from the signature, but overloads don't play well with that functionality. So Jordan Harband (@ljharb), if you had ES6 as the target, that extra overload caused issues. We believe that the most reasonable solution is to just say it's optional.

[Tom Jenkinson (tjenkinson)]

Ok thanks for the explanation.

It’s a shame but not the end of the world because es5-shim protects us now :). I guess we could also add the definition to our projects manually.

[Jordan Harband (ljharb)]

Daniel Rosenwasser (@DanielRosenwasser) it seems like fixing the underlying flaws with overloads is a pretty important thing to address :-/

[Daniel Rosenwasser (DanielRosenwasser)]

I agree, but I don't think we have anything concrete that we feel good about yet.