-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor for bindgen updates #15
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
Conversation
| // we're just exposing the serde values directly for backwards compat | ||
| return optionalize("std::collections::HashMap<String, serde_json::Value>") | ||
| } | ||
| return "i64"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: we have a type for i64 but don't yet support it because of json limitations. i have an issue filed for this.
* Refactor: see dylibso/xtp-bindgen#18 * Support nullable vs required
c87a800 to
eb8f8e9
Compare
| return optionalize(`types::${helpers.capitalize((type as EnumType).name)}`) | ||
| case 'map': | ||
| const { keyType, valueType } = type as MapType | ||
| return optionalize(`std::collections::HashMap<${toRustTypeX(keyType)}, ${toRustTypeX(valueType)}>`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to use serde_json::Value::Map instead of HashMap while we're here. (serde_json::Map gives the plugin author the ability to preserve key order.)
|
Closing since this work was included in #18 |
See dylibso/xtp-bindgen#18
Required Vs Nullable
Picking up where #10 left off . How i am implementing required vs nullable
This schema:
Results in this object: