learningequality / learningequality/studio

Stop using Vuex file module

Open
#3,466 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

DEV: frontend TAG: performance
Dominant language
Python
Stars
191
Forks
307
Avg merge
5d 6h
Merged PRs (30d)
10

Description

Summary

When users work with files (e.g. resources upload, thumbnails preview, ...), we use the Vuex file module

https://github.com/learningequality/studio/blob/f1534d7bc6b2ded87f984e2a44df80bc4e1d3be4/contentcuration/contentcuration/frontend/shared/vuex/file/index.js#L6-L34

The goal of this issue is to remove this module in favor of more local state that will be cleared when appropriate.

The majority of this work has been done in https://github.com/learningequality/studio/pull/3427 (related issue https://github.com/learningequality/studio/issues/3364) where the file module has been removed in favour of new composables (useFileUpload, useContentNodesFiles, ...). However, we can't use them as they are because currently, they reach directly to IndexedDB for data since during that work, we wanted to experiment with such an approach. However, it didn't improve performance the way we expected and performance results were even worst. Because of that, we decided not to reach directly to IndexedDB as originally planned and still keep the state similarly to our current architecture around Vuex, but it should be rather local and data should not accumulate. The code of composables needs to be updated to reflect this decision and all updates done in that PR need to be properly tested.

Background

  • We accumulate files data in Vuex file/state from various Studio features and there is no mechanism for clearing them which causes memory leaks. This issue is part of a larger group of issues (see
    https://github.com/learningequality/studio/issues/3363) that aim to refactor problematic features away from using Vuex global state towards private in-components state or state that’s shared between more components but is cleared at some point and optimized performance-wise in general.

Acceptance criteria

  • Vuex file module exists no more. Data are handled more locally and do not accumulate.
  • The new implementation avoids the pitfalls of our current implementation that are described in the “Background” section and is optimized performance-wise overall

Blocked by

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with contentcuration/contentcuration/frontend/shared/vuex/file/index.js and review the useFileUpload and useContentNodesFiles composables introduced in pull request 3427. Check issue 3468 before starting, since this work is blocked. Done means the Vuex file module is removed, file data is handled locally without accumulation, and the related changes are properly tested.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend, performance
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.