2
\$\begingroup\$

I am designing this nice form handling in React/Redux/Saga and I am quite afraid it might not be optimised. So I decided to ask you what you think :)

First of all, I won't get into details how I normalise the json-s to get to this structure, because this post will get huge, so this is how my redux state is being initialised on FORM/INIT:

form: {
  formName: {
    fields: { //this is were I save my form definition, for rendering purposes
      section1: {
        children: {
          textInput: {
            id: 'textInput',
            name: 'Name',
            type: 'text',
            validations: [
              {
                type: 'required',
                message: 'This field is required!'
              }
            ]
          }
        }
      }
    },
    model: {
      path: 'section1.children.textInput',
      value: 'Some value',
      isTouched: false,
      activeErrorState: {
        hasError: false,
        message: ''
      }
    }
  }
}

Basically, for "speed" (which might not be the case...) I am separating my values/active-error-state/flags from my definitions. Plus, I flatten the model and strip representational types like "section", "group", "formGroup" that are used by the component, that generates the form from the "fields" definition. Plus, I have this "path" key, which in my head helps me browse the "fields" tree faster :) In this example, it is just 3 levels nested, but in my real world, I have sometimes up to 10 levels depth.

So here is what worries me in my code.

This is the saga, that is triggered on every change in my form fields:

function* changeFormField(action) {
  // selects the redux store
  const store = yield select();
  // the payload have the name of the form, the name of the field and the value that need to get changed in the redux store (the model part of my form)
  const { formName, fieldName, value } = action.payload;
  // finding the form's fields definition and model is quite easy:
  const { fields, model } = store.form[formName];
  // I need to search the whole fields three to find the deeply nested field name, so that I can get it's validations definitions:
  const fieldDefinition = findInObjectByString(fields, model[fieldName].path);
  // I have a function that creates validation functions based on the validation definitions
  const validations = getValidations(fieldDefinition.validations);
  // I check if the new value is correct or not
  const runValidationsResult = runValidations(fieldName, value, validations);
  // this line triggers an action that updates the model.
  yield put(
    Actions.updateFormField(
      formName,
      fieldName,
      value,
      runValidationsResult
    )
  );
}

As you can see, I need to go through this findInObjectByString function which browses the "fields" three for the validations definition on every "onChange".

export const findInObjectByString = (nestedObject, string) => {
  // I clone the nested object, because I am afraid of mutations :)
  let clonedNestedObject = deepclone(nestedObject);
  // strip the string just in case
  string = string.replace(/\[(\w+)\]/g, '.$1');
  // remove a possible first '.' just in case
  string = string.replace(/^\./, '');
  // split the string into array of keys to iterate and search the three
  const pathArray = string.split('.');

  // eslint-disable-next-line no-plusplus
  for (let i = 0, n = pathArray.length; i < n; ++i) {
    const k = pathArray[i];
    if (k in clonedNestedObject) {
      clonedNestedObject = clonedNestedObject[k];
    } else {
      return;
    }
  }
  // eslint-disable-next-line consistent-return
  return clonedNestedObject;
};

And I also need to use it one more time (get the field's type), when I flatten the object, so I can send it to backend:

export const flattenFormModel = (formObject) => {
  const { fields, model } = formObject;

  // this is a reduce function that takes a flat model and flattens it even more :D because BE has specific needs as it comes to how it takes the values, so I need to make some small modifications to my model and send it away:
  return Object.keys(model).reduce((acc, cur) => {
    let currentValue = model[cur].value;
    const fieldDefinition = findInObjectByString(fields, model[cur].path);

    // BE specific things, is why this is needed :)
    if (fieldDefinition.type === FIELD_TYPES.DROPDOWN && currentValue) {
      currentValue = currentValue.value;
    }

    // BE specific things, is why this is needed :)
    if (fieldDefinition.type === FIELD_TYPES.DYNAMIC && currentValue) {
      currentValue = currentValue.map((item) => {
        if (item.newField) {
          delete item.newField;
          return {
            ...item,
            id: 0
          };
        }
        return item;
      });
    }

    return {
      ...acc,
      [cur]: currentValue
    };
  }, {});
};

Now in my gut, I have the feeling that this is not a great approach. I find it too much three search (even though it is a hash three, I access it by key). I was thinking of putting validations and type in the model as well, but doesn't that means that there is no need for the fields/model separation?

One thing that needs to be considered however is that the BE is completely unaware of my form's structure. It sends me initial model like flat structure: [{name: value}, {name: value}].

Please help a fellow developer become a better code writer :)

10x a lot!

\$\endgroup\$
0
2
\$\begingroup\$

The reason why you have to search in the way you currently are is because your redux store is so deeply nested. Flatten it and it will make things a lot easier. Have all text inputs, regardless of which form or section it belongs to, in a data structure with its own unique id.

The textfield doesn't need to "know" which form or section it belongs to. You make that the job of another aspect of your store. I prefer JavaScript Maps, but you are more than welcome to use objects.

Create keys dedicated to keeping track of which sections contain which form, and what textfields belong to which section.

When you need to access inputs, all you need is to grab the id of the textfield directly

Something along the lines of this. Model only needs to know the id. Not the object nesting structure.

{

forms: new Map(
    [
        'form1',
        ['section1', 'section2' //...]
    ]
)

fields: new Map(
    [
        'section1',
        ['textInput', 'otherTextInput']
    ]
)

textInputs: new Map(
    [
        'textInput',
        {
            id: 'textInput',
            name: 'Name',
            type: 'text',
            validations: [
              {
                type: 'required',
                message: 'This field is required!'
              }
            ]
        }
    ],
    [
        'otherTextInput',
        {
            id: 'otherTextInput',
            name: 'Name',
            type: 'text',
            validations: [
              {
                type: 'required',
                message: 'This field is required!'
              }
            ]
        }
    ]
)

model: {
    path: 'textInput',
    value: 'Some value',
    isTouched: false,
    activeErrorState: {
      hasError: false,
      message: ''
    }
  }
}
```
\$\endgroup\$
2
  • \$\begingroup\$ Hi Andrew! Your answer is great, however I try to keep my state this nested and not flat, because this is the payload that the Backend needs. So it is quite easy for me to just take the redux state and POST it to the API \$\endgroup\$ – Chris Panayotova Nov 7 '19 at 10:18
  • \$\begingroup\$ @ChrisPanayotova Your backend code should not affect the structure of your frontend. They are completely separate entities and you tangle up one in order to perfectly accommodate for the other. Your redux store should be written in terms of ease of use for your frontend. Write an algorithm to structure your redux data to match what the backend needs before the api call is made. \$\endgroup\$ – Andrew Nov 7 '19 at 22:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.