js-sdk
js-sdk copied to clipboard
fix: null derefs on missing metadata.name
We can be more defensive around missing provider metadata (typings make them required, but there's always JS).
I am not sure about this one @toddbaert. In the TS world, this is "wrong" in the sense that the type does not declare it as optional, as you already said. If we were to make this change, I think we have more places where we have nested props that are accessed non optionally where in JS, nothing would prevent from making it null or undefined. The change is fine to me but I think we should be able to rely on TS here as we would otherwise have to ignore TS on several other places 🤔
I see your point, and if we were just writing an internal TS library I would be OK with that. But don't we want to support JS as well? And if so, shouldn't we generally be defensive about this sort of thing? If I'm a JS dev, it's it quite possible I forget something as simple as this metadata?
Draft for now.
I see your point, and if we were just writing an internal TS library I would be OK with that. But don't we want to support JS as well? And if so, shouldn't we generally be defensive about this sort of thing? If I'm a JS dev, it's it quite possible I forget something as simple as this metadata?
I think we should support JS, but I also think that a TS interface simply has to be implemented correctly in JS and otherwise the error is not really avoidable. I mean we could go as fa as checking the existence of the evaluation methods if we wanted to check the provider interface compliance but I would not say this should be responsibility of the SDK. Most IDEs/editors will check the type compliance by using the typedefs anyways.
Did you encounter an issue with these properties @toddbaert? I have seen it mentioned in #971 but there it was due to the casting of the incomplete type right?