Skip to content

fix double serialize on invoke#5639

Merged
lucasfernog merged 2 commits intotauri-apps:devfrom
LucasJavaudin:invoke
Nov 20, 2022
Merged

fix double serialize on invoke#5639
lucasfernog merged 2 commits intotauri-apps:devfrom
LucasJavaudin:invoke

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.

2 participants