Skip to content

fix(ArrayBuffer): the ArrayBuffer constructor does not require an argument #53096

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

Closed
wants to merge 1 commit into from

Conversation

ljharb
Copy link
Contributor

@ljharb ljharb commented Mar 5, 2023

new ArrayBuffer() works fine and is the same as new ArrayBuffer(0).

@typescript-bot
Copy link
Collaborator

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src/lib' or possibly our lib generator. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src/lib' or https://github.com/microsoft/TypeScript-DOM-lib-generator

@typescript-bot typescript-bot added the lib update PR modifies files in the `lib` folder label Mar 5, 2023
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Mar 5, 2023
@jakebailey
Copy link
Member

See also: #50657 (comment)

Code like this only works because undefined is coerced to 0 (and the first parameter being optional is not documented on MDN), so I'm not sure that this is desirable, in a way similar to how parseInt technically accepts anything but coerces to string before using it, but we type it as only accepting string anyway.

@ljharb
Copy link
Contributor Author

ljharb commented Mar 5, 2023

Whether it’s desirable or not doesn’t seem relevant - it’s how the language works. Arguments that can be omitted are optional, by definition.

Note that I’m not trying to make it take an explicit undefined - i agree that that would be handling coercion behaviors - but an absent argument is different.

@fatcerberus
Copy link

fatcerberus commented Mar 6, 2023

Note that I’m not trying to make it take an explicit undefined - i agree that that would be handling coercion behaviors - but an absent argument is different.

This feels like a slippery slope. If I write the JS function

function printNum(x) {
    console.log(+x);
}

or more to the point

function makeBuf(sz) {
    return new ArrayBuffer(sz);
}

Let's assume the JS was written exactly like that and I'm now writing a corresponding .d.ts. Should the TS type for these functions be (x: number) => void or (x?: number) => void? The latter works but only because undefined coerces to 0. This is why what the spec says happens (in a language with no UB, even) can't be directly used as justification for typechecking behavior. What matters is intent: Is it 1) intended that ArrayBuffer will be called without arguments, and 2) if someone deliberately does so, is it likely to do what they intended? (parseInt(nonString), e.g., fails this second point)

@RyanCavanaugh
Copy link
Member

I don't see the value of this change. A zero-length ArrayBuffer is useless; if you omitted this argument you've forgotten to do something important. It'd be like calling String#substring or Math.sin with no arguments -- something happens but that's not plausibly-correct code.

Whether it’s desirable or not doesn’t seem relevant - it’s how the language works

The raison d'être of TypeScript is to identify undesirable code, regardless of whether or not the runtime behavior involves throwing an exception. "It happens to work at runtime" isn't the bar here.

Even if we're deferring to the spec, the spec says this isn't optional either. The signature is

25.1.3.1 ArrayBuffer ( length )

vs an optional parameter like

21.1.3.6 Number.prototype.toString ( [ radix ] )

@ljharb
Copy link
Contributor Author

ljharb commented Mar 6, 2023

Fair enough, the spec saying it's required is pretty solid reasoning.

Although if it was useless to have a zero byteLength then I assume you wouldn't be able to create one, so I don't buy that argument.

@ljharb ljharb deleted the arraybuffer-optional branch March 6, 2023 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug lib update PR modifies files in the `lib` folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants