-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(api): "command not found" error when running addPluginListener #14132
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
Conversation
the backend expects the command name to be in snake case we've made this change already for check_permissions and request_permissions, but missed register_listener
Package Changes Through bd253c6There are 9 changes which include @tauri-apps/api with minor, tauri-cli with minor, tauri-utils with minor, tauri-runtime-wry with minor, tauri-runtime with minor, tauri with minor, tauri-bundler with minor, @tauri-apps/cli with minor, tauri-macros with patch Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
No we did not miss it, we reverted it because for some unknown reason snake case didn't work: #11423 |
oh ok i'll need to dig deeper |
good luck 😅 i hope you can find it. i couldn't make sense of it. especially since it appears to be a bit inconsistent at times. |
well like someone once said, the whole permission validation is a bit.. messy, crazy flow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong
- On desktop, we match the ACL and the command by the exact command names
- On mobile, we still use the exact command names when matching ACLs, but we transform the command calls to camelCase
tauri/crates/tauri/src/webview/mod.rs
Line 1870 in f5851ee
heck::AsLowerCamelCase(message.command).to_string(), |
And from the current casing conventions, registerListener
seems to be an outlier? I know we can't change that in v2 or we break existing ones that uses the camelCase though
crates/tauri/src/webview/mod.rs
Outdated
&acl_origin, | ||
) | ||
.or_else(|| { | ||
if request.cmd.chars().any(|c| c.is_uppercase()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly don't like this, it means we now match the commands in different casing without formal documentation, and that's confusing, I'd rather use a try catch call in the addPluginListener
function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we just have a particular case for registerListener then? i don't really want to make two calls for a single command :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well i'll just go with your idea.. better than bloating the rust side with a lot of temp logic :(
the backend expects the command name to be in snake case
we've made this change already for check_permissions and request_permissions, but missed register_listener