-
Notifications
You must be signed in to change notification settings - Fork 29
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
Migrate to tauri v2 #1400
Migrate to tauri v2 #1400
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Merge branch 'main' into pierremtb/issue1349
The API changed a bit here, which forces us to use the unstable crate feature. The call to open devtools is also new; it's now on the webview not window. Signed-off-by: Paul R. Tagliamonte <paul@kittycad.io>
817ed41
to
7846136
Compare
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.
This is good work @pierremtb! I just have a couple of questions and I want to allow other teammates a chance to approve since it's a big update. I'll get around to some testing of the built versions of this today or Monday
app.shell() | ||
.open(auth_uri.secret(), None) |
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.
This is a much nicer API, neat
src-tauri/capabilities/migrated.json
Outdated
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.
Are these used, or being kept here for posterity? I don't see how they would be loaded, maybe a magic file location?
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.
They're used. I don't know why the name is migrated.json. That was the auto porting script's making. We should rename it to something less confusing
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.
Renamed to desktop.json
which looks standard from what I saw at https://beta.tauri.app/references/v2/acl/
await rename( | ||
await join(context.selectedDirectory.path, oldName), | ||
(await join(context.selectedDirectory.path, name)) + | ||
(name.endsWith(FILE_EXT) || isDir ? '' : FILE_EXT), | ||
{} |
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.
Nice this much more readable now
// From https://github.com/tauri-apps/tauri/blob/1.x/tooling/api/src/fs.ts#L159 | ||
// Removed from tauri v2 | ||
export interface FileEntry { | ||
path: string | ||
/** | ||
* Name of the directory/file | ||
* can be null if the path terminates with `..` | ||
*/ | ||
name?: string | ||
/** Children of this entry if it's a directory; null otherwise */ | ||
children?: FileEntry[] | ||
} | ||
|
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.
Is there a better type I should use from @tauri-apps/plugin-fs
? I'd rather we use their type if they have one exposed, otherwise couldn't we have a mismatch between our expected shape of the data and the actually-returned data from their read function. I guess it's not a concern since we're shimming our own read_dir_recursive
function from V1 everywhere?
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.
@franknoirot There is a new type yes, but it's not recursive-able. So unless we find a way to rewrite the filesystem reading logic we have to use the non-recursive readDir
we can't really use it I think
It's DirEntry
, defined at https://github.com/tauri-apps/plugins-workspace/blob/v2/plugins/fs/guest-js/index.ts#L625-L641
c2141ba
to
a8094ba
Compare
@franknoirot No I can't find anything that points out why |
Merging after extensive manual testing of the Windows and macOS builds. |
Will fix #1400 and unblock #625
Made the compromise of disabling the linux e2e tests so we could merge and get out of rebase hell, and will work on re-enabling them in a new PR.
Also, I can't get the cargo test to show green. They don't seem very happy in general when I look at https://github.com/KittyCAD/modeling-app/actions/workflows/cargo-test.yml
@franknoirot I updated a couple of path concatenations to
join
in async contexts, and kept tosep
(which is now a function!) in other contexts. Most breaking changes came from the new fs plugin, I did my best to test things but we'll have to see how it all plays out.Another note: the recursive option is now gone from tauri's
readDir
, in order to limit the amount of rewrite needed, I added the tauri v1 code under aread_dir_recursive
rust function we invoke on the web side. The code is atsrc-tauri/src/main.rs
. I believe with their MIT and Apache 2.0 license combo we're in the clear for doing that. Let me know if you have concerns around this.Also had to change the wasm url from https://tauri.localhost to http://tauri.localhost after hitting an issue on Windows and reading this comment