Skip to content
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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion types/asn1/asn1-tests.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Ber, BerReader, BerWriter } from "asn1";

let buf = Buffer.alloc(0);
let buf: Buffer = Buffer.alloc(0);
let bufferOrNull: Buffer | null = Buffer.alloc(0);
const bool = false;
let boolOrNull: boolean | null = false;
Expand Down
2 changes: 1 addition & 1 deletion types/b4a/b4a-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ u8a = b4a.fill(buf, 1, 2, 3, "utf8");
u8a = b4a.from(sb);
u8a = b4a.from(buf);
u8a = b4a.from("this is a tést");
u8a = b4a.from(buf, 1, 2);
u8a = b4a.from(buf.buffer, 1, 2);
u8a = b4a.from([0x62, 0x75, 0x66, 0x66, 0x65, 0x72] as readonly number[]);
// @ts-expect-error
b4a.from({});
Expand Down
20 changes: 6 additions & 14 deletions types/b4a/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,30 +65,22 @@ export function fill(
end?: number,
encoding?: BufferEncoding,
): Buffer | Uint8Array;
/**
* See https://nodejs.org/api/buffer.html#static-method-bufferfromarray
*/
export function from(array: Uint8Array | readonly number[]): Buffer | Uint8Array;
/**
* See https://nodejs.org/api/buffer.html#static-method-bufferfromarraybuffer-byteoffset-length
*/
export function from(
arrayBuffer: WithImplicitCoercion<ArrayBuffer | SharedArrayBuffer>,
arrayBuffer: ArrayBuffer | SharedArrayBuffer,
byteOffset?: number,
length?: number,
): Buffer | Uint8Array;
/**
* See https://nodejs.org/api/buffer.html#static-method-bufferfrombuffer
*/
export function from(data: Uint8Array | readonly number[]): Buffer | Uint8Array;
/**
* See https://nodejs.org/api/buffer.html#static-method-bufferfromarray
*/
// tslint:disable-next-line unified-signatures
export function from(data: WithImplicitCoercion<Uint8Array | readonly number[] | string>): Buffer | Uint8Array;
/**
* See https://nodejs.org/api/buffer.html#static-method-bufferfromstring-encoding
*/
export function from(
str: WithImplicitCoercion<string> | { [Symbol.toPrimitive](hint: "string"): string },
encoding?: BufferEncoding,
): Buffer | Uint8Array;
export function from(string: string, encoding?: BufferEncoding): Buffer | Uint8Array;
/**
* See https://nodejs.org/api/buffer.html#bufincludesvalue-byteoffset-encoding
* @param buffer
Expand Down
2 changes: 1 addition & 1 deletion types/buffers/buffers-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Buffers = require("buffers");
var any: any;
var num: number;
var str: string;
var buf = new Buffer([1, 2, 3]);
var buf: Buffer = new Buffer([1, 2, 3]);
Copy link
Member

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?

Copy link
Member

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?

Copy link
Contributor Author

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 to ArrayBuffer, so this was allowable. It now isn't, so the variable has to be explicitly declared as a Buffer<ArrayBufferLike> to accept both kinds of values.

This problem isn't specific to Buffer. Any usage along the lines of

declare namespace someAPI {
  function returnsTypedArray(): Int8Array;
}

let typedArray = new Int8Array(1024);
//  ^ Int8Array<ArrayBuffer>
typedArray = someAPI.returnsTypedArray();
//           ^ Int8Array<ArrayBufferLike>

is now going to raise errors.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This problem isn't specific to Buffer. Any usage along the lines of [...] is now going to raise errors.

This is unavoidable since ArrayBuffer and SharedArrayBuffer types diverge under --target ES2024.

var bufs: Buffers = new Buffers(); // with new, as type

bufs = Buffers(); // no new
Expand Down
2 changes: 1 addition & 1 deletion types/ecurve/ecurve-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const curvePt = ecparams.G.multiply(BigInteger.fromBuffer(privateKey));
const x = curvePt.affineX.toBuffer(32);
const y = curvePt.affineY.toBuffer(32);

let publicKey = Buffer.concat([new Buffer([0x04]), x, y]);
let publicKey: Buffer = Buffer.concat([new Buffer([0x04]), x, y]);
console.log(publicKey.toString("hex"));
// => 04d0988bfa799f7d7ef9ab3de97ef481cd0f75d2367ad456607647edde665d6f6fbdd594388756a7beaf73b4822bc22d36e9bda7db82df2b8b623673eefc0b7495

Expand Down
2 changes: 1 addition & 1 deletion types/hexo-util/hexo-util-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { createHash } from "crypto";
import { join } from "path";
import { Readable } from "stream";

let buffer = Buffer.alloc(0);
let buffer: Buffer = Buffer.alloc(0);
let directory: { [x: string]: any } = {};
let string = "";

Expand Down
150 changes: 112 additions & 38 deletions types/node/buffer.buffer.d.ts
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
Expand All @@ -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}.
Expand All @@ -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.
Expand All @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 arrayBuffer parameter was an actual array buffer, rather than an "implicit coercion" object.

While from({ valueOf: () => new ArrayBuffer(n) }) would return a Buffer<ArrayBuffer> as intended, the more common from(new ArrayBuffer(n)) would fail to infer TArrayBuffer and would return a Buffer<ArrayBufferLike>. These were previously effectively identical types, but are now not in TS 5.9.

/**
* 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
Expand Down Expand Up @@ -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;
};
}
15 changes: 4 additions & 11 deletions types/node/buffer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@ declare module "buffer" {
* @param toEnc To target encoding.
*/
export function transcode(source: Uint8Array, fromEnc: TranscodeEncoding, toEnc: TranscodeEncoding): Buffer;
export const SlowBuffer: {
/** @deprecated since v6.0.0, use `Buffer.allocUnsafeSlow()` */
new(size: number): Buffer;
prototype: Buffer;
};
/**
* Resolves a `'blob:nodedata:...'` an associated `Blob` object registered using
* a prior call to `URL.createObjectURL()`.
Expand Down Expand Up @@ -237,7 +232,10 @@ declare module "buffer" {
}
export import atob = globalThis.atob;
export import btoa = globalThis.btoa;

export type WithImplicitCoercion<T> =
| T
| { valueOf(): T }
| (T extends string ? { [Symbol.toPrimitive](hint: "string"): T } : never);
global {
namespace NodeJS {
export { BufferEncoding };
Expand All @@ -256,11 +254,6 @@ declare module "buffer" {
| "latin1"
| "binary"
| "hex";
type WithImplicitCoercion<T> =
| T
| {
valueOf(): T;
};
/**
* Raw data is stored in instances of the Buffer class.
* A Buffer is similar to an array of integers but corresponds to a raw memory allocation outside the V8 heap. A Buffer cannot be resized.
Expand Down
Loading