-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Complete integration of @hc-spartan/holo-hash
or revert
#265
Comments
I would advocate against using objects in client responses as that will create interoperability issues if someone is using a different js-client. I already noticed that the latest PR breaks apps using that new client within Moss which is using an older version of the client because Moss expects pure Uint8Arrays in iframe messages from apps but it gets javascript objects instead. |
I think it would be smart to stay with the pure serialized responses as anyone would get them when decoding websocket responses from javascript and keep these classes as optional sugar to make things more ergonomic in other contexts. |
I agree... Right now I think the PR makes the types for the responses coming from the I think for this to be properly implemented it should be at the level of msgpack's extension types, where one of the extension types would be |
Reverting for now. I think the JS client could provide its own class for hashes as is implemented by @hc-spartan/holo-hash with useful methods and proper comparison operators, but would need to convert all Uint8Array response types that are hashes to instances of this class, as you all pointed out. I don't have a strong preference here. If the majority prefer to have hashes be a specific class, I'm happy to properly implement that with more thought this time. Regarding the implementation, msgpack extension types could be used to encode that the value is of a hash type or even which type of hash it is. In my opinion that's an additional layer of type consistency and the JS client implementation of hash types does not depend on it. |
This PR #259 introduced some bugs: by using the classes in
@spartan-hc/holo-hash
for type definitions it makes it appear that the types you are getting back from client calls are instances of those classes, but they're not.We either need to modify client responses to transform Uint8Arrays into their expected
@spartan-hc/holo-hash
class instances, or to revert.The text was updated successfully, but these errors were encountered: