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

feat(store)!: fully rework and add auto save #1550

Merged
merged 46 commits into from
Oct 1, 2024

Conversation

Legend-Master
Copy link
Contributor

@Legend-Master Legend-Master commented Jul 15, 2024

Closes #1535

Rework and add a setting to enable a store to debounce save on modification (on calls like set, clear, delete, reset)

@Legend-Master
Copy link
Contributor Author

To be honest, I wonder why with_store works in this way, I'm thinking it should be creating a Resource and with that we can store the configs in Rust side so that we don't pass in path and auto_save in every call

@Legend-Master Legend-Master marked this pull request as ready for review July 15, 2024 05:24
@Legend-Master Legend-Master requested a review from a team as a code owner July 15, 2024 05:24
@FabianLars
Copy link
Member

To be honest, I wonder why with_store works in this way, I'm thinking it should be creating a Resource and with that we can store the configs in Rust side so that we don't pass in path and auto_save in every call

  1. with_store is older than the Resource api
  2. Feel free to rewrite the whole impl/api 😂 (i'm serious, the plugin probably deserves it having requests like [store] Question: Why not keep the store cache in the WebView process? #15 in mind)

@Legend-Master
Copy link
Contributor Author

Legend-Master commented Jul 15, 2024

with_store is older than the Resource api

😮 makes sense then, I could probably do it in this PR maybe another PR

I don't think we can do synchronous commands right now with Tauri's IPC? The way we currently do thing is having something in the Rust side and give js side the ability to interact with it but things will stay in the Rust side, #15 means we either need the ability to do synchronous IPC or we reverse the process and make rust side request data from the js side

@FabianLars
Copy link
Member

I don't think we can do synchronous commands right now with Tauri's IPC?

Nope, and i doubt we'll see this any time soon.

The way we currently do thing is having something in the Rust side and give js side the ability to interact with it but things will stay in the Rust side, #15 means we either need the ability to do synchronous IPC or we reverse the process and make rust side request data from the js side

I mean, it's totally fine to not want to "solve" #15 i just think we never thought much about it.

@Legend-Master Legend-Master changed the title feat(store): add auto save feat!(store): add auto save Jul 17, 2024
@Legend-Master Legend-Master changed the title feat!(store): fully rework and add auto save feat(store)!: fully rework and add auto save Sep 8, 2024
fn drop(&mut self) {
let auto_save_debounce_sender = self.auto_save_debounce_sender.lock().unwrap();
if let Some(ref sender) = *auto_save_debounce_sender {
let _ = sender.send(AutoSaveMessage::Cancel);
Copy link
Member

Choose a reason for hiding this comment

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

are you sure we want to cancel on drop? shouldn't we force save here instead?

Copy link
Contributor Author

@Legend-Master Legend-Master Oct 2, 2024

Choose a reason for hiding this comment

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

I forget why I did this too be honest, we probably should move this to save instead, not sure we really need to do anything on drop

Copy link
Member

@lucasfernog lucasfernog left a comment

Choose a reason for hiding this comment

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

we should also add a JS option to configure the debounce delay, but i'll take this

@lucasfernog
Copy link
Member

really good work btw

lucasfernog added a commit to tauri-apps/tauri-docs that referenced this pull request Oct 1, 2024
@lucasfernog lucasfernog merged commit f12d356 into tauri-apps:v2 Oct 1, 2024
19 checks passed
@Legend-Master
Copy link
Contributor Author

We probably need a way to create named stores to allow them to be shared easier, kinda like how the tray icons work, also still need to think about how should save on exit work since we can now having multiple different store settings

@lucasfernog What do you think about these? Currently we're doing nothing on exit, this is different from what we used to do, also do you think we should just prevent duplicated stores with the same path or maybe add a function to share a store with a name?

@lucasfernog
Copy link
Member

do you think we should just prevent duplicated stores with the same path

yes I think that's reasonable

@Legend-Master
Copy link
Contributor Author

I'll need to think about this a bit more on how to do it, I will be out for a few hours, can get back on is when I'm back

gezihuzi pushed a commit to Hypobenthos/plugins-workspace that referenced this pull request Oct 3, 2024
* Add auto save to store plugin

* Put jsdoc at constructor instead of class level

* Clippy

* Use enum instead of bool

* Some(AutoSaveMessage::Cancel) | None

* from_millis

* u64

* Add change file

* Rename to emit_on_change

* should use Duration in `with_store`

* Add breaking change notice to change file

* Emit change event for inserts by reset

* Update readme example

* Update example

* Remove extra line

* Make description clear it only works with managed

* Fix links in docstring

* Fix doc string closing

* get_mut

* Proof of concept

* fmt

* Load store on create

* cargo fmt

* Fix merge conflits

* Format

* small cleanup

* update docs, use `impl Into<JsonValue>`

* fix doctests, further simplification of api

* add store options

---------

Co-authored-by: Tillmann <28728469+tweidinger@users.noreply.github.com>
Co-authored-by: Lucas Nogueira <lucas@tauri.app>
Sir-Thom pushed a commit to Sir-Thom/plugins-workspace that referenced this pull request Oct 22, 2024
* Add auto save to store plugin

* Put jsdoc at constructor instead of class level

* Clippy

* Use enum instead of bool

* Some(AutoSaveMessage::Cancel) | None

* from_millis

* u64

* Add change file

* Rename to emit_on_change

* should use Duration in `with_store`

* Add breaking change notice to change file

* Emit change event for inserts by reset

* Update readme example

* Update example

* Remove extra line

* Make description clear it only works with managed

* Fix links in docstring

* Fix doc string closing

* get_mut

* Proof of concept

* fmt

* Load store on create

* cargo fmt

* Fix merge conflits

* Format

* small cleanup

* update docs, use `impl Into<JsonValue>`

* fix doctests, further simplification of api

* add store options

---------

Co-authored-by: Tillmann <28728469+tweidinger@users.noreply.github.com>
Co-authored-by: Lucas Nogueira <lucas@tauri.app>
@HuakunShen
Copy link

Where is the file stored by config plugin?
I couldn't find it under appDataDir()

@Legend-Master
Copy link
Contributor Author

Not sure what do you mean by config plugin, if you mean the store plugin, you can see the path used with resolve_store_path, it should be under appDataDir if your store's path is a relative path or if it's an absolute path then it will just use that path

@HuakunShen
Copy link

Found it, thanks! @Legend-Master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[store] support auto save with debounce
6 participants