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

Refactor: Move all @tauri-apps calls to src/commands #46

Merged
merged 6 commits into from
Sep 9, 2024

Conversation

maidh91
Copy link
Contributor

@maidh91 maidh91 commented Sep 7, 2024

Context

  • This is one step in the plan to support this app running on web browser, and mobile
  • Goal
    • in Desktop mode, src/commands will calls to @tauri-apps
    • in Web browser mode, src/commands will calls to some backend
      • eventually, the dir structure can be like this
wealthfolio/
├── src/
├── src-tauri/
├── src-rust-api/    # some REST api backend
├── src-rust-lib/    # shared rust code using `src-tauri` and `src-rust-api` 

Summary

  • Move all @tauri-apps calls to src/commands

    • This src/commands directory contains all actions to communicate with Tauri via @tauti-apps
      library, e.g. invoke, listen, open
    • All other directories under src are native React code only
  • Add README.md to src/commands to note the convention

@maidh91
Copy link
Contributor Author

maidh91 commented Sep 7, 2024

@afadil CR pls

Thank you for a great project Fadil.
I would love to contribute to open-source projects like yours.
Happy to join some chat tool for more development discussion (you should have a Discord for this masterpiece)

@afadil
Copy link
Owner

afadil commented Sep 7, 2024

Thank you @maidh91 for your contribution to this project.

Just to understand the context here, when you say you want the app to run in the browser, is this mean to have a full web version with an backend api hosted in a server and client SPA app?
This is a big change to the scope the the app. And in this case I don’t think Tauri is the right framework (Tauri is designed for desktop and mobile apps).
Can you please describe the architecture you have in mind ?

@maidh91
Copy link
Contributor Author

maidh91 commented Sep 7, 2024

Hi @afadil, thanks for your response, I was waiting for it the whole day.

First of all, allow me to say 'we' sometime because I really want to be a part of this amazing project 🤗🤗🤗

  • I don't want to make any big changes to the scope of your architecture. The idea is just to extend it gradually without any big change.
  • Currently, this app is using React for Frontend (FE), and Tauri + Rust for Backend (BE) => this is a combination I am using in my projects
  • We can take leverage of this stack to support desktop web browser, mobile web browser, and mobile native app

In the future, what we can have are

  • Desktop app: React + Tauri + Rust (support all OS)
  • Desktop web standalone: React + some browser storage (support all browsers)
  • Desktop web cloud: React + Rust backend hosted in some cloud (support all browsers)
  • Mobile web standalone (support all browsers)
  • Mobile web cloud (support all browsers)
  • Mobile native app (android, ios)

In terms of technical details

  • I really enjoyed reading your code, love the architecture and structure (tbh, I am using that in most of my side projects, 99% same I think)
  • In your current code + this PR, everything in src will be native React code, the only place calling to outside is under src/commands
  • Now, src/commands is calling Tauri => Later, src/commands can be an abstract layer to call to different places
    • Desktop app: src/commands calls Tauri
    • Desktop web standalone: src/commands calls some browser storage framework
    • Desktop web cloud: src/commands calls some REST endpoint on the cloud

If you can look at my other PRs

The structure can be like this

wealthfolio/
├── src/                                # for FE with React
├── src-tauri/                       # for desktop app with Tauri
├── src-browser-storage/    # for web standalone running in browser + browser storage only 
├── src-rust-api/                  # for web cloud to call to this REST api running on some cloud
├── src-rust-lib/                   # shared rust code using `src-tauri` and `src-rust-api` 

@maidh91
Copy link
Contributor Author

maidh91 commented Sep 7, 2024

Hi @afadil,

This PR is safe btw, no logic change, just moving code of calling @tauri-apps into one place src/commands

@maidh91
Copy link
Contributor Author

maidh91 commented Sep 8, 2024

hi @afadil

#69 => here is the code to extend src/commands to support multiple platforms

@afadil
Copy link
Owner

afadil commented Sep 8, 2024

Hi @maidh91
I’m afraid supporting multiple platforms is gonna bring a lot of complexity at this stage. I would rather focus on features and reliability now.
Also the whole spirit is the have all data and compute in local laptop (or phone later). The target here is people who want privacy and not willing to give their data to third party.

I can see in the future a mobile app and some native sync of the SQLite db between devices (using iCloud for example).

I’ll write some bullet points for a short term roadmap and I’ll be glad if you can contribute to this vision.
🙏

@maidh91
Copy link
Contributor Author

maidh91 commented Sep 8, 2024

Hi @afadil

I 100% agree with you on privacy which is also my job now. I am mainly working on P2P systems, we can take leverage of this to sync data instead of going through some cloud like iCloud which is also a third party. With P2P computers, phones can connect to each other directly through a secure network protocol.

In terms of multi platforms, I mean we still prioritize platforms with local data first, e.g.

  • desktop app
  • phone app
  • web offline app with local browser storage

@maidh91
Copy link
Contributor Author

maidh91 commented Sep 8, 2024

Hi @afadil

Besides the plan, if we only look at the code, do you see any issue of the code change in this PR?

This code change only makes the code cleaner, any thought? @afadil

@afadil
Copy link
Owner

afadil commented Sep 9, 2024

Yes, I think the code is cleaner. I'll do some testing and merge the PR.
Thanks @maidh91

@afadil afadil merged commit 4a7ab30 into afadil:main Sep 9, 2024
@maidh91 maidh91 deleted the refactor-commands-tauri branch September 9, 2024 22:10
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.

2 participants