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(tauri): Webview::navigate unnecessarily borrows &mut self #12461

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

WSH032
Copy link

@WSH032 WSH032 commented Jan 21, 2025

close #12430


Sorry for opening this PR without permission.

#12430 is blocking me from implementing the Python bindings for Webview::navigate. I plan to release pytauri v0.2 soon, so I hope pytauri can catch up with this PR.

Is there any specific reason why tauri::Webview needs to do this? If not, can we change it to only require an immutable borrow &self?

Not sure if my thoughts are correct, feel free to close this PR if I am wrong.

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

github-actions bot commented Jan 21, 2025

Package Changes Through 8df2800

There are 4 changes which include tauri-cli with patch, @tauri-apps/cli with patch, tauri-bundler with patch, tauri 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-bundler 2.2.2 2.2.3
tauri 2.2.3 2.3.0
@tauri-apps/cli 2.2.5 2.2.6
tauri-cli 2.2.5 2.2.6

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


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

@sftse
Copy link

sftse commented Jan 21, 2025

This would break the public API and need a major version update to respect semver.

@FabianLars
Copy link
Member

FabianLars commented Jan 21, 2025

Does this really need a major version? What would a user's code look like that works with &mut self but not &self ? I can't think of anything 🤔 From my testing this doesn't even emit check/clippy warnings which would be totally fine.

Edit: In comparison switching from &self to &mut self would indeed be a clear breaking change.

In context of #12465 and this:
{1D2E50D4-C451-4717-9215-27E28A7571F8}
i wouldn't mind a minor instead of patch though.

@WSH032
Copy link
Author

WSH032 commented Jan 21, 2025

i wouldn't mind a minor instead of patch though.

TBH, if strictly following semver, I agree more with releasing it as a minor.

I chose patch just for selfish reasons, as I worry that the tauri v2.3 release cycle might be too long, causing pytauri v0.2 to miss this PR.

Anyway, whether tauri chooses patch or minor, I am okay with it. Feel free to push to this PR.

@FabianLars
Copy link
Member

I chose patch just for selfish reasons, as I worry that the tauri v2.3 release cycle might be too long, causing pytauri v0.2 to miss this PR.

I plan to release 2.3 for the new webview2-com and windows version relatively soon - (and yes, that's actually a breaking change but we can't have it locked for the whole lifetime of v2). I just want to get a few last patches in before that.

@sftse
Copy link

sftse commented Jan 21, 2025

I withdrew my objection because I tested the cases I had in mind and they turned out to be bogus, do you have an example how this is indeed a breaking change?

fn foo<T>(_: &T) {}
fn bar<T>(t: &mut T) {
    foo(t)
}

Edit: Seems to compile, which I've been reading as proof that this is backwards compatible.

@FabianLars
Copy link
Member

do you have an example how this is indeed a breaking change?

as i said, no i don't :D I just can't think of any breakage for &mut self -> &self because you can call &self methods on mutable objects.

&mut self -> &self however is pretty obvious as you need let mut instead of let for &mut self to work.

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] Webview::navigate borrows &self instead of &mut self
3 participants