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

Tracking Issue: Remove use of Vuex in favour of composables #11722

Open
8 of 14 tasks
rtibbles opened this issue Jan 12, 2024 · 3 comments
Open
8 of 14 tasks

Tracking Issue: Remove use of Vuex in favour of composables #11722

rtibbles opened this issue Jan 12, 2024 · 3 comments
Assignees

Comments

@rtibbles
Copy link
Member

rtibbles commented Jan 12, 2024

Overview

Our usage of Vuex for state management has caused some additional complexity, often simply due to the way that we have used it using shared global state that can be independently updated by different code paths.

Of more concern is the fact that our use of Vuex in the core of the app creates an implicit API that it becomes hard to reason about, and difficult to alert to end users when it is no longer available. This is because to access Vuex state, getters, or actions, they are referenced in components using e.g.: mapGetters(['isAdmin', 'isLearner']) - so if we were to rename these, update them, or remove them, code relying on them would would very little indication that something had gone awry.

Description and outcomes

It is desirable to remove our usage of Vuex as we update and make changes to our code base.

However, because of the extent of our usage of Vuex, this should be done incrementally.

As the primary concerns are removal of an implicit API, and mutations to shared state across different pages within apps, these should be addressed first - so removal of the core Vuex state, getters, mutations, and actions is the first focus of this tracking issue. Individual plugins can then be independently responsible for registering Vuex and instantiating the store, and can take their own path to removing dependence on vuex.

Core Vuex API

Preview Give feedback
  1. DEV: frontend help wanted
    Wck-iipi
  2. DEV: frontend TAG: tech update / debt help wanted
    nathanaelg16
  3. 1 of 1
    DEV: frontend TAG: tech update / debt help wanted
    iamshobhraj
  4. DEV: frontend TAG: tech update / debt help wanted
    nathanaelg16
  5. DEV: frontend TAG: tech update / debt help wanted
    nathanaelg16
  6. rtibbles
  7. DEV: frontend TAG: tech update / debt
    ozer550
  8. DEV: frontend help wanted
    nathanaelg16
  9. DEV: frontend TAG: tech update / debt help wanted
    iamshobhraj
@rtibbles rtibbles added this to the upcoming major milestone Jan 12, 2024
@rtibbles rtibbles self-assigned this Jan 12, 2024
@nick2432
Copy link
Contributor

can i try this one?

@MisRob
Copy link
Member

MisRob commented Jan 18, 2024

Hi @nick2432, thank you for volunteering. This is already assigned to @rtibbles. I recommend searching for "help wanted" issues.

@nucleogenesis
Copy link
Member

It may be worth looking at the changes to how the promises are handled in CoachToolsModule in #12755 - the issue that I ran into there was in part related to the fact that the handleApiError action ends with throw error - which led me to run into weird UncaughtError issues when the handleApiError was dispatched within the beforeRouteEnter guard.

When I removed that throw error line, however, the issue was resolved.

But, there are too many uses of the handleApiError action in Kolibri for me to make that change to fix that particular problem, but during work on this issue it'd be worth considering whether or not we need or want it to throw the error back at us.

FWIW - when I removed that throw error the tests passed... but that doesn't mean we've covered error-throwing cases in particular though so it'll require some manual review in service of this issue.

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

No branches or pull requests

4 participants