Skip to content
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

Generic maps aren't ergonomic in WRAP 0.1 #7

Open
dOrgJelli opened this issue May 2, 2023 · 2 comments
Open

Generic maps aren't ergonomic in WRAP 0.1 #7

dOrgJelli opened this issue May 2, 2023 · 2 comments

Comments

@dOrgJelli
Copy link
Member

Currently generic maps (Map<K, V>) are serialized using a custom msgpack extension. Additionally when they're deserialized within WebAssembly, it is not "friendly" and only accepts the custom extension, and does not accept standard msgpack maps (what we use as Objects).

This leads to problems within Rust & Python (for example) that @namesty and @Niraj-Kamdar have brought up.

@Niraj-Kamdar
Copy link
Contributor

We should fix this in abi-0.2 when we have support for custom extension types. We should have capability of changing bindings for that and injecting it during codegen process. Since every type can be represented as array, object or scalar, Wasm binding layer should be able to deserialize the type correctly even if it's not marked with extension type marker.

@namesty
Copy link

namesty commented May 2, 2023

If we zoom out a bit, my hot take is I think that for right now (abi-0.1) it's just easier not to use the GenericMap extension type at all; and just have something like the following:

Instead of this:

type Module {
  mapMethod(
    map: Map @annotate(type: "Map<String!, Int!>")
  ): Map @annotate(type: "Map<String!, Int!>")
}

Have this:

type MapEntry {
  key: String!
  value: Int!
}

type Module {
  mapMethod(
    map: [MapEntry]
  ): [MapEntry]
}

May be more text but:

  • Does not use special syntax
  • No annotations required
  • Doesn't use anything outside Graphql's standard toolkit
  • Won't need special representation or serialization or deserialization
  • No need to explain user how to use it, how to build one (BTreeMap in Rust, wrapped in a polywrap_client::msgpack::Map, etc.)

These are all burdens for users and for ourselves. I think the complexity of what we introduce with the GenericMap type significantly outweighs the benefit of not needing to access the key or value prop everytime; or having an additional type for each unique key-value combination

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants