types: return more precise Buffer<ArrayBuffer> from toBuffer

[Andarist]

No description provided.

[Andarist]

I think this is kinda more compatible with strict package managers like pnpm. If you'd like to declare an explicit range for compatible @types/node then you'd have to declare the range within a regular peerDependencies declaration and, in here, the correct range would be something like:

^18.19.54 || ^20.16.10 || >=22.7.4

Those are the versions published on 2024-09-27 and they introduced this change: DefinitelyTyped/DefinitelyTyped#70694

[lovell]

Hi Mateusz, thanks for the PR.

If you hadn't seen, the next 0.35.0 release of sharp will also allow output via a new toUint8Array() to complement the existing toBuffer() - see #4355

toUint8Array(): Promise<{ data: Uint8Array; info: OutputInfo }>;

My understanding is that Node.js Buffer instances allocated by native code are treated as being backed by a SharedArrayBuffer and are considered non-transferable whereas Uint8Array instances are always backed by an ArrayBuffer and are transferable (e.g. between worker threads).

I think this means that that existing toBuffer() will resolve with a Buffer<SharedArrayBuffer> - is this something you're able to check?

[lovell]

@Andarist Hej Mateusz, is this something you're still interested in working on? No worries if not, I'm happy to take it on.

[lovell]

Posting a possible diff here for my future self to use as this might be a breaking change:

--- a/lib/index.d.ts
+++ b/lib/index.d.ts
@@ -667,7 +667,7 @@ declare namespace sharp {
          * @param options.resolveWithObject Resolve the Promise with an Object containing data and info properties instead of resolving only with data.
          * @returns A promise that resolves with the Buffer data.
          */
-        toBuffer(options?: { resolveWithObject: false }): Promise<Buffer>;
+        toBuffer(options?: { resolveWithObject: false }): Promise<Buffer<SharedArrayBuffer>>;
 
         /**
          * Write output to a Buffer. JPEG, PNG, WebP, AVIF, TIFF, GIF and RAW output are supported.
@@ -676,14 +676,14 @@ declare namespace sharp {
          * @param options.resolveWithObject Resolve the Promise with an Object containing data and info properties instead of resolving only with data.
          * @returns A promise that resolves with an object containing the Buffer data and an info object containing the output image format, size (bytes), width, height and channels
          */
-        toBuffer(options: { resolveWithObject: true }): Promise<{ data: Buffer; info: OutputInfo }>;
+        toBuffer(options: { resolveWithObject: true }): Promise<{ data: Buffer<SharedArrayBuffer>; info: OutputInfo }>;
 
         /**
          * Write output to a Uint8Array backed by a transferable ArrayBuffer. JPEG, PNG, WebP, AVIF, TIFF, GIF and RAW output are supported.
          * By default, the format will match the input image, except SVG input which becomes PNG output.
          * @returns A promise that resolves with an object containing the Uint8Array data and an info object containing the output image format, size (bytes), width, height and channels
          */
-        toUint8Array(): Promise<{ data: Uint8Array; info: OutputInfo }>;
+        toUint8Array(): Promise<{ data: Uint8Array<ArrayBuffer>; info: OutputInfo }>;
 
         /**
          * Set output density (DPI) in EXIF metadata.

[Andarist]

I tested this with:

const sharp = require("sharp");
const {
  MessageChannel,
  isMarkedAsUntransferable,
} = require("node:worker_threads");

(async () => {
  for (const mode of ["toBuffer", "toUint8Array"]) {
    const image = sharp({
      create: {
        width: 1,
        height: 1,
        channels: 3,
        background: "red",
      },
    }).png();

    const value =
      mode === "toBuffer"
        ? await image.toBuffer()
        : (await image.toUint8Array()).data;

    console.log(`\n${mode}`);
    console.log("sharp:", sharp.versions.sharp);
    console.log("node:", process.version);
    console.log("Buffer.isBuffer:", Buffer.isBuffer(value));
    console.log("value.constructor.name:", value.constructor.name);
    console.log(
      "value.buffer.constructor.name:",
      value.buffer.constructor.name,
    );
    console.log(
      "value.buffer instanceof ArrayBuffer:",
      value.buffer instanceof ArrayBuffer,
    );
    console.log(
      "value.buffer instanceof SharedArrayBuffer:",
      value.buffer instanceof SharedArrayBuffer,
    );
    console.log(
      "isMarkedAsUntransferable(value.buffer):",
      isMarkedAsUntransferable(value.buffer),
    );

    const { port2 } = new MessageChannel();

    try {
      port2.postMessage(value.buffer, [value.buffer]);
      console.log("transfer: ok");
    } catch (err) {
      console.log("transfer:", err.name, err.message);
    }
  }
})();

and got this output:

  toBuffer
  Buffer.isBuffer: true
  value.constructor.name: Buffer
  value.buffer.constructor.name: ArrayBuffer
  value.buffer instanceof ArrayBuffer: true
  value.buffer instanceof SharedArrayBuffer: false
  isMarkedAsUntransferable(value.buffer): true
  transfer: DataCloneError Cannot transfer object of unsupported type.

  toUint8Array
  Buffer.isBuffer: true
  value.constructor.name: Buffer
  value.buffer.constructor.name: ArrayBuffer
  value.buffer instanceof ArrayBuffer: true
  value.buffer instanceof SharedArrayBuffer: false
  isMarkedAsUntransferable(value.buffer): false
  transfer: ok

From what I can tell, none of those return SharedArrayBuffer here.

[lovell]

Brilliant, thanks for the update Mateusz.

Although ArrayBuffer instances are supposed to be transferable (MDN), it looks like a Node.js Buffer instance wraps an ArrayBuffer that is marked as non-transferable. Given the outcome of your research this behaviour feels a bit like a bug, and there's some discussion about this upstream at nodejs/node#55593 so we're not alone.

Node-API added an experimental node_api_create_external_sharedarraybuffer method from Node.js 26.1.0 but we can't use that yet, and it's unclear if we need to anyway.

In summary I think this means that Buffer<ArrayBuffer> is correct in theory but a bit broken in practice.