Skip to content

Use format_callback::format_raw for channels - #13288

Merged
Legend-Master merged 6 commits into
tauri-apps:devfrom
oscartbeaumont:error-handling-for-channels-callback
Apr 29, 2025
Merged

Use format_callback::format_raw for channels#13288
Legend-Master merged 6 commits into
tauri-apps:devfrom
oscartbeaumont:error-handling-for-channels-callback

Conversation

@oscartbeaumont

Copy link
Copy Markdown
Member

Right now when reloading the webview a channel from the previous page might fire. This will cause some JS code similar to window[_some_id](...) to be injected into the page. When window[_some_id] evaluates it will be undefined as the webview context was reset when the page was reloaded. This will cause the webview to crash, sometimes taking out other JS running on the page with it.

This fix makes use of the existing format_callback::format_raw for parsing data back to the webview which has graceful handling on this condition.

@oscartbeaumont
oscartbeaumont requested a review from a team as a code owner April 23, 2025 23:40
@github-actions

github-actions Bot commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

Package Changes Through 7900367

There are 4 changes which include tauri with minor, @tauri-apps/api with minor, tauri-cli with patch, @tauri-apps/cli with patch

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.5.0 2.6.0
tauri 2.5.1 2.6.0
@tauri-apps/cli 2.5.0 2.5.1
tauri-cli 2.5.0 2.5.1

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


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

Comment thread crates/tauri/src/ipc/channel.rs Outdated
Comment thread crates/tauri/src/ipc/channel.rs Outdated
let _ = webview_clone.eval(format!(
"window['_{callback_id}']({{ end: true, index: {current_index} }})",
));
let _ = webview_clone.eval(format!("window['_{callback_id}']()",));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't quite understand this change, is this some left over code?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was just a mistake.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Then why don't we remove it? let _ = webview_clone.eval(format!("",)); doesn't make sense either

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, misunderstood. Fixed.

@Legend-Master

Copy link
Copy Markdown
Contributor

Also a change file like this one would be nice 🙃

@oscartbeaumont

Copy link
Copy Markdown
Member Author

I have applied the fixes and added a changelog.

I have been unable to test these changes as when I run pnpm build I get the following:

> tauri-workspace@0.0.0 build /Users/oscar/Desktop/tauri
> pnpm run -r build

Scope: 6 of 7 workspace projects
examples/api build$ vite build
│ vite v6.2.6 building for production...
│ transforming...
│ ✓ 6 modules transformed.
│ ✗ Build failed in 104ms
│ error during build:
│ [vite]: Rollup failed to resolve import "@tauri-apps/api/core" from "/Users/oscar/Desktop/tauri/examples/api/src/App.svelte".
│ This is most likely unintended because it can break your application at runtime.
│ If you do want to externalize this module explicitly add it to
│ `build.rollupOptions.external`
│     at viteLog (file:///Users/oscar/Desktop/tauri/node_modules/.pnpm/vite@6.2.6_@types+node@22.13.5_jiti@2.4.2_sass@1.77.8_terser@5.39.…
│     at onRollupLog (file:///Users/oscar/Desktop/tauri/node_modules/.pnpm/vite@6.2.6_@types+node@22.13.5_jiti@2.4.2_sass@1.77.8_terser@5…
│     at onLog (file:///Users/oscar/Desktop/tauri/node_modules/.pnpm/vite@6.2.6_@types+node@22.13.5_jiti@2.4.2_sass@1.77.8_terser@5.39.0_…
│     at file:///Users/oscar/Desktop/tauri/node_modules/.pnpm/rollup@4.40.0/node_modules/rollup/dist/es/shared/node-entry.js:20704:32
│     at Object.logger [as onLog] (file:///Users/oscar/Desktop/tauri/node_modules/.pnpm/rollup@4.40.0/node_modules/rollup/dist/es/shared/…
│     at ModuleLoader.handleInvalidResolvedId (file:///Users/oscar/Desktop/tauri/node_modules/.pnpm/rollup@4.40.0/node_modules/rollup/dis…
│     at file:///Users/oscar/Desktop/tauri/node_modules/.pnpm/rollup@4.40.0/node_modules/rollup/dist/es/shared/node-entry.js:21288:26
└─ Failed in 1.1s at /Users/oscar/Desktop/tauri/examples/api
packages/api build$ rollup -c --configPlugin typescript
└─ Running...
packages/cli build$ cross-env TARGET=node napi build --platform --profile release-size-optimized
└─ Running...
/Users/oscar/Desktop/tauri/examples/api:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  api@1.0.0 build: `vite build`
Exit status 1
 ELIFECYCLE  Command failed with exit code 1.

Following the CONTRIBUTING.md this is required to run examples/api which would let me to test if these changes are working correctly.

@Legend-Master

Copy link
Copy Markdown
Contributor

I have been unable to test these changes as when I run pnpm build I get the following:

What if you build the API and CLI and then go to the example/api directory and run pnpm build? I think it might be the API build task deleted the dist folder when regenerating out files while vite is trying to build from them

Comment thread crates/tauri/src/ipc/format_callback.rs Outdated
@oscartbeaumont-crabnebula

Copy link
Copy Markdown

What if you build the API and CLI and then go to the example/api directory

The following works:

# in the `/` directory of the repo
pnpm i
cd packages/api
pnpm build
cd ../cli
pnpm build
cd ../../examples/api
pnpm tauri dev

Thanks for your help!

@Legend-Master
Legend-Master merged commit 94b77b3 into tauri-apps:dev Apr 29, 2025
@Legend-Master

Copy link
Copy Markdown
Contributor

Nice catch, good 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.

3 participants