-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add support for basic hash/object conversions #201
Conversation
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.
Hi there! Thanks for the contribution!
At first glance, I think this is pretty good. I've got a few nits.
- Add to the
CHANGELOG.md
. - Remove or make add more of a description to the
README.md
in the hash example.
I don't wanna expand the scope of this task too much but I wonder if we care about impl<'a, T> JSValue<'a> for HashMap<U, T> where U: Eq + Hash + JSValue, T: JSValue
(and TryIntoJS
). Thoughts?
Ha, I used the array example as a template and obviously missed some cruft. I will update the readme and changelog, and give the additional impl some thought. |
I played around a little with the idea of a generic hash key type, but I think it would effectively mean needing to support |
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.
LGTM! Thanks!
LGTM |
bors r+ |
1 similar comment
bors r+ |
Was there a reason this wasn't merged? It'd be very useful to have native support for HashMap. |
Addresses #169