Restructure redux state to better reflect component state, validation and repeating groups

[olemartinorg]

Description

Today, app-frontend-react will fetch the layout(s) from the server and store them in the formLayout.layouts redux state almost unchanged. When, for example, a form input is changed, this will update the state in formData, a different redux root state. This is also the case for other state directly tied to a specific component, such as:

• Validation messages - in the formValidations.validations state
• Conditional rendering - rules stored in formDynamics.conditionalRendering and effects stored in formLayout.uiConfig.hiddenFields
• Attachments (for FileUpload and FileUploadWithTag), stored in attachments.attachments
• Tags (for FileUploadWithTag), stored in formLayout.uiConfig.fileUploadersWithTag
• Repeating group rows, stored in formLayout.uiConfig.repeatingGroups

Repeating groups contributes to making this especially difficult to work with, as we have to make sure all state is kept in sync when adding/deleting rows to repeating groups; i.e. that the validation state, attachments, etc. is updated to remove the state belonging to a row that was just deleted, and that subsequent rows gets "moved up" to replace the removed row.

With this architecture it means we have to write a lot of code to move stuff around, which sometimes might be finicky and might lead to bugs and unexpected behaviour (such as FileUpload not being supported inside repeating groups, some validations not working for DatePicker when in repeating groups or some validations not working in combination with conditional rendering when in repeating groups). It also means that implementing fixes for these require lots of code to move things around, which leads to large PRs for seemingly minor features.

I propose to invert the way we use the Redux store to keep state about components, rewriting this to work in a way where all the state tied to a component is stored alongside the component itself.

Additional Information

Redux with nested state and dynamically adding reducers:

• https://github.com/reduxjs/redux-toolkit/issues/259
• https://medium.com/@matheusmm/redux-injecting-reducers-dynamically-e4e193eaa905

Relevant/related issues

• 205

Analysis

A proposed solution is to use nested slices to construct a hierarchy from the layout definition, along with mixing in re-usable reducers for common functionality (such as form data, validations, etc).

An example of such a hierarchical state (somewhat abbreviated, with explaining comments):

{
  formNodeCounter: {
    // This counter should only be incremented, and represents the number of
    // components added during the application run time. The number here is
    // used for the `nodeId` property, which uniquely identifies
    counter: 5,
  },
  form: [
    {
      nodeId: 1,
      id: 'topLevelInput',
      type: 'Input',
      formData: 'hello world', // From a re-usable reducer
      hidden: false, // Computed by re-usable actions/reducers using form dynamics
      dataModelBindings: { simpleBinding: 'TopLevelInput' },
    },
    {
      nodeId: 2,
      id: 'familyMembers',
      type: 'Group',
      maxCount: 99,
      prototype: [
        // This is the 'prototype' for a group, which defines an immutable
        // definition of which components belong to a group. This will be
        // copied into a new row inside 'rows', along with appropriate
        // actions/reducers when a new row is added to the group.
        {
          id: 'firstName',
          type: 'Input',
          dataModelBindings: { simpleBinding: 'FamilyMembers.FirstName' },
          // Additional properties from FormLayout, but no formData
        },
        {
          id: 'lastName',
          type: 'Input',
          dataModelBindings: { simpleBinding: 'FamilyMembers.LastName' },
          // Additional properties from FormLayout, but no formData
        },
        {
          // A nested group, with its prototype definition
          id: 'hobbies',
          type: 'Group',
          maxCount: 99,
          prototype: [{ id: 'hobbyName', type: 'Input' }],
        },
      ],
      rows: [
        // These rows are copies from 'prototype', now represented as actual
        // components in the form (with nodeId, formData, etc). Deleting a row
        // index now deletes all relevant component state with it, and we don't
        // have to 'shift up' subsequent rows.
        [
          { nodeId: 3, id: 'firstName', type: 'Input', formData: 'Kari' },
          { nodeId: 4, id: 'lastName', type: 'Input', formData: 'Nordmann' },
          {
            nodeId: 5,
            id: 'hobbies',
            type: 'Group',
            // Nested groups should also only have their rows here, not their prototype definition
            rows: [
              {
                nodeId: 6,
                id: 'hobbyName',
                type: 'Input',
                formData: 'Volleyball',
              },
              {
                nodeId: 7,
                id: 'hobbyName',
                type: 'Input',
                formData: 'Fishing',
              },
            ],
          },
        ],
        [
          { nodeId: 8, id: 'firstName', type: 'Input', formData: 'Ola' },
          { nodeId: 9, id: 'lastName', type: 'Input', formData: 'Nordmann' },
          {
            nodeId: 10,
            id: 'hobbies',
            type: 'Group',
            rows: [
              {
                nodeId: 11,
                id: 'hobbyName',
                type: 'Input',
                formData: 'Football',
              },
              {
                nodeId: 12,
                id: 'hobbyName',
                type: 'Input',
                formData: 'Sewing',
              },
            ],
          },
        ],
      ],
    },
  ],
}

This solution should also handle multiPage groups, in a way such that the functionality becomes transparent to code using the group state.

The point of nodeId here is to have an internal reference to each component (called node here, as a component can appear multiple times inside a repeating group). Updating the formData for a node can be as easy as dispatching an event with payload { nodeId: 5, newFormData: 'hello world' }. Each node will have its own reducer, and will only reduce actions specifying its own nodeId. Gaps in the sequence of nodeIds are allowed (as rows can be deleted and thus nodes will be removed), and care should be taken to ensure we don't write code that attempts to update/change the nodeId for a given component instance once first constructed.

Alternatives

Libraries like redux-orm might help keep relational data in sync, but it's quite possible it won't work directly with the state we have now.

Conclusion

This might be doable, but we'll need to move forwards with a proof-of-concept before committing to this approach. It WILL be a fundamental change.

[olemartinorg]

Update: This is kind-of being implemented in the layout expressions project. In this project, we're implementing ways to find a nearby component instance in relation to the current component, so implementing it something like that without tooling would have caused considerable headache. As of writing this comment, the layout expressions branch includes tooling for generating such a hierarchical representation of the layout.

TL;DR: Consider the layout expressions project to be blocking this issue - it will quite possibly be easier to implement after that is merged into main.

[olemartinorg]

An update, yet again! :wave: I looked trough the documentation yesterday, and while reading about why we have a normalized redux store (which, in a way, is the problem we're trying to solve here), it turns out one of the challenges that solves is useless re-rendering caused by nested state. It might look like nesting state in redux is hard to get right.

When I looked around for solutions, I found RecoilJS, developed by Meta/Facebook (just like React), and it looks like a more flexible (and at the same time simpler) way to share state in React - I really like the idea. I think we could perhaps build on RecoilJS to achieve this goal. When watching the deep dive video about Recoil, I quickly recognized an overlapping design philosophy with what I've been thinking here. I've wanted to get rid of our way of mutating the component id to make it unique (adding repeating group row indices like component-2-3), and rather introduce an auto-incremented nodeId for each component instance (which is not tied to row indices). That way we could, for example, avoid re-rendering all components inside a repeating group when a row is deleted.

Introducing RecoilJS also requires getting rid of Redux Saga, making all of this more of a major rewrite. Getting rid of sagas sounds like a good thing though, and I'd love to have all events automatically triggered whenever the state they depend on changes - but writing procedural-like code (for sagas) in a reactive environment has its own set of challenges (mainly minimizing re-renders).

For an example of this type of architecture and its problems, see #576. The useProcess hook performs tasks that should, in my opinion, have been written as a saga instead. The problem in #576 was that the process object was passed into the dependency array (React.useEffect(..., [process])), and two properties (taskType and taskId) of that object was read when performing an action. When an entirely different property changes, the process object of course changed, re-running the effect. This caused redux actions to be dispatched when they shouldn't. I'm not sure if RecoilJS really helps us avoid these kind of traps, so us having to be vigilant when writing code like this is a risk. In conclusion, I think RecoilJS might be a simple way to achieve this goal, but it won't be easy.

[olemartinorg]

Also relevant here (if we're smart, we can also solve this while we're at it, if we rewrite to RecoilJS):

• 200

[olemartinorg]

I think it's time to close this issue now. Pretty much everything related to this has been rewritten in v4, and we don't even have Redux there any longer. So the essence of what we wanted here, even if it may not include every smallest part of it, has been resolved now. Closing an epic Epic. 🙏