Skip to content

feat(tauri/Emitter): add emit_str* method to emit serialized data #12460

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

Merged
merged 2 commits into from
Jan 26, 2025

Conversation

WSH032
Copy link
Contributor

@WSH032 WSH032 commented Jan 21, 2025

close #12459

@WSH032 WSH032 requested a review from a team as a code owner January 21, 2025 05:19
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

this feels more like emit_str than emit_json, let's rename

let _span = tracing::debug_span!("window::emit::json").entered();
Ok(EmitArgs {
event_name: event.into(),
event: serde_json::to_string(event)?,
Copy link
Member

Choose a reason for hiding this comment

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

let's remove this serde call as well, it is unnecessary, also do the same for EmitArgs::new.

Don't forget to update emit_js_script function later down in this file to add wrapping single quotes around event name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's remove this serde call as well, it is unnecessary, also do the same for EmitArgs::new.

I don't quite understand, do you mean event: event.into()? If so, wouldn't event_name and event be the same?

Don't forget to update emit_js_script function later down in this file to add wrapping single quotes around event name.

Wouldn't this cause issues with escaping " and '?

Copy link
Member

Choose a reason for hiding this comment

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

it shouldn't be an issue because event names are validated to contain alphanumeric, -, /, : and _

Copy link
Member

@amrbashir amrbashir Jan 21, 2025

Choose a reason for hiding this comment

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

we can remove the event_name as well, seems uneeded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I call it a day. If there are any other changes needed, I will handle them tomorrow; or feel free to push to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, I will wait on #12444 first before we can move forward with this PR as it touches the same code

@AzharHussain366
Copy link

AzharHussain366 commented Jan 21, 2025 via email

Copy link
Contributor

github-actions bot commented Jan 21, 2025

Package Changes Through 43c993e

There are 7 changes which include tauri-cli with minor, tauri-runtime with minor, tauri-runtime-wry with minor, tauri-utils with minor, tauri with minor, @tauri-apps/api with minor, @tauri-apps/cli with minor

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
@tauri-apps/api 2.2.0 2.3.0
tauri-utils 2.1.1 2.2.0
tauri-bundler 2.2.3 2.2.4
tauri-runtime 2.3.0 2.4.0
tauri-runtime-wry 2.3.0 2.4.0
tauri-codegen 2.0.4 2.0.5
tauri-macros 2.0.4 2.0.5
tauri-plugin 2.0.4 2.0.5
tauri-build 2.0.5 2.0.6
tauri 2.2.5 2.3.0
@tauri-apps/cli 2.2.7 2.3.0
tauri-cli 2.2.7 2.3.0

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@amrbashir
Copy link
Member

#12444 is now merged, we can move on with this PR once you fix the conflicts

@WSH032
Copy link
Contributor Author

WSH032 commented Jan 24, 2025

will do it now

@WSH032
Copy link
Contributor Author

WSH032 commented Jan 26, 2025

@amrbashir. Sorry for pinging you, could you please review this PR? I noticed that tauri v2.3 #12518 is about to be released, and if this PR can make it into v2.3, pytauri v0.2 will also benefit.

It's okay if you don't have time right now. I can postpone this feature to pytauri v0.3.

@FabianLars
Copy link
Member

FabianLars commented Jan 26, 2025

No worries, 2.3 still needs a few days/weeks. I just started merging minor PRs for it. I also asked Amr to not merge yours until now (because I wanted to release patches first) so this is on me :)

@amrbashir amrbashir merged commit abdd558 into tauri-apps:dev Jan 26, 2025
20 checks passed
@amrbashir amrbashir changed the title feat(tauri/Emitter): add emit_json* method to emit serialized data feat(tauri/Emitter): add emit_str* method to emit serialized data Jan 26, 2025
@WSH032 WSH032 deleted the feat/emitter branch January 26, 2025 16:00
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.

[feat] Emit json string
4 participants