-
Notifications
You must be signed in to change notification settings - Fork 12.8k
fix(dts): add missing types to support resizable arraybuffer #54637
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,58 @@ | ||||||
interface ArrayBuffer { | ||||||
/** | ||||||
* Read-only. The maximum length that this ArrayBuffer can be resized to (in bytes). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spec proposal points out that this is equal to
Suggested change
|
||||||
*/ | ||||||
readonly maxByteLength: number; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @Josh-Cena, the spec example uses accessors, so it makes sense to do so here. (It's quite likely that old There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That seems like a breaking change we might want to take, though perhaps only if we can find a way to only enable it when |
||||||
|
||||||
/** | ||||||
* Read-only. Whether this ArrayBuffer can be resized or not. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Actually, my comments about growable apply; I'd prefer to delete this comment. |
||||||
*/ | ||||||
readonly resizable: boolean; | ||||||
|
||||||
/** | ||||||
* Resizes the ArrayBuffer to the specified size (in bytes). | ||||||
* @param newLength The new length, in bytes, to resize the ArrayBuffer to. | ||||||
*/ | ||||||
resize(newLength: number): undefined; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would call this |
||||||
} | ||||||
|
||||||
/** | ||||||
* ArrayBuffer constructor options | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would delete this. It doesn't add anything to the name of the type. |
||||||
*/ | ||||||
interface ArrayBufferOptions { | ||||||
maxByteLength?: number; | ||||||
} | ||||||
|
||||||
interface ArrayBufferConstructor { | ||||||
new(byteLength: number, options?: ArrayBufferOptions): ArrayBuffer; | ||||||
} | ||||||
|
||||||
interface SharedArrayBuffer { | ||||||
/** | ||||||
* Read-only. Whether this SharedArrayBuffer can be grow or not. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Although, |
||||||
*/ | ||||||
readonly growable: number; | ||||||
|
||||||
/** | ||||||
* Read-only. The maximum length that this SharedArrayBuffer can be grown to (in bytes). | ||||||
*/ | ||||||
readonly maxByteLength: number; | ||||||
|
||||||
/** | ||||||
* Grows the SharedArrayBuffer to the specified size (in bytes). | ||||||
* @param newLength The new length, in bytes, to resize the SharedArrayBuffer to. | ||||||
*/ | ||||||
grow(newLength: number): undefined; | ||||||
} | ||||||
|
||||||
/** | ||||||
* ArrayBuffer constructor options | ||||||
*/ | ||||||
interface SharedArrayBufferOptions { | ||||||
maxByteLength?: number; | ||||||
} | ||||||
|
||||||
interface SharedArrayBufferConstructor { | ||||||
new(); | ||||||
new(byteLength: number, options?: SharedArrayBufferOptions): SharedArrayBuffer; | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
/// <reference lib="es2023" /> | ||
/// <reference lib="esnext.intl" /> | ||
/// <reference lib="esnext.arraybuffer" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rbuckton Do you have an opinion abut we should add a separate lib entry for arraybuffer? This feels like the kind of thing that went in to earlier
es20xx.collection
entries.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resizable ArrayBuffers and growable SharedArrayBuffers are Stage 4 and will be in ES2024 when it is standardized, so a
lib.es2024.arraybuffer.d.ts
might make sense at this point, though it would still only be referenced fromesnext
since we don't have a--target es2024
yet..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or a
lib.es2024.collections.d.ts
, rather. I agree it makes more sense to try to keep the names somewhat consistent.