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

Experiment: Storing the data model in a Context #1175

Closed
Tracked by #339 ...
olemartinorg opened this issue May 22, 2023 · 0 comments · Fixed by #1712
Closed
Tracked by #339 ...

Experiment: Storing the data model in a Context #1175

olemartinorg opened this issue May 22, 2023 · 0 comments · Fixed by #1712
Assignees
Labels
area/data-storage fe-v4 Issues to be solved before v4 goes gold feature-complete Features needed for parity with Altinn 2 (target: June 2023) quality/debt

Comments

@olemartinorg
Copy link
Contributor

This issue describes an approach to restructure our data model (in-memory) storage for app-frontend, extracted from #345.

Background

Currently, we're using Redux and some complex sagas to manipulate the data model, and we're storing everything in dot-notation internally. We do this because Redux has trouble with deeper stuctures, and I trust that former developers has gone down that route and landed on Redux and dot-notation for a reason. It even says this is a good thing in our docs.

Right now, this structure has multiple downsides, mainly:

  • Empty objects cannot exist in dot-notation. This has lead to complex state management for adding a new row to a repeating group (because until you fill out some data in that row, it is essentially an empty object). We cheat by increasing the index property on our repeating group state, without ever letting the server know that a new row was added. Changing this, as proposed in Validation trigger on group should re-validate on delete + new row #555, would be a breaking change.
  • With empty object being deleted by us, we actually change the data model and send our changes back to the server with empty objects removed. This has caused problems in the past for app-developers who wants to initialize empty objects in their data structure first, and use them in later in requests.
  • Without proper/good tools, and transparency in code making smells very visible, we've fallen into a routine where state management has to be done using dot notation. That means, if you want to add/remove items in an array, you're in a world of pain (where you have to unset keys in the dot-notation object and 'move up' later values in the array). Proper array tools would have made this much easier for us, and would have made Checkbox component should support mapping to an array #273 a breeze to implement (instead of being in the backlog for years).
  • With complex state handling comes complex workarounds for seemingly simple problems. In Possible to configure components to save data onChange (debounced)  #155, I made a useDelayedSaveState hook that debounces input from the user so that we wait a while before storing the change in Redux (and subsequently calling the server). In hindsight, our data model storage should more intelligently handle multiple changes in rapid succession, and wait to save the data on the server when our state has stabilized. Multiple/rapid updates to the data model currently works, but has caused multiple problems (see one of the outstanding issues in List component: remaining issues #925 and the bug reported in Timing issue regarding updating the data model (Stateless) #989).

Proposed solution

Instead of swapping out the entire state manager (as it is sort-of proposed in #345), this issue suggests creating a React Context for the data model storage, and provide tools to manipulate and fetch data in that context. This means we could still have reducers and complex tasks that would happen as the data model updates, but by implementing this ourselves we get better creative control over how this is handled.

A few requirements for this context should be:

  • It should expose tools tailored to our needs, such as a hook that binds to a data model binding (or multiple?) and gives you functions to update the state.
  • The data model itself should be stored as an object in memory, not using dot-notation
  • It should preferably handle very rapid state changes, such as key-presses, so that we can remove useDelayedSavedState altogether
  • Saving the actual data model should only happen after the state has stabilized
  • It should be possible to map to an array, and use both low-level hooks and high-level hooks that allow for smooth and easy manipulation of state.
  • The interface for these hooks should preferably be backwards-compatible, or otherwise be made in a way that allows us to postpone the activation of this feature until v4, since some of it breaks backwards compatibility (see Validation trigger on group should re-validate on delete + new row #555).
@olemartinorg olemartinorg moved this to 👷 In Progress in Team Apps May 22, 2023
@olemartinorg olemartinorg self-assigned this May 22, 2023
@RonnyB71 RonnyB71 added the feature-complete Features needed for parity with Altinn 2 (target: June 2023) label May 30, 2023
@HanneLauritsen1967 HanneLauritsen1967 moved this from 👷 In Progress to 📈 Todo in Team Apps Jun 14, 2023
@olemartinorg olemartinorg moved this from 📈 Todo to 👷 In Progress in Team Apps Aug 28, 2023
olemartinorg added a commit that referenced this issue Aug 29, 2023
Co-authored-by: Ole Martin Handeland <git@olemartin.org>
@olemartinorg olemartinorg moved this from 👷 In Progress to 📈 Todo in Team Apps Sep 7, 2023
@olemartinorg olemartinorg added the fe-v4 Issues to be solved before v4 goes gold label Sep 25, 2023
@olemartinorg olemartinorg moved this from 📈 Todo to ⚠️ Blocked in Team Apps Oct 10, 2023
@RonnyB71 RonnyB71 mentioned this issue Nov 22, 2023
Closed
@olemartinorg olemartinorg moved this from ⚠️ Blocked to 👷 In Progress in Team Apps Nov 28, 2023
@olemartinorg olemartinorg moved this from 👷 In Progress to 🧪 Test in Team Apps Jan 2, 2024
@RonnyB71 RonnyB71 moved this from 🧪 Test to ✅ Done in Team Apps Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/data-storage fe-v4 Issues to be solved before v4 goes gold feature-complete Features needed for parity with Altinn 2 (target: June 2023) quality/debt
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants