knowledgevis / miqa-annotate

A Medical Imaging Annotation Tool
https://annotate.knowledgevis.com
Apache License 2.0
2 stars 0 forks source link

Remove `direct-vuex` #8

Closed davidshq closed 1 year ago

davidshq commented 1 year ago

One of the changes I made to MIQA within the last ~2 months was removing direct-vuex - I'd like to push this change over but wanted to open up this issue for brief discussion in case anyone else things differently.

My reasoning is as follows:

  1. It adds complexity to the application
  2. It breaks with Vue/Vuex conventions, e.g. normally one would use something like:

    • store.dispatch("someAction", somePayload)

    Whereas direct-vuex uses something like the following:

    • store.dispatch.someAction(somePayload)
  3. Another example is when using Vuex in components. Normally one might do something like:

    ...mapState([
      'reviewMode',
      'experiments'
    ])
    
    // ...
    const reviewMode = this.reviewMode

    Whereas with direct-vuex one can skip this step and use the store directly, e.g.

    store.state.reviewMode
    1. If direct-vuex was used consistently throughout the code base I'd be more inclined to continue utilizing direct-vuex but the code base is currently a mix of the two and we should choose one or the other.
    2. The library hasn't been updated in ~2 years and doesn't appear to be widely utilized by the Vue community
    3. There is limited documentation on the library
    4. More recent versions of Vuex and Pinia offer better built-in TypeScript support and thus removal of the library is likely eventually anyways.
    5. Getting assistance from the Vue community is more difficult than necessary because the number of folks familiar with direct-vuex is quite limited.

That said we do lose something by moving off the library:

  1. The controversial advantage of an OOP syntax for interacting with the store
  2. The more concrete advantage of better type checking by TS of the store and of better IDE hints

Over the course of the last six months or so I've found the various problems that direct-vuex has caused me much more time consuming than any issues arising from removing direct-vuex.

Thoughts?

annehaley commented 1 year ago

I'm inclined to agree, especially since we don't use it consistently. You make good points, so you have my support on this.

davidshq commented 1 year ago

Great! I already discussed this with Curt when I initially made the changes and he was good with them, so I'll close this out and get a PR ready.