-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
TryIntoJs
trait and derive macro for it
#3999
Conversation
is it ok to use `create_data_property_or_throw`? in other words, am I create an object correctly?
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.
Why do we need to have a separate derive and implementation for it we can have an automatic impl
for it, that uses TryFromJs
. Like the standard library does it: https://doc.rust-lang.org/beta/src/core/convert/mod.rs.html#788-815. Implementing TryFromJs
should automaticaly implement TryIntoJs
.
Am I missing something?
In std mod we can have an object of any type as value in So
There is try_js_into fn that do the same as |
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.
Ahh, I see, yes that makes sense!
The PR looks good to me! :)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3999 +/- ##
==========================================
+ Coverage 47.24% 52.80% +5.55%
==========================================
Files 476 481 +5
Lines 46892 46791 -101
==========================================
+ Hits 22154 24706 +2552
+ Misses 24738 22085 -2653 ☔ View full report in Codecov by Sentry. |
I should probably be a |
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.
Pretty impressive work! Looks really nice :)
This Pull Request closes #3874
Notes
HashMap
intoJsObject
.Should it be converted into
JsMap
or into ordinary object{ k1: v1, ..., kN: vN }
?Or maybe I need to add a macro attribute to allow choose a way of conversion of
Map
types?