feat: JSON-RPC responses async iterable iterator

[tien]

Add support for the for await...of statement for handling responses.

Currently, to listen to all responses, you would use a while loop like this:

try {
  while (true) {
    const response = await chain.nextJsonRpcResponse();
    console.info(response);
  }
} catch (error) {
  if (error instanceof AlreadyDestroyedError) {
    console.info("No more responses");
  } else {
    console.error(error);
  }
}

This PR introduces a more concise syntax:

try {
  for await (const response of chain.jsonRpcResponses) {
    console.log(response);
  }
  console.info("No more responses");
} catch (error) {
  console.error(error);
}

In addition to the syntactic improvement and avoiding the while loop, this approach allows for closing the stream without requiring an error to be thrown.

[tomaka]

👍

[tomaka]

I would actually much prefer if this function was left as it is, and jsonRpcResponses was implemented on top of it. This function is much more simple than jsonRpcResponses from an implementation point of view.

[tien]

Having nextJsonRpcResponse be an implementation of jsonRpcResponses allow jsonRpcResponses to be more generic here, specifically stopping the iteration without having to throw an error (by returning { done: true }), whilst nextJsonRpc can still throw an error if there are no more responses (the chain is already removed), keeping backward compatibility.

But in saying that, that is the only benefit that I can think of though, I don't have too much of a strong opinion on this, let me know if you think otherwise and I'll revert this right away 🙏

[tomaka]

Why remove the chain here?

[tien]

I believe this behaviour make sense according to the documentation here. But I have removed it anyway, let me know what is your final judgement on this, thank you.

[tomaka]

What about?

Suggested change

	jsonRpcResponses: AsyncIterableIterator<string>
	readonly jsonRpcResponses: AsyncIterableIterator<string>

And turn jsonRcpResponses into a getter?

[tien]

I have made this a readonly property. I think a property make more sense compared to a getter, because there's in fact only 1 iterable here:

• Each iteration yield the next value
• If multiple iterables are created and are iterated on, they will actually overlap (similar to call nextJsonResponse simultaneously)

[tomaka]

What I meant by getter is to do get jsonRpcResponses() { return { next: ..., ... } }

[tien]

Right, here I believe just a TypeScript readonly modifier is sufficient, like technically speaking other properties/methods like sendJsonRpc, nextJsonRpcResponse, etc are actually mutable as well. If you want to enforce immutability at runtime on the JS side, I think calling Object.freeze on the entire chain object is more effective.

[tomaka]

Looks good overall, could you also add an entry to wasm-node/javascript/CHANGELOG.md? 🙏

[tien]

Looks good overall, could you also add an entry to wasm-node/javascript/CHANGELOG.md? 🙏

I've added an entry to the changelog, the file you reference doesn't seem to exist though, so I have created one accordingly 🙏

[tomaka]

I've added an entry to the changelog, the file you reference doesn't seem to exist though, so I have created one accordingly 🙏

It was in fact wasm-node/CHANGELOG.md

[tien]

I've added an entry to the changelog, the file you reference doesn't seem to exist though, so I have created one accordingly 🙏

It was in fact wasm-node/CHANGELOG.md

Okay, thanks for clarifying, have added the entry to the correct file 💪