farmOS / field-kit

A modular, offline-first companion app to farmOS.
https://farmOS.org
GNU General Public License v3.0
62 stars 39 forks source link

Replace Vuex & custom events with provide/inject API #485

Closed jgaehring closed 2 years ago

jgaehring commented 2 years ago

As I'm upgrading to Vue 3 (#420), I'm realizing it would be far better to use provide/inject for passing entities from the store down to field modules, rather than passing them down as props as we're doing now. This would allow field module components to declare more precisely where and when they wish to access those entities, w/o the need for that kind "prop dirlling".

With Vue 3, provide/inject is available in both the Options API and the Composition API. The docs for using the Composition API's provide/inject also suggests using the readonly function with provide, to prevent mutations by the injecting component, although I believe this could be used with the Options API as well.

This might be good to use in tandem with whatever alternative to the loadEntity action we may consider in the future, as I descibed in https://github.com/farmOS/farmOS-client/issues/274#issuecomment-1008008036:

One idea is to shift away from this "loading/syncing" paradigm towards more of a "checkout" or "read/write" paradigm, where a module requests access to write to an entity and must manually relinquish that access before the changes are synced.

jgaehring commented 2 years ago

I experimented with this a bit while working on updating the HomeWidgets component to work with Vue 3. Ultimately I realized the problem I was facing with props getting passed down to the widget components was due to the fact that props are now unnested in the Vue 3 Render Function API, but in the course of diagnosing this I tried using a provider in place of props, and it worked exceedingly well.

In the injecting child component all that was necessary was to change

{
  props: ['logs', 'user'],
}

to

{
  inject: ['logs', 'user'],
}

And for the providing parent component I created a little Vuex helper, similar to mapState, to facilitate providing many pieces of state w/o tons of boilerplate:

https://github.com/farmOS/farmOS-client/blob/cebb0e14852bba5b51522943265d26f76551d1e3/src/core/store/index.js#L31-L46

This is all to say I really like this idea, but ultimately held off on committing it to my vue-3 migration branch b/c there was still the issue of how to filter logs, assets, etc for the individual widgets, but that is problem unique to the HomeWidgets component and should not be an issue for the main data passed down Field Modules. (On that note, I think the widgets could have their own filtered portions of the state passed down by HomeWidgets itself, b/c as far as I can tell an injecting component ascends the tree of components until it finds the first parent with a matching inject key, but I'll have to experiment to be sure.)

The other nice aspect I learned is that Vue actually encourages the use of provider methods to accompany the provider state that is injected into child components, if those child components need to modify it in any way:

However, there are times where we need to update the data inside of the component where the data is injected. In this scenario, we recommend providing a method that is responsible for mutating the reactive property.

This could be the perfect way of implementing the "writer" half of the read/write paradigm I outlined in #274 and quoted in the previous comment above. It would also obviate the need for the mixins I'm currently using to provide those updateEntity methods, which should help avoid potential collisions if, say, a field module wishes to define its own updateLogs method.

So yea, a very promising approach indeed!

jgaehring commented 2 years ago

The other nice aspect I learned is that Vue actually encourages the use of provider methods to accompany the provider state that is injected into child components, if those child components need to modify it in any way:

However, there are times where we need to update the data inside of the component where the data is injected. In this scenario, we recommend providing a method that is responsible for mutating the reactive property.

This could be the perfect way of implementing the "writer" half of the read/write paradigm I outlined in #274 and quoted in the previous comment above. It would also obviate the need for the mixins I'm currently using to provide those updateEntity methods, which should help avoid potential collisions if, say, a field module wishes to define its own updateLogs method.

With regard to that I'm renaming this issue to cover entity methods as well.

jgaehring commented 2 years ago

I'm going to rename and shift the focus of this issue a bit, since I'm now intending to use a global window.farm object to expose the checkout/revise/commit methods for working with entities specifically. However, these ideas may still be applicable to other pieces of state, mutations and actions in the Vuex store which are being rather needlessly passed down to every component as props.

The best example of this is probably the alert() mutation and the corresponding errors state. Both the state and the method could go into a Provider, using the strategies mentioned above, and this could alleviate the need for Vuex entirely.

There are also things like the configDocuments Vuex module which really has no business being in the store, since the only thing being exposed to components are the entity types, which is another prime use case for provide/inject.

Lastly, there are the custom event listeners in AppShell that probably don't need to be events, but could instead be a provider. They're really very similar to alert()/errors in that respect. Having all that co-located within the AppShell makes a lot of sense, since the shell has responsibility for parts of the UI that correspond to that state and behavior.

It is probably best to just start with Vue's ref(), reactive() and computed() functions to create reactive state that can be injected by components as they wish, but there is also a new-ish library called Pinia, which is actually slated to be the successor to Vuex, and is a lot more lightweight alternative, should the need arise down the road:

https://pinia.vuejs.org/introduction.html

jgaehring commented 2 years ago

Resolved by b5b4316.