-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
[node] buffer: improve constructor signatures, fix CI for TS 5.9 #72056
Changes from all commits
8bef28d
22126ec
0b1e78c
045cd76
7278d5b
3805a9a
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 |
---|---|---|
@@ -1,4 +1,6 @@ | ||
declare module "buffer" { | ||
type ImplicitArrayBuffer<T extends WithImplicitCoercion<ArrayBufferLike>> = T extends | ||
{ valueOf(): infer V extends ArrayBufferLike } ? V : T; | ||
global { | ||
interface BufferConstructor { | ||
// see buffer.d.ts for implementation shared with all TypeScript versions | ||
|
@@ -24,7 +26,7 @@ declare module "buffer" { | |
* @param array The octets to store. | ||
* @deprecated since v10.0.0 - Use `Buffer.from(array)` instead. | ||
*/ | ||
new(array: Uint8Array): Buffer<ArrayBuffer>; | ||
new(array: ArrayLike<number>): Buffer<ArrayBuffer>; | ||
/** | ||
* Produces a Buffer backed by the same allocated memory as | ||
* the given {ArrayBuffer}/{SharedArrayBuffer}. | ||
|
@@ -33,20 +35,6 @@ declare module "buffer" { | |
* @deprecated since v10.0.0 - Use `Buffer.from(arrayBuffer[, byteOffset[, length]])` instead. | ||
*/ | ||
new<TArrayBuffer extends ArrayBufferLike = ArrayBuffer>(arrayBuffer: TArrayBuffer): Buffer<TArrayBuffer>; | ||
/** | ||
* Allocates a new buffer containing the given {array} of octets. | ||
* | ||
* @param array The octets to store. | ||
* @deprecated since v10.0.0 - Use `Buffer.from(array)` instead. | ||
*/ | ||
new(array: readonly any[]): Buffer<ArrayBuffer>; | ||
/** | ||
* Copies the passed {buffer} data onto a new {Buffer} instance. | ||
* | ||
* @param buffer The buffer to copy. | ||
* @deprecated since v10.0.0 - Use `Buffer.from(buffer)` instead. | ||
*/ | ||
new(buffer: Buffer): Buffer<ArrayBuffer>; | ||
/** | ||
* Allocates a new `Buffer` using an `array` of bytes in the range `0` – `255`. | ||
* Array entries outside that range will be truncated to fit into it. | ||
|
@@ -58,40 +46,120 @@ declare module "buffer" { | |
* const buf = Buffer.from([0x62, 0x75, 0x66, 0x66, 0x65, 0x72]); | ||
* ``` | ||
* | ||
* If `array` is an `Array`\-like object (that is, one with a `length` property of | ||
* If `array` is an `Array`-like object (that is, one with a `length` property of | ||
* type `number`), it is treated as if it is an array, unless it is a `Buffer` or | ||
* a `Uint8Array`. This means all other `TypedArray` variants get treated as an `Array`. To create a `Buffer` from the bytes backing a `TypedArray`, use `Buffer.copyBytesFrom()`. | ||
* a `Uint8Array`. This means all other `TypedArray` variants get treated as an | ||
* `Array`. To create a `Buffer` from the bytes backing a `TypedArray`, use | ||
* `Buffer.copyBytesFrom()`. | ||
* | ||
* A `TypeError` will be thrown if `array` is not an `Array` or another type | ||
* appropriate for `Buffer.from()` variants. | ||
* | ||
* `Buffer.from(array)` and `Buffer.from(string)` may also use the internal `Buffer` pool like `Buffer.allocUnsafe()` does. | ||
* `Buffer.from(array)` and `Buffer.from(string)` may also use the internal | ||
* `Buffer` pool like `Buffer.allocUnsafe()` does. | ||
* @since v5.10.0 | ||
*/ | ||
from<TArrayBuffer extends ArrayBufferLike>( | ||
arrayBuffer: WithImplicitCoercion<TArrayBuffer>, | ||
byteOffset?: number, | ||
length?: number, | ||
): Buffer<TArrayBuffer>; | ||
from(array: WithImplicitCoercion<ArrayLike<number>>): Buffer<ArrayBuffer>; | ||
/** | ||
* Creates a new Buffer using the passed {data} | ||
* @param data data to create a new Buffer | ||
* This creates a view of the `ArrayBuffer` without copying the underlying | ||
* memory. For example, when passed a reference to the `.buffer` property of a | ||
* `TypedArray` instance, the newly created `Buffer` will share the same | ||
* allocated memory as the `TypedArray`'s underlying `ArrayBuffer`. | ||
* | ||
* ```js | ||
* import { Buffer } from 'node:buffer'; | ||
* | ||
* const arr = new Uint16Array(2); | ||
* | ||
* arr[0] = 5000; | ||
* arr[1] = 4000; | ||
* | ||
* // Shares memory with `arr`. | ||
* const buf = Buffer.from(arr.buffer); | ||
* | ||
* console.log(buf); | ||
* // Prints: <Buffer 88 13 a0 0f> | ||
* | ||
* // Changing the original Uint16Array changes the Buffer also. | ||
* arr[1] = 6000; | ||
* | ||
* console.log(buf); | ||
* // Prints: <Buffer 88 13 70 17> | ||
* ``` | ||
* | ||
* The optional `byteOffset` and `length` arguments specify a memory range within | ||
* the `arrayBuffer` that will be shared by the `Buffer`. | ||
* | ||
* ```js | ||
* import { Buffer } from 'node:buffer'; | ||
* | ||
* const ab = new ArrayBuffer(10); | ||
* const buf = Buffer.from(ab, 0, 2); | ||
* | ||
* console.log(buf.length); | ||
* // Prints: 2 | ||
* ``` | ||
* | ||
* A `TypeError` will be thrown if `arrayBuffer` is not an `ArrayBuffer` or a | ||
* `SharedArrayBuffer` or another type appropriate for `Buffer.from()` | ||
* variants. | ||
* | ||
* It is important to remember that a backing `ArrayBuffer` can cover a range | ||
* of memory that extends beyond the bounds of a `TypedArray` view. A new | ||
* `Buffer` created using the `buffer` property of a `TypedArray` may extend | ||
* beyond the range of the `TypedArray`: | ||
* | ||
* ```js | ||
* import { Buffer } from 'node:buffer'; | ||
* | ||
* const arrA = Uint8Array.from([0x63, 0x64, 0x65, 0x66]); // 4 elements | ||
* const arrB = new Uint8Array(arrA.buffer, 1, 2); // 2 elements | ||
* console.log(arrA.buffer === arrB.buffer); // true | ||
* | ||
* const buf = Buffer.from(arrB.buffer); | ||
* console.log(buf); | ||
* // Prints: <Buffer 63 64 65 66> | ||
* ``` | ||
* @since v5.10.0 | ||
* @param arrayBuffer An `ArrayBuffer`, `SharedArrayBuffer`, for example the | ||
* `.buffer` property of a `TypedArray`. | ||
* @param byteOffset Index of first byte to expose. **Default:** `0`. | ||
* @param length Number of bytes to expose. **Default:** | ||
* `arrayBuffer.byteLength - byteOffset`. | ||
*/ | ||
from(data: Uint8Array | readonly number[]): Buffer<ArrayBuffer>; | ||
from(data: WithImplicitCoercion<Uint8Array | readonly number[] | string>): Buffer<ArrayBuffer>; | ||
from<TArrayBuffer extends WithImplicitCoercion<ArrayBufferLike>>( | ||
arrayBuffer: TArrayBuffer, | ||
byteOffset?: number, | ||
length?: number, | ||
): Buffer<ImplicitArrayBuffer<TArrayBuffer>>; | ||
Comment on lines
+130
to
+134
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 old variant of this signature suffered from an inference failure when the While |
||
/** | ||
* Creates a new Buffer containing the given JavaScript string {str}. | ||
* If provided, the {encoding} parameter identifies the character encoding. | ||
* If not provided, {encoding} defaults to 'utf8'. | ||
* Creates a new `Buffer` containing `string`. The `encoding` parameter identifies | ||
* the character encoding to be used when converting `string` into bytes. | ||
* | ||
* ```js | ||
* import { Buffer } from 'node:buffer'; | ||
* | ||
* const buf1 = Buffer.from('this is a tést'); | ||
* const buf2 = Buffer.from('7468697320697320612074c3a97374', 'hex'); | ||
* | ||
* console.log(buf1.toString()); | ||
* // Prints: this is a tést | ||
* console.log(buf2.toString()); | ||
* // Prints: this is a tést | ||
* console.log(buf1.toString('latin1')); | ||
* // Prints: this is a tést | ||
* ``` | ||
* | ||
* A `TypeError` will be thrown if `string` is not a string or another type | ||
* appropriate for `Buffer.from()` variants. | ||
* | ||
* `Buffer.from(string)` may also use the internal `Buffer` pool like | ||
* `Buffer.allocUnsafe()` does. | ||
* @since v5.10.0 | ||
* @param string A string to encode. | ||
* @param encoding The encoding of `string`. **Default:** `'utf8'`. | ||
*/ | ||
from( | ||
str: | ||
| WithImplicitCoercion<string> | ||
| { | ||
[Symbol.toPrimitive](hint: "string"): string; | ||
}, | ||
encoding?: BufferEncoding, | ||
): Buffer<ArrayBuffer>; | ||
from(string: WithImplicitCoercion<string>, encoding?: BufferEncoding): Buffer<ArrayBuffer>; | ||
/** | ||
* Creates a new Buffer using the passed {data} | ||
* @param values to create a new Buffer | ||
|
@@ -383,4 +451,10 @@ declare module "buffer" { | |
subarray(start?: number, end?: number): Buffer<TArrayBuffer>; | ||
} | ||
} | ||
/** @deprecated Use `Buffer.allocUnsafeSlow()` instead. */ | ||
var SlowBuffer: { | ||
/** @deprecated Use `Buffer.allocUnsafeSlow()` instead. */ | ||
new(size: number): Buffer<ArrayBuffer>; | ||
prototype: Buffer; | ||
}; | ||
} |
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.
Trying to understand why all of these are now annotated; what happens if they aren't?
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.
Yeah, so removing these makes the tests fail; is this not indicative of a greater problem that this will break people pretty badly?
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.
So, this is essentially the second assignability issue from the PR description.
When these variables are being initialised by calling the Buffer constructor methods, the inferred variable type without the annotation is
Buffer<ArrayBuffer>
, which is the TS 5.7+ behaviour for TypedArray constructors.Subsequently, there are attempts to write values of type
Buffer<ArrayBufferLike>
to those variables.Previously,
ArrayBufferLike
was assignable toArrayBuffer
, so this was allowable. It now isn't, so the variable has to be explicitly declared as aBuffer<ArrayBufferLike>
to accept both kinds of values.This problem isn't specific to Buffer. Any usage along the lines of
is now going to raise errors.
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.
This is unavoidable since
ArrayBuffer
andSharedArrayBuffer
types diverge under--target ES2024
.