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 to vue-query #2567

Merged
merged 227 commits into from Aug 25, 2022
Merged

Migrate to vue-query #2567

merged 227 commits into from Aug 25, 2022

Conversation

nicksellen
Copy link
Member

@nicksellen nicksellen commented Jul 23, 2022

Branch deployment URL: https://add-more-vue-query.dev.karrot.world

What does this PR do?

This switches our frontend data layer from vuex to vue-query.

There were a few motivations for this:

  • our vuex data layer had got very complex and abstract to work with
  • pinia has replaced vuex as the recommended store (we're not switching to pinia)
  • vuex has a serious bug in that getters are not being cached getters result doesn't get cached vuejs/vuex#2102 - this caused enormous slowdown and made karrot unusable, there is no fix available
  • our pattern of enriching data caused a lot of re-renders

One of the complexities of the existing data layer is passing values down through many layers of components. We did originally have a philosophy of props in, events out for components, so they would be usable in isolation, but this pushed a lot of abstract complexity to the data layer. The data that the components needed would have to be managed far away from those components in the stores.

So, a new approach!

  • no enriching of data in stores, the data for each object will just be what the server returned
  • instead of enriching the base objects, we have helpers to calculate those kind of values (e.g. isCurrentUser)
  • data is loaded into the components where it is needed via composable functions (aka. hooks)
  • stores are replaced in some cases with lightweight services (which are singleton/cached objects), in other cases queries can be used directly in components

This also has a big impact on testing strategy, that is dealt with over here --> #2536

TODO

This is a slightly daunting list of the modules (src/*/datastore/*.js) to convert over.

  • activities / activities
  • activities / activityPlugin
  • activities / activitySeries
  • activities / activityTypes
  • activities / activityTypeStylesheetPlugin
  • agreements / agreements - CAN BE REMOVED
  • alerts / banners
  • alerts / toasts
  • applications / applications
  • applications / applicationsPlugin
  • authuser / auth
  • authuser / deleteAccount
  • authuser / verifymail
  • base / base
  • base / dependentState
  • base / geo
  • base / i18n
  • base / i18nPlugin
  • base / persistedState
  • base / routeError
  • base / route
  • base / routeMeta - REMOVED
  • base / routerPlugin
  • communityFeed / communityFeed
  • feedback / feedback
  • group / currentGroup
  • group / timezones
  • groupInfo / groups
  • history / history
  • invitations / invitations REMOVED
  • issues / issues
  • issues / issuesPlugin
  • messages / conversations
  • messages / currentThread
  • messages / detail
  • messages / latestMessages
  • notifications / notifications
  • offers / currentOffer
  • places / places
  • places / plugin
  • sidenav / sidenavBoxes
  • status / status
  • subscriptions / fcm
  • topbar / breadcrumbs
  • topbar / loadingprogress
  • topbar / loadingProgressReporter
  • topbar / search
  • unsubscribe / unsubscribe
  • users / users
  • utils / about
  • utils / connectivity
  • utils / helpers
  • utils / presence
  • utils / pwa
  • utils / refresh
  • utils / refreshPlugin

Links to related issues

Checklist

  • added a test, or explain why one is not needed/possible...
  • no unrelated changes
  • asked someone for a code review
  • joined #karrot:matrix.org
  • added an entry to CHANGELOG.md (description, pull request link, username(s))
  • tried out on a mobile device (does everything work as expected on a smaller screen?)

@nicksellen
Copy link
Member Author

This PR is a bit crazy, sorry! It's a bit of a creative exploration. It's a bit like when I rearrange a room, just start pulling all the furniture out, moving things around, there is a crazy chaos for a while, then slowly the new form emerges...

My progress is quite good though. If it was needed to make a more incremental PR, I could extract some parts out to another PR, but I'll carry on with this way for now.

@nicksellen nicksellen merged commit e02d4a5 into master Aug 25, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants