feature: Adding option to use custom Id in request

[ivankhm]

Hello!
I think there should be a way to use a custom Id for the request. Besides, the current way can be unsafe, for example, if 2 clients simultaneously make the first request - they will have the same id, which can be confusing for a server.

I suggest passing a function into the constructor, that returns the next id, and using it in this method:

client-js/src/RequestManager.ts

Line 48 in 7909cd9

const internalID = (++this.lastId).toString();

like this:

const internalID = this.getNextId();

And initiate this.getNextId in the constructor via parameter or default value.

[BelfordZ]

Thanks @ivankhm for the issue!

This is 100% something that we need to address. If you want to take a stab at a PR, feel free to take a stab. I've created another issue to track the 'custom id' function, as well as a flag to switch from string to number for the id's type, if you want to solve both, that would be sweet!

[zcstarr]

Yeah I can see this being problematic, instantiate two clients to the same server, you have no way to manage the id's for those request, they're both listening for the same server responses. There's no way to coordinate between them.

I could imagine this being as simple as abstracting id generation Logic as you suggest to some interface around

interface RequestIDGenerator {
  nextID(): int | string
}

function requestGenerator() {
   let idCount = 0
   return () => {
      idCount = idCount + 1
      return idCount;
   }
}

Where a simple closure can encapsulate some stateful data.

I think you want the following at the end of the day.

new GeneratedClient( options: {requestOptions: {nextId: ()=>number| string})
new Client(new RequestManager([transports],requestOptions))

Then we can have the behavior remain the same for people that don't care about that management. Additionally people using multiple clients can just lean on the same generator and won't hit that collision point around instantiating multiple clients.

Let me know what you think or if this makes sense to y'all @BelfordZ @ivankhm

[BelfordZ]

@zcstarr yeah something like that. Though the generatedClient part will be done elsewhere, and the options should reference the classes by name, so more like
new GeneratedClient( options: {requestManager?: RequestManagerOptions )

For this specific part, we would just be adding, as you said, constructor options for RequestManager (along with exported RequestManagerOptions interface). The first constructor option being nextId function, and as highlighted in #279 , an option to use a number instead of a string if using the default nextId functionality

[openrpc-bastion]

🎉 This issue has been resolved in version 1.7.0 🎉

The release is available on:

• GitHub release
• 1.7.0
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀