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

fix: include trait bounds in where clause of From<JsValue> #31

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

Pantamis
Copy link

This should once and for all (I hope 😅) fix #25
I also updated the e2e test to catch the case of trait bounds (and even lifetime constraint)

However I am not very satisfied by this fix because it actually replaces Self in where clause with the type on which we are implementing IntoWasmAbi and From<JsValue>which I think is a bit hacky and maybe not how the syn::Generics interface is intended to be used.

A "more syn friendly interface" could be to not implement From<JsValue> but Into directly and use Self as usual, this will however require to disable a clippy warning.

So every solution I think of are meh, one thing for sure: we really need those e2e tests with more case coverage 😬

Let me know if you have a better idea to fix this @siefkenj @nappa85

@siefkenj
Copy link
Owner

I don't think this is the right solution. You should grab the user's specified where clause from cont.serde_container.original.generics.where_clause and append that to the one you're generating for From.

@Pantamis
Copy link
Author

Thanks for the hint ! My main issue was that I did not know how to get the #ty_generics tokens to build the where clause before calling split_for_impl but you can actually use the #generics to get the same tokens directly, I did not know that !

So I can actually borrow the generics from the input container one more time to build the correct where clause on the cloned and mutated one.

I think it is now a much more satisfying solution 👍

@siefkenj siefkenj merged commit cfc19b8 into siefkenj:main Apr 16, 2024
3 checks passed
@Pantamis Pantamis deleted the second_fix_22 branch April 16, 2024 17:47
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

Successfully merging this pull request may close these issues.

Regression: into_wasm_abi requiring unnecessary trait bound
2 participants