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

docs(features): update store page #1957

Merged
merged 2 commits into from
Mar 31, 2024

Conversation

pewsheen
Copy link
Contributor

@pewsheen pewsheen commented Mar 13, 2024

What kind of changes does this PR include?

  • New or updated content
    • Update features/store page

Description

@pewsheen pewsheen requested a review from a team as a code owner March 13, 2024 14:11
Copy link

netlify bot commented Mar 13, 2024

Deploy Preview for tauri-docs-starlight ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b45307a
🔍 Latest deploy log https://app.netlify.com/sites/tauri-docs-starlight/deploys/6608c4d91146330009ba9a8e
😎 Deploy Preview https://deploy-preview-1957--tauri-docs-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@vasfvitor
Copy link
Contributor

I feel that it could be explicit that with_store interoperates with the frontend. What do you think?

@pewsheen
Copy link
Contributor Author

I feel that it could be explicit that with_store interoperates with the frontend. What do you think?

Actually I do not know the purpose of with_store. 🤔 I guess it is some kind of builder patent, but its usage has changed over the years.

I was testing the plugin on my Mac and iOS, the behavior seemed different and complicated on mobile.

Mac - The backend can interoperate with the frontend without using with_store. (maybe it's a coincidence?)

iOS - The StoreBuilder without assigning mobile_plugin_builder can't load or save the store to the mobile storage. And the way to make it work on mobile is just like how with_store does.

So I'm starting to wonder if the original purpose of the plugin is to let the backend and frontend share the store by default. And the reason we need to use with_store to interoperate with the frontend is caused by some other reason, like on the iOS?

@FabianLars
Copy link
Member

Mac - The backend can interoperate with the frontend without using with_store. (maybe it's a coincidence?)

It can? How so? Or differently: Do you have a small code example?

iOS - The StoreBuilder without assigning mobile_plugin_builder can't load or save the store to the mobile storage. And the way to make it work on mobile is just like how with_store does.

Yeah, mobile support is quite fresh and was contributed by a community member. The PR even made with_store private (probably by accident?) so the relation between mobile and with_store is still a bit unknown.


I'm also not a fan of with_store in general but back then it looked like it, or something like it would be required (i didn't add it myself). I'd like to rework large parts of the plugin, maybe get it closer to electron-store, maybe we can get rid of with_store then.

@FabianLars
Copy link
Member

okay, looking at with_store right now it indeed doesn't make that much sense to me anymore to let users use it. The only thing we'd have to think about if we want to remove it is the mobile plugin handle it sets for new stores. I'm not a fan of the explicit api for that, maybe a new storecollection::builder api makes sense idk. That's something to discuss in the plugins repo.

If someone agrees with me to remove with_store from the public interface (still makes sense internally ig) then i think we should get rid of it in this PR already.

@pewsheen
Copy link
Contributor Author

okay, looking at with_store right now it indeed doesn't make that much sense to me anymore to let users use it. The only thing we'd have to think about if we want to remove it is the mobile plugin handle it sets for new stores. I'm not a fan of the explicit api for that, maybe a new storecollection::builder api makes sense idk. That's something to discuss in the plugins repo.

If someone agrees with me to remove with_store from the public interface (still makes sense internally ig) then i think we should get rid of it in this PR already.

I think the name with_store is confusing. It's like a "find or create store" function. I would prefer to extract the "find or create" logic and use it in the StoreBuilder and commands, and then we can refactor or remove the with_store.

@FabianLars
Copy link
Member

completely agree

Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

Thanks everyone, LGTM ✅

@dreyfus92 dreyfus92 merged commit 183e93f into tauri-apps:next Mar 31, 2024
7 checks passed
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.

Create Store guide
4 participants