svix-webhooks
svix-webhooks copied to clipboard
Libs: make the payload parameter of create message generic, not "any" for all libs
In rust: should probably just be a generic over json serializable if it's not already.
For everything else (that supports generic): just make it generic. This will let people have more strict type checking.
Did a survey of what's what right now.
- [x] C# (typed as
Object
) - [x] Go (typed as
map[string]interface{}
which is probably fine) - [x] Java (typed as
Object
aka POJO) - [ ] JavaScript (the ts type is currently
any
, :-1:) - [ ] Kotlin (typed as
Any
:-1:, need to check to see how generics play with Json lib or otherwise switch toHashMap<String, Any>
) - [x] PHP seems to not offer APIs for create message :shrug:
- [x] Python (typed as
Dict[str, Any]
which is probably okay) - [ ] Ruby - not sure what to do here. Looks like we could add some runtime checks to the initializer but...
- [x] Rust (typed as
serde_json::Value
which is okay but it would be better if it insisted on an Object variant)
I'm basing this assessment on the fact I thought the payload
was meant to be an object, not any valid json value.
This is to say, I'd have expected it to be an error if I sent a create message request with:
{"eventType": "nonsense", "payload": false}
The object requirement not be the case according to our spec, but at least some of the code generators seem to be adhering to this.
A problem I see is the typing here is entirely in the hands of the OpenAPI codegen since this is a field on MessageIn
. I need to learn more about how to influence what the generators spit out in each problem case.
@svix-onelson, I actually meant something, else, but what I meant didn't make sense. Though looking at your report, and given that it's based on MessageIn, I don't think there's anything sane we can do at this point. So maybe let's take a pause on this ticket?
@tasn sounds good.
I think we're actually in pretty decent shape as things are. If I were to cherry-pick one of these, it'd be JS. Re-typing as {[key: string]: any}
would bring things back inline with most of the others in the list.
Yeah, but changing the generation is a bit of a pita. I suspect that maybe updating the generator version, or maybe even fixing our spec (in case it's lacking) would be sufficient.