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

Migrate from vuex store to pinia #10210

Closed
11 tasks done
JammingBen opened this issue Dec 19, 2023 · 7 comments · Fixed by #10349 or #10372
Closed
11 tasks done

Migrate from vuex store to pinia #10210

JammingBen opened this issue Dec 19, 2023 · 7 comments · Fixed by #10349 or #10372
Assignees
Labels
Category:Technical Technical ehancements

Comments

@JammingBen
Copy link
Contributor

JammingBen commented Dec 19, 2023

Description

In the long-term we want to migrate from vuex to pinia since it gives us e.g. better typing and makes unit testing easier. Also, pinia is the official recommended way for Vue store implementations.

There's a long way to go, meaning this is an ongoing process and won't be done via one single PR. Stores that need to be refactored:

Stores to migrate

To clarify

  • In general: how much buisness logic do we want to have inside our store composables? Depends on the store. For now we keep it mostly the same as before to ease the migration process.
  • How do we want to organize the file store? Some parts like e.g. the current folder shouldn't live exclusively in the files app. Some other things though are only needed in the files app. Split it up into 2 stores, one living in web-pkg, one in the files app? See PR description here: refactor: moves resources to pinia store #10368
  • Split the files store in files and shares?
  • What parts from the navigation store are still needed? Looks a bit like old/deprecated extension code to me 🤷 -> removed the deprecated stuff
  • Do we want to keep storing left nav items in a separated store, or make it part of the extension system? -> handle via extension registry
  • Do we want the integrate our message queue in the store again? -> yes!
  • The config store as well as the configuration manager currently share logic such as defining defaults. There should be only one place to do that, which means we need to discuss responsibilities here. -> one composable to rule them all!
@JammingBen JammingBen added the Category:Technical Technical ehancements label Dec 19, 2023
@JammingBen JammingBen moved this to Refactor / Technical Debt in Infinite Scale Team Board Dec 19, 2023
@dschmidt
Copy link
Member

We could try to identify stores with the least dependencies on other stores to determine a reasonable order. I remember when we tried to port ancestorMetadata it was basically impossible to do that in isolation because of dependencies....

@JammingBen
Copy link
Contributor Author

We could try to identify stores with the least dependencies on other stores to determine a reasonable order. I remember when we tried to port ancestorMetadata it was basically impossible to do that in isolation because of dependencies....

Good idea! I'd probably start with the "easy" ones such as messages, user, config etc. and then go to the more complex ones like files, that also have dependencies on other stores.

@kulmann
Copy link
Member

kulmann commented Dec 19, 2023

I don't agree that we should port all of the stores to pinia. ;-) E.g. for the modals I never understood why we have a store module for that... would expect that to use the event bus instead 🙈

@JammingBen
Copy link
Contributor Author

I don't agree that we should port all of the stores to pinia. ;-) E.g. for the modals I never understood why we have a store module for that... would expect that to use the event bus instead 🙈

What is the benefit of using an event bus for modals? 🤔 I like pinia as well here, it provides a convenient and established way to handle the state of the modals and is easy to test.

@kulmann
Copy link
Member

kulmann commented Dec 19, 2023

I don't agree that we should port all of the stores to pinia. ;-) E.g. for the modals I never understood why we have a store module for that... would expect that to use the event bus instead 🙈

What is the benefit of using an event bus for modals? 🤔

I just think that modals and notification messages don't need a global state. For both (modal and notification message) we have a component which is responsible for all the handling and rendering. So I don't see a need to leak the state of modals and notification messages to the outside of the respective component.

@dschmidt
Copy link
Member

Well,

I don't agree that we should port all of the stores to pinia. ;-) E.g. for the modals I never understood why we have a store module for that... would expect that to use the event bus instead 🙈

What is the benefit of using an event bus for modals? 🤔

I just think that modals and notification messages don't need a global state. For both (modal and notification message) we have a component which is responsible for all the handling and rendering. So I don't see a need to leak the state of modals and notification messages to the outside of the respective component.

Yes, and the point is: we need some sort of state tracking (in the component or in a store) and that's exactly what a store makes easy. With the before mentioned benefit of making tests easier.

I like @JammingBen 's current implementation and I see it becoming only more convoluted when getting rid of pinia and using the eventbus instead.

@dschmidt
Copy link
Member

i would consider it to be a model view separation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Technical Technical ehancements
Projects
Archived in project
3 participants