bindings/crypto-nodejs: Take strings for user IDs, device IDs, and room IDs

[turt2live]

otherwise we end up with a lot of useless object creation in the JS side. For example:

const roomMembers = await this.client.getRoomMembers(roomId);
const keysClaimReq = await this.machine.getMissingSessions(roomMembers.map(u => new UserId(u)));

we probably don't need these objects when it's clear they will be user IDs.

[turt2live]

(non-blocker for bindings usage)

[Hywan]

Hey :-),

Thanks for opening this issue. I will probably not fix this, as it's intentional. Creating a new UserId or RoomId needs to parse the string to ensure it's fully valid. We don't want to handle all those parsing, validation and errors everytime we expect a user ID or a room ID, inside our function bodies. Hence those types. Also, it clarifies what functions expect with proper types. I understand that looping over collections of ID on your side may be annoying, but it's for the better I believe :-). I'm opened to suggestions though.

[turt2live]

This will have performance impacts down the line where I think it'll be required to make this change, given the massive number of objects which will be landing on the stack.

Validation this strict is also highly questionable as it provides very little value. The validation comes from other parts of the overall system like the server itself or the consequences of getting it wrong (ie: it's not the SDK's problem that the consumer supplied an invalid user ID).

[Hywan]

This will have performance impacts down the line where I think it'll be required to make this change, given the massive number of objects which will be landing on the stack.

The Rust API needs to parse and validate them in all cases as the internal API uses ruma::UserId & co. as types, not Strings.

Validation this strict is also highly questionable as it provides very little value. The validation comes from other parts of the overall system like the server itself or the consequences of getting it wrong (ie: it's not the SDK's problem that the consumer supplied an invalid user ID).

Well, first, the internal API requires that, and second, we must never trust data provided by the user, even if it is a Matrix server.

[Hywan]

Triaging. I'm closing it but feel free to re-open, I'm OK to discuss about that if it's a real blocker.

[turt2live]

I'm going to reopen this as it's a major consideration for whether the bot-sdk continues to use the rust bindings or not.