RFC: Use io-ts to validate user input.

[BenPope]

RFC: This implements checks for RemoveRoomRoleForUserOptions only.

• [ ] How much existing code will break; should the checks be for values only?
• [ ] Is there a better way to allowJs without removed declaration: true
• [ ] Are the dependencies ok?
• [x] is #67 OK?
• [ ] Is the new semver ok?

What?

Define the requirements in terms of io-ts types and check them at runtime. Add tests for Room Roles

Why?

When the typescript is converted to javascript, all type checks are lost.

---

• [x] CHANGELOG updated if relevant?

[callum-oakley]

I like it. Hadn't seen io-ts before. It is a whole bunch of extra machinery though... and I'd be happier if we were using it everywhere, but maybe using it in this one method is a good test bed for whether we want to put the work in to do it for the whole API?

When you say should the check be for values only... do you mean that some people might be feeding numbers to removeRoomRoleForUser, and it wouldn't have been an issue before, but now it will be? If so I don't feel strongly about it. The public interface has always been documented as taking strings, so it's not strictly a breaking change to start rejecting non-strings.

I think the real issue here is with the API being too "clever" and interpreting an empty string room ID as indicating a global role. Maybe we should change that in V7 instead and then all the SDKs would get this fix for free. I'd be surprised if this issue didn't also feature in all our other SDKs.

[BenPope]

I like it. Hadn't seen io-ts before. It is a whole bunch of extra machinery though... and I'd be happier if we were using it everywhere, but maybe using it in this one method is a good test bed for whether we want to put the work in to do it for the whole API?

The intention is to use it everywhere.

When you say should the check be for values only... do you mean that some people might be feeding numbers to removeRoomRoleForUser, and it wouldn't have been an issue before, but now it will be?

More likely it would be a parameter on another method, perhaps messageId.

If so I don't feel strongly about it. The public interface has always been documented as taking strings, so it's not strictly a breaking change to start rejecting non-strings.

Fair enough.

I think the real issue here is with the API being too "clever" and interpreting an empty string room ID as indicating a global role. Maybe we should change that in V7 instead and then all the SDKs would get this fix for free. I'd be surprised if this issue didn't also feature in all our other SDKs.

That makes sense to me.