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 double serialize on invoke #5639

Merged
merged 2 commits into from Nov 20, 2022
Merged

Conversation

LucasJavaudin
Copy link
Contributor

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

When using the invoke function asynchronously to run a Rust function and return a value that can be serialized, the serialized value is serialized a second time, uselessly. This is because of a useless conversion in the function hooks::respond_async_serialized: The task returns a Result<JsonValue, InvokeError> that is converted into a InvokeResponse by the call to into(). An InvokeResponse is in practise exactly equal to a Result<JsonValue, InvokeError> so the conversion should be immediate. However, the call to into() use the impl From<Result<T, InvokeError>> for InvokeResponse (where T: Serialize), which serialize the JsonValue into a JsonValue (and that is not instantaneous).

Copy link
Member

@lucasfernog lucasfernog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, thanks!

@lucasfernog lucasfernog merged commit 677838c into tauri-apps:dev Nov 20, 2022
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.

None yet

2 participants