TypeScript type adjustments for Socket and Worker

[cmidgley]

Three typescript .d.ts file changes:

• socket.d.ts: define a type for Socket
  * xs.d.ts: Make concat on ArrayBuffer optional as it isn't always available on the various ArrayBufferLike types.
• worker.d.ts: Strengthed the worker types, including adding the creation properties.

(Edited to remove the xs.d.ts changes, per discussion below)

[phoddie]

I think this means that the concat method may not be present on some ArrayBuffer instances. I understand that it is an XS extension, but it is always present when running under XS. My concern is that this declaration will require TypeScript users of XS to write buffer?.concat(...). Maybe not the end of the world, but I'd like to the goal of this change better.

[cmidgley]

I don't recall all the reasons why I needed this, but here is one.

function doSomething(buffer: ArrayBuffer): void { }

const myArray = new Uint8Array(10);
doSomething(myArray);

This works perfectly on Node typings/execution, but fails without this change on XS:

error TS2345: Argument of type 'Uint8Array' is not assignable to parameter of type 'ArrayBuffer'.
      Property 'concat' is missing in type 'Uint8Array' but required in type 'ArrayBuffer'.

I've not done a comparative study between Node and XS, so this may be a naive solution. Perhaps the correct solution is adjusting Uint8Array (and associated) types to have concat?

[phoddie]

A Uint8Array is not a subclass of ArrayBuffer. The call to doSomething should be doSomething(myArray.buffer).

[cmidgley]

I understand that a view is logically not a buffer, but it does work in Node. Worse, third party modules that I depend upon consume it that way. You can even ask ArrayBuffer if it is a view or not with ArrayBuffer.isView(xxx). For example, in Node:

function doSomething(buffer: ArrayBuffer): void { 
	// it fully works as an ArrayBuffer...
	console.log(buffer.byteLength);
	console.log(new Uint8Array(buffer)[0]);
	console.log(new Uint8Array(buffer.slice(0, 3))[0]);
	console.log(ArrayBuffer.isView(buffer));
	// console.log(buffer[0]); // this is a type error as expected
}

const myArray = new Uint8Array(10);
myArray.fill(65);
doSomething(myArray);

Will output:

10
65
65
true

Also, ArrayBufferLike is challenging to work with in XS as the following (basically your example) works in Node:

function doSomething(buffer: ArrayBuffer) {}
const myArray = new Uint8Array(10);
myArray.fill(65);
doSomething(myArray.buffer);

But on XS it fails to compile:

error TS2345: Argument of type 'ArrayBufferLike' is not assignable to parameter of type 'ArrayBuffer'.
  Property 'concat' is missing in type 'SharedArrayBuffer' but required in type 'ArrayBuffer'.

29 doSomething(myArray.buffer);
               ~~~~~~~~~~~~~~

  ../../../../../../../../../../../moddable/xs/includes/xs.d.ts:65:2
    65  concat(...buffers: ArrayBufferLike[]): ArrayBuffer;
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    'concat' is declared here.

This ArrayBufferLike problem doesn't get solved by making concat optional, as the error then morphs to:

error TS2345: Argument of type 'ArrayBufferLike' is not assignable to parameter of type 'ArrayBuffer'.
  Type 'HostBuffer' is missing the following properties from type 'ArrayBuffer': slice, [Symbol.toStringTag]

29 doSomething(myArray.buffer);
               ~~~~~~~~~~~~~~

I do agree in hindsight that concat? is a hack and wrong (and doesn't address the whole problem). But I'm unclear how to get compatibility with Node modules without doing something like adding concat, slice, etc. to HostBuffer and adjust ArrayBufferLike so it works across the types.

As always I'm glad to help with a PR, but I need guidance on this one. Thanks!

[phoddie]

I understand that a view is logically not a buffer, but it does work in Node.

The example above generates the same result running under XS as Node.

You can even ask ArrayBuffer if it is a view or not with ArrayBuffer.isView(xxx).

You can ask ArrayBuffer.isView about anything. It returns true for TypedArrays and DataView.

ArrayBuffer.isView("123");
ArrayBuffer.isView({});
ArrayBuffer.isView(1);
ArrayBuffer.isView();

Also, ArrayBufferLike is challenging to work with in XS as the following (basically your example) works in Node

That generates the same result in XS and Node too: both agree that an ArrayBuffer instance isn't a view.

ArrayBuffer and Uint8Array are not generally interchangeable. It is a mistake to pass a Uint8Array to a call that expects an ArrayBuffer though it might work in some cases - as you found. For example. new Uint8Array(buffer) works for Uint8Array because the TypedArray constructor will iterate the Uint8Array to get the byte values (it will do the same for any other TypedArray). But that's making a copy into a new buffer, which is very different from what happens when passing the Uint8Array constructor an ArrayBuffer (the ArrayBuffer is used as-is without making a copy).

I think the question here is why does TypeScript let you pass a Uint8Array to a call declared as expecting an ArrayBuffer since they really aren't interchangeable. I think that's because when the types don't match, TypeScript falls back to "duck typing". A strictly conforming ArrayBuffer instance (prior to resizable support in ES2024, at least) has the following properties (beyond Object.prototype): byteLength, slice, and detached. Maybe TypeScript decides that matches and the addition of concat by XS breaks the duck typing check. If you enable ES2024 in TypeScript, does your example still pass type checks under Node?

[cmidgley]

If you enable ES2024 in TypeScript, does your example still pass type checks under Node?

I have es2022, strict: true, strickNullChecks: true, and noImplicitAny: true. I did go to the TypeScript playground, where I can select es2023 (it does not have es2024) and it works (see this playground).

Maybe TypeScript decides that matches and the addition of concat by XS breaks the duck typing check.

I do think you are correct that concat causes duck typing to not match, and that's even worse with ArrayBufferLike due to HostBuffer.

Node's typings allow duck typing to work for things like UInt8Array ->ArrayBuffer and ArrayBufferLike -> ArrayBuffer. I don't know if that's intentional (and long-term supported) or just how it works today. However, third-party NPM modules depend on this, and while technically wrong (they should use ArrayBufferLike), it makes it difficult to port them over. For example, @nats-io has many cases where they pass a Uint8Array to an ArrayBuffer method. I can't fix this without forking and owning merging forever, and that would make it impractical with mcpack. I may be able to suggest they change it with a PR, and I can look into that if necessary, but I'll bet they aren't the only ones doing this.

You can ask ArrayBuffer.isView about anything.

My thinking about ArrayBuffer.isView is that by having the typeguard on ArrayBuffer it implies they intend for an ArrayBuffer to be a buffer or a view of a buffer. By checking if it's a view you get extended properties on the buffer (via ArrayBufferView) and can make smart decisions about how to consume it ... but maybe that's tea leaf reading.

So given all this, if the goal is type compatibility with Node, we might consider something like this:

1. Extend HostBuffer to be duck type compatible with ArrayBuffer (by adding the missing properties), as existing node code knows nothing about HostBuffer

2. Add the ArrayBuffer.isView type guard so it's Node compatible, and we can be efficient about reusing a view rather than cloning it in methods that receive an ArrayBufferLike.

3. Adding the XS extension concat to the various view objects, so they remain duck type compatible with ArrayBuffer (and therefore, with ArrayBufferLike)

An approach similar to this would extend the functionality a bit on the core XS buffer services, but hopefully make it type cross-compatible with Node. The devil is in the details, so some experiments would need to be done to be sure the end result works (mock the types and verify compatibility before investing in the implementations).

p.s. I mentioned earlier @nats-io. It's an impressive scalable messaging, object store, and distributed service execution engine with a well-abstracted TypeScript client library (@nats-io/nats.js). It's quite large and complex, and I have gotten it working on XS simply with polyfills (and my concat?` hack) and not a single line change to their library. All on cheap little ESP32 with 4MB PS-RAM. I'm totally blown away at the power of XS!

[phoddie]

I found an ES2024 TypeScript PR. It contains an update to ArrayBuffer with resizable support. Replacing the ArrayBuffer support in xs.d.ts and building your example gives:

main.ts:16:13 - error TS2345: Argument of type 'Uint8Array' is not assignable to parameter of type 'ArrayBuffer'.
  Type 'Uint8Array' is missing the following properties from type 'ArrayBuffer': maxByteLength, resizable, resize, detached, and 2 more.

16 doSomething(myArray);
               ~~~~~~~

So.... the problem isn't concat but that you got lucky, thanks to duck typing and the multitude of options in the TypedArray constructors, in ever being able to pass a Uint8Array to a function expecting an ArrayBuffer. I'm not sure what the right solution is, but am reasonably confident it isn't changing concat (unless ES2024 TypeScript is going to make all the additions to ArrayBuffer for resizable support optional, which seems unlikely). My feeling is that this should be fixed in the script (doSomething, etc):

function doSomething(buffer: ArrayBufferLike): void {}
...
doSomething(myArray.buffer);

Note that ArrayBufferLike is what doSomething intends to accept, since it would also work with a SharedArrayBuffer (in the standard) and with a HostBuffer under XS.

---

Changing HostBuffer isn't an option -- host buffers are created in many places and are deliberately minimal. Any... anyway, you would also have to deal with SharedArrayBuffer being different (it is growable in ES2024, not resizable!).

---

Your @nats-io experience is great. I've had good luck, as have others, pulling in npm packages with minimal dependencies and more-or-less having them "just work" under XS. We should collect those together somehow... it would benefit more folks. But that's definitely a separate conversation from this thread.

[cmidgley]

Thanks for all your time on this. I really appreciate it.

I'll work with the @nats-io and try to get this changed (they've been very responsive so far). Having es2024 as a reference point will help the argument.

I'll update this PR to remove the xs.d.ts change, and see what other feedback you may have at that time.

[phoddie]

Thanks for sticking with this. It is a small point, but I don't want to make strange changes to core typings without understanding them well first.

The rest of the PR looks reasonable, but for one small detail. The in WorkerOptions everything is optional (see the implementation of getIntegerProperty() in modWorker.c) and number and symbol are also numbers; so it should be more like this:

	export interface WorkerOptions {
 		static?: number;
 		chunk?: {
 			initial?: number;
 			incremental?: number;
 		};
 		heap?: {
 			initial?: number;
 			incremental?: number;
 		};
 		stack?: number;
 		keys?: {
 			initial?: number;
 			incremental?: number;
 			name?: number;
 			symbol?: number;
 		};
 	}

[cmidgley]

Excellent - not sure how I ended up with string (goes to show I don't use those...), but I didn't realize the optionality of the others. Changes made, and open to any other suggestions. Thx.

[mkellner]

This has been committed and will be available in the next update.