[node] buffer: improve constructor signatures, fix CI for TS 5.9 #72056

Renegade334 · 2025-02-26T07:40:54Z

microsoft/TypeScript#60150 caused breakage.

Two main themes:

Buffer is no longer assignable to ArrayBuffer.
- This was always an unintentional quirk of the shape of the ArrayBuffer prototype, and the change has revealed that several of the DT tests were mistaking Buffers for ArrayBuffers.
- This is simply a case of fixing the bad tests, although this also flagged some inconsistencies with the Buffer.from() overloads, which have been addressed.
Buffer<ArrayBufferLike> is no longer assignable to Buffer<ArrayBuffer>.
- This is more of a problem, since there is a divergence between the Buffer constructor methods in node:buffer, which return Buffer<ArrayBuffer> etc., and the remainder of the library, where the APIs ubiquitously use the default Buffer<ArrayBufferLike>.
- These were previously mutually assignable, as ArrayBufferLike was previously (unintentionally) assignable to ArrayBuffer.
- There are plenty of examples in the tests of cases like the following:
```
let buffer = Buffer.from("test"); // Buffer<ArrayBuffer>
buffer = someNodeAPI.returnsBuffer(); // Buffer<ArrayBufferLike>
```
  which now raises an error.
- This affects not only the @types/node tests, but several other DT packages with similar usage in their tests, and undoubtedly countless more in the wild.

In theory, this issue could be eliminated at source by removing the TArrayBuffer-narrowing behaviour of the Buffer constructor methods. However, this would create divergence between the constructor behaviours of Buffer and Uint8Array, and if everyone is going to have to update their affected usage of every other typed array anyway, then there isn't a compelling motive to hack this away for Buffer specifically.

Renegade334 · 2025-02-28T02:38:05Z

types/node/buffer.buffer.d.ts

+            from<TArrayBuffer extends WithImplicitCoercion<ArrayBufferLike>>(
+                arrayBuffer: TArrayBuffer,
+                byteOffset?: number,
+                length?: number,
+            ): Buffer<ImplicitArrayBuffer<TArrayBuffer>>;


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.

typescript-bot · 2025-02-28T03:01:05Z

@Renegade334 Thank you for submitting this PR!

This is a live comment that I will keep updated.

9 packages in this PR

asn1 — on npm, on unpkg
b4a — on npm, on unpkg
buffers — on npm, on unpkg
ecurve — on npm, on unpkg
hexo-util — on npm, on unpkg
node — on npm, on unpkg
typedarray-to-buffer — on npm, on unpkg
uuid — on npm, on unpkg
varstruct — on npm, on unpkg

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ A DT maintainer needs to approve changes that affect more than one package

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 72056,
  "author": "Renegade334",
  "headCommitOid": "3805a9a3eab0ada03a3653741f5a7031afc02918",
  "mergeBaseOid": "a57d33ad37d67f39c651185709c364a3f1d2af9d",
  "lastPushDate": "2025-02-26T07:40:54.000Z",
  "lastActivityDate": "2025-02-28T21:43:16.000Z",
  "mergeOfferDate": "2025-02-28T21:39:32.000Z",
  "mergeRequestDate": "2025-02-28T21:43:16.000Z",
  "mergeRequestUser": "Renegade334",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "asn1",
      "kind": "edit",
      "files": [
        {
          "path": "types/asn1/asn1-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "jgeurts",
        "shortstuffsushi"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "b4a",
      "kind": "edit",
      "files": [
        {
          "path": "types/b4a/b4a-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/b4a/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "BendingBender"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "buffers",
      "kind": "edit",
      "files": [
        {
          "path": "types/buffers/buffers-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "rhencke"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "ecurve",
      "kind": "edit",
      "files": [
        {
          "path": "types/ecurve/ecurve-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "mhegazy"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "hexo-util",
      "kind": "edit",
      "files": [
        {
          "path": "types/hexo-util/hexo-util-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "segayuu",
        "KentarouTakeda"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/buffer.buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/buffer.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/http.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/https.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/net.ts",
          "kind": "test"
        },
        {
          "path": "types/node/ts5.6/buffer.buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/buffer.buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/test/buffer.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/test/http.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/test/https.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/test/net.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/ts5.6/buffer.buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v20/buffer.buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v20/buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v20/test/buffer.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/test/http.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/test/https.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/test/net.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/ts5.6/buffer.buffer.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "Microsoft",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "galkin",
        "parambirs",
        "eps1lon",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "peterblazejewicz",
        "addaleax",
        "victorperin",
        "NodeJS",
        "LinusU",
        "wafuwafu13",
        "mcollina",
        "Semigradsky"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "typedarray-to-buffer",
      "kind": "edit",
      "files": [
        {
          "path": "types/typedarray-to-buffer/typedarray-to-buffer-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "leonsilicon"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "uuid",
      "kind": "edit",
      "files": [
        {
          "path": "types/uuid/v2/uuid-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "iamolivinius",
        "felipeochoa",
        "cjbarth",
        "LinusU",
        "ctavan"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "varstruct",
      "kind": "edit",
      "files": [
        {
          "path": "types/varstruct/varstruct-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "BendingBender"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "rbuckton",
      "date": "2025-02-28T21:38:53.000Z",
      "isMaintainer": true
    },
    {
      "type": "stale",
      "reviewer": "jgeurts",
      "date": "2025-02-28T03:04:04.000Z",
      "abbrOid": "591bf94"
    }
  ],
  "mainBotCommentID": 2689601005,
  "ciResult": "pass"
}

typescript-bot · 2025-02-28T03:01:07Z

🔔 @jgeurts @shortstuffsushi @BendingBender @rhencke @mhegazy @segayuu @KentarouTakeda @microsoft @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @galkin @parambirs @eps1lon @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @yoursunny @qwelias @ExE-Boss @peterblazejewicz @addaleax @victorperin @nodejs @LinusU @wafuwafu13 @mcollina @Semigradsky @leonsilicon @iamolivinius @felipeochoa @cjbarth @ctavan — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

- `Buffer.from(ArrayBuffer)` should return `Buffer<ArrayBuffer>` (previously inferred as `Buffer<ArrayBufferLike>`) - `Buffer.from()` accepts other `TypedArray`s and array-likes

Renegade334 · 2025-02-28T07:27:08Z

types/b4a/index.d.ts

It looks like b4a never supported implicit coercion – these types were just copied across from @types/node when the package was created.

typescript-bot · 2025-02-28T07:41:35Z

@jgeurts Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

jakebailey · 2025-02-28T20:38:11Z

types/buffers/buffers-tests.ts

@@ -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]);


Trying to understand why all of these are now annotated; what happens if they aren't?

Yeah, so removing these makes the tests fail; is this not indicative of a greater problem that this will break people pretty badly?

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.

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.

jakebailey · 2025-02-28T20:43:48Z

I'm confused how this didn't fail CI; when I run locally:

$ pnpm install --filter . --filter '{./types/typedarray-to-buffer}...'
$ pnpm test typedarray-to-buffer

> [email protected] test /home/jabaile/work/DefinitelyTyped
> node --enable-source-maps node_modules/@definitelytyped/dtslint/ types "typedarray-to-buffer"

[email protected]
Error:
/home/jabaile/work/DefinitelyTyped/types/typedarray-to-buffer/typedarray-to-buffer-tests.ts
  4:1  error  [email protected] compile error:
Type 'Buffer<ArrayBufferLike>' is not assignable to type 'Uint8Array<ArrayBuffer>'.
  Types of property 'buffer' are incompatible.
    Type 'ArrayBufferLike' is not assignable to type 'ArrayBuffer'.
      Type 'SharedArrayBuffer' is not assignable to type 'ArrayBuffer'.
        Types of property '[Symbol.toStringTag]' are incompatible.
          Type '"SharedArrayBuffer"' is not assignable to type '"ArrayBuffer"'  @definitelytyped/expect

✖ 1 problem (1 error, 0 warnings)

    at combineErrorsAndWarnings (/home/jabaile/work/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/src/index.ts:259:26)
    at runTests (/home/jabaile/work/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/src/index.ts:250:18)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async main (/home/jabaile/work/DefinitelyTyped/node_modules/.pnpm/@[email protected][email protected]/node_modules/@definitelytyped/dtslint/src/index.ts:103:22)
``

jakebailey · 2025-02-28T20:46:26Z

I'm dumb, I didn't even check out this PR. oops

rbuckton · 2025-02-28T21:19:20Z

Buffer is no longer assignable to ArrayBuffer.

This was always an unintentional quirk of the shape of the ArrayBuffer prototype, and the change has revealed that several of the DT tests were mistaking Buffers for ArrayBuffers.

This is simply a case of fixing the bad tests, although this also flagged some inconsistencies with the Buffer.from() overloads, which have been addressed.

🎉

Buffer<ArrayBufferLike> is no longer assignable to Buffer<ArrayBuffer>.

This is more of a problem, since there is a divergence between the Buffer constructor methods in node:buffer, which return Buffer<ArrayBuffer> etc., and the remainder of the library, where the APIs ubiquitously use the default Buffer<ArrayBufferLike>.

These were previously mutually assignable, as ArrayBufferLike was previously (unintentionally) assignable to ArrayBuffer.

This break actually occurred earlier than 60150, in TS 5.7, but would only be noticeable when using --target ES2024 due to the change to the ArrayBuffer and SharedArrayBuffer types to support resizable and growable array buffers. It's showing up now because the Symbol.toStringTag makes that difference evident in earlier targets. In general, I think it's good to address now before users start to adopt --target ES2024 regularly.

In theory, this issue could be eliminated at source by removing the TArrayBuffer-narrowing behaviour of the Buffer constructor methods. However, this would create divergence between the constructor behaviours of Buffer and Uint8Array, and if everyone is going to have to update their affected usage of every other typed array anyway, then there isn't a compelling motive to hack this away for Buffer specifically.

Unfortunately, the TArrayBuffer type parameter was added to avoid other breaks as a result of https://github.com/tc39/proposal-resizablearraybuffer. Removing TArrayBuffer from Buffer would mean either inheriting from Uint8Array<ArrayBufferLike>, which would run afoul of the same other breaks, or inherit from Uint8Array<ArrayBuffer>, which would likely break some users using Buffer.from() with a SharedArrayBuffer (https://nodejs.org/docs/latest/api/buffer.html#static-method-bufferfromarraybuffer-byteoffset-length)

typescript-bot · 2025-02-28T21:39:32Z

@Renegade334: Everything looks good here. I am ready to merge this PR (at 3805a9a) on your behalf whenever you think it's ready.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! ❤️

Renegade334 · 2025-02-28T21:43:16Z

Ready to merge.

Renegade334 force-pushed the node-buffer-assignability branch 8 times, most recently from 66f756c to 2bd4e37 Compare February 28, 2025 01:49

Renegade334 changed the title ~~[WIP] [node] buffer: fix CI for TS 5.9~~ [node] buffer: improve constructor signatures, fix CI for TS 5.9 Feb 28, 2025

Renegade334 force-pushed the node-buffer-assignability branch from 2bd4e37 to 591bf94 Compare February 28, 2025 02:35

Renegade334 commented Feb 28, 2025

View reviewed changes

Renegade334 marked this pull request as ready for review February 28, 2025 03:00

Renegade334 requested review from eps1lon and peterblazejewicz as code owners February 28, 2025 03:00

typescript-bot added Critical package Edits multiple packages labels Feb 28, 2025

jgeurts approved these changes Feb 28, 2025

View reviewed changes

Renegade334 added 6 commits February 28, 2025 07:23

improve Buffer.from() signatures

8bef28d

- `Buffer.from(ArrayBuffer)` should return `Buffer<ArrayBuffer>` (previously inferred as `Buffer<ArrayBufferLike>`) - `Buffer.from()` accepts other `TypedArray`s and array-likes

make SlowBuffer typesVersions-compliant

22126ec

improve Buffer.from() tests

0b1e78c

fix @types/node test suite assignability errors

045cd76

fix test errors in dependent packages

7278d5b

remove implicit coercion from b4a.from()

3805a9a

Renegade334 force-pushed the node-buffer-assignability branch from 591bf94 to 3805a9a Compare February 28, 2025 07:24

Renegade334 commented Feb 28, 2025

View reviewed changes

jakebailey reviewed Feb 28, 2025

View reviewed changes

jakebailey closed this Feb 28, 2025

jakebailey reopened this Feb 28, 2025

rbuckton approved these changes Feb 28, 2025

View reviewed changes

typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Feb 28, 2025

typescript-bot merged commit f0e6aeb into DefinitelyTyped:master Feb 28, 2025
30 checks passed

Renegade334 deleted the node-buffer-assignability branch February 28, 2025 22:04

sapphi-red mentioned this pull request Mar 3, 2025

[node] buffer: allow ArrayBuffer | string to be passed to Buffer.from #72097

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[node] buffer: improve constructor signatures, fix CI for TS 5.9 #72056

[node] buffer: improve constructor signatures, fix CI for TS 5.9 #72056

Renegade334 commented Feb 26, 2025 •

edited

Loading

Renegade334 Feb 28, 2025

typescript-bot commented Feb 28, 2025 •

edited

Loading

typescript-bot commented Feb 28, 2025

Renegade334 Feb 28, 2025

typescript-bot commented Feb 28, 2025

jakebailey Feb 28, 2025

jakebailey Feb 28, 2025

Renegade334 Feb 28, 2025

rbuckton Feb 28, 2025

jakebailey commented Feb 28, 2025

jakebailey commented Feb 28, 2025

rbuckton commented Feb 28, 2025

typescript-bot commented Feb 28, 2025

Renegade334 commented Feb 28, 2025

[node] buffer: improve constructor signatures, fix CI for TS 5.9 #72056

[node] buffer: improve constructor signatures, fix CI for TS 5.9 #72056

Conversation

Renegade334 commented Feb 26, 2025 • edited Loading

Renegade334 Feb 28, 2025

Choose a reason for hiding this comment

typescript-bot commented Feb 28, 2025 • edited Loading

9 packages in this PR

Code Reviews

Status

typescript-bot commented Feb 28, 2025

Renegade334 Feb 28, 2025

Choose a reason for hiding this comment

typescript-bot commented Feb 28, 2025

jakebailey Feb 28, 2025

Choose a reason for hiding this comment

jakebailey Feb 28, 2025

Choose a reason for hiding this comment

Renegade334 Feb 28, 2025

Choose a reason for hiding this comment

rbuckton Feb 28, 2025

Choose a reason for hiding this comment

jakebailey commented Feb 28, 2025

jakebailey commented Feb 28, 2025

rbuckton commented Feb 28, 2025

typescript-bot commented Feb 28, 2025

Renegade334 commented Feb 28, 2025

Renegade334 commented Feb 26, 2025 •

edited

Loading

typescript-bot commented Feb 28, 2025 •

edited

Loading