[@mantine/form] Fix incorrect form errors behavior with `form.resorde…

…rListItem` and `form.insertListItem` handlers (mantinedev#3828)
milhamm · Mar 28, 2023 · 004b75a · 004b75a
1 parent d985250
commit 004b75a
Show file tree

Hide file tree

Showing 12 changed files with 286 additions and 24 deletions.
diff --git a/docs/src/docs/form/use-form.mdx b/docs/src/docs/form/use-form.mdx
@@ -100,7 +100,7 @@ form.insertListItem('fruits', { name: 'Orange', available: true }, 1);
 form.removeListItem('fruits', 1);
 
 // Swaps two items of the list at the specified path.
-// If no element exists at the `from` or `to` index, the list doesn't change.
+// You should make sure that there are elements at at the `from` and `to` index.
 form.reorderListItem('fruits', { from: 1, to: 0 });
 ```
 

diff --git a/src/mantine-form/src/clear-list-state/index.ts b/src/mantine-form/src/clear-list-state/index.ts
diff --git a/src/mantine-form/src/lists/change-error-indices.test.ts b/src/mantine-form/src/lists/change-error-indices.test.ts
@@ -0,0 +1,117 @@
+import { changeErrorIndices } from './change-error-indices';
+
+const TEST_ERRORS = {
+  name: 'name-error',
+
+  // single level of nesting
+  'fruits.0.name': 'fruit-error-1',
+  'fruits.0.available': 'fruit-error-2',
+  'fruits.4.available': 'fruit-error-3',
+  'fruits.15.inner.name': 'fruit-error-4',
+  'fruits.15.inner.0.name': 'fruit-error-5',
+
+  // multiple levels of nesting
+  'nested.0.inner.1.name': 'nested-error-1',
+  'nested.0.inner.2.name': 'nested-error-2',
+  'nested.2.inner.2.name': 'keep-nested-error-1',
+  'nested.3.inner.0.name': 'keep-nested-error-2',
+  'nested.5.inner.1.check': 'keep-nested-error-2',
+  'nested.0.inner.2.check': 'nested-error-3',
+  'nested.0.inner.5.check': 'nested-error-4',
+};
+
+describe('@mantine/form/change-error-indices', () => {
+  it('increments error indices', () => {
+    expect(changeErrorIndices('fruits', 4, TEST_ERRORS, 1)).toStrictEqual({
+      name: 'name-error',
+      // Errors with index lower than the given one don't change
+      'fruits.0.name': 'fruit-error-1',
+      'fruits.0.available': 'fruit-error-2',
+      // Increment everything else
+      'fruits.5.available': 'fruit-error-3',
+      'fruits.16.inner.name': 'fruit-error-4',
+      'fruits.16.inner.0.name': 'fruit-error-5',
+      // Ignore non-matching paths
+      'nested.0.inner.1.name': 'nested-error-1',
+      'nested.0.inner.2.name': 'nested-error-2',
+      'nested.2.inner.2.name': 'keep-nested-error-1',
+      'nested.3.inner.0.name': 'keep-nested-error-2',
+      'nested.5.inner.1.check': 'keep-nested-error-2',
+      'nested.0.inner.2.check': 'nested-error-3',
+      'nested.0.inner.5.check': 'nested-error-4',
+    });
+  });
+
+  it('decrements error indices and removes errors for the removed element', () => {
+    expect(changeErrorIndices('fruits', 4, TEST_ERRORS, -1)).toStrictEqual({
+      name: 'name-error',
+      // Errors with index lower than the given one don't change
+      'fruits.0.name': 'fruit-error-1',
+      'fruits.0.available': 'fruit-error-2',
+      // Remove the error with the given index
+      // Decrement everything else
+      'fruits.14.inner.name': 'fruit-error-4',
+      'fruits.14.inner.0.name': 'fruit-error-5',
+      // Ignore non-matching paths
+      'nested.0.inner.1.name': 'nested-error-1',
+      'nested.0.inner.2.name': 'nested-error-2',
+      'nested.2.inner.2.name': 'keep-nested-error-1',
+      'nested.3.inner.0.name': 'keep-nested-error-2',
+      'nested.5.inner.1.check': 'keep-nested-error-2',
+      'nested.0.inner.2.check': 'nested-error-3',
+      'nested.0.inner.5.check': 'nested-error-4',
+    });
+  });
+
+  it('increments deeply nested errors', () => {
+    expect(changeErrorIndices('nested.0.inner', 2, TEST_ERRORS, 1)).toStrictEqual({
+      name: 'name-error',
+      'fruits.0.name': 'fruit-error-1',
+      'fruits.0.available': 'fruit-error-2',
+      'fruits.4.available': 'fruit-error-3',
+      'fruits.15.inner.name': 'fruit-error-4',
+      'fruits.15.inner.0.name': 'fruit-error-5',
+      'nested.0.inner.1.name': 'nested-error-1',
+      'nested.0.inner.3.name': 'nested-error-2',
+      'nested.2.inner.2.name': 'keep-nested-error-1',
+      'nested.3.inner.0.name': 'keep-nested-error-2',
+      'nested.5.inner.1.check': 'keep-nested-error-2',
+      'nested.0.inner.3.check': 'nested-error-3',
+      'nested.0.inner.6.check': 'nested-error-4',
+    });
+  });
+
+  it('decrements deeply nested errors and removes errors for the removed element', () => {
+    expect(changeErrorIndices('nested.0.inner', 2, TEST_ERRORS, -1)).toStrictEqual({
+      name: 'name-error',
+      'fruits.0.name': 'fruit-error-1',
+      'fruits.0.available': 'fruit-error-2',
+      'fruits.4.available': 'fruit-error-3',
+      'fruits.15.inner.name': 'fruit-error-4',
+      'fruits.15.inner.0.name': 'fruit-error-5',
+      'nested.0.inner.1.name': 'nested-error-1',
+      'nested.2.inner.2.name': 'keep-nested-error-1',
+      'nested.3.inner.0.name': 'keep-nested-error-2',
+      'nested.5.inner.1.check': 'keep-nested-error-2',
+      'nested.0.inner.4.check': 'nested-error-4',
+    });
+  });
+
+  describe('returns unchanged object', () => {
+    it('if index is undefined', () => {
+      expect(changeErrorIndices('fruits', undefined, TEST_ERRORS, 1)).toStrictEqual(TEST_ERRORS);
+    });
+
+    it('if path is not a string', () => {
+      expect(changeErrorIndices(1, 1, TEST_ERRORS, 1)).toStrictEqual(TEST_ERRORS);
+    });
+
+    it('if path does not exist', () => {
+      expect(changeErrorIndices('does-not-exist', 1, TEST_ERRORS, 1)).toStrictEqual(TEST_ERRORS);
+    });
+
+    it('if index is bigger than any error index', () => {
+      expect(changeErrorIndices('fruits', 100, TEST_ERRORS, 1)).toStrictEqual(TEST_ERRORS);
+    });
+  });
+});
diff --git a/src/mantine-form/src/lists/change-error-indices.ts b/src/mantine-form/src/lists/change-error-indices.ts
@@ -0,0 +1,59 @@
+import { clearListState } from './clear-list-state';
+
+/**
+ * Gets the part of the key after the path which can be an index
+ */
+function getIndexFromKeyAfterPath(key: string, path: string): number {
+  const split = key.substring(path.length + 1).split('.')[0];
+  return parseInt(split, 10);
+}
+
+/**
+ * Changes the indices of every error that is after the given `index` with the given `change` at the given `path`.
+ * This requires that the errors are in the format of `path.index` and that the index is a number.
+ */
+export function changeErrorIndices<T extends Record<PropertyKey, any>>(
+  path: PropertyKey,
+  index: number,
+  errors: T,
+  change: 1 | -1
+): T {
+  if (index === undefined) {
+    return errors;
+  }
+  const pathString = `${String(path)}`;
+  let clearedErrors = errors;
+  // Remove all errors if the corresponding item was removed
+  if (change === -1) {
+    clearedErrors = clearListState(`${pathString}.${index}`, clearedErrors);
+  }
+
+  const cloned = { ...clearedErrors };
+  const changedKeys = new Set<string>();
+  Object.entries(clearedErrors)
+    .filter(([key]) => {
+      if (!key.startsWith(`${pathString}.`)) {
+        return false;
+      }
+      const currIndex = getIndexFromKeyAfterPath(key, pathString);
+      if (Number.isNaN(currIndex)) {
+        return false;
+      }
+      return currIndex >= index;
+    })
+    .forEach(([key, value]) => {
+      const currIndex = getIndexFromKeyAfterPath(key, pathString);
+
+      const newKey: keyof T = key.replace(
+        `${pathString}.${currIndex}`,
+        `${pathString}.${currIndex + change}`
+      );
+      cloned[newKey] = value;
+      changedKeys.add(newKey);
+      if (!changedKeys.has(key)) {
+        delete cloned[key];
+      }
+    });
+
+  return cloned;
+}
diff --git a/...clear-list-state/clear-list-state.test.ts → ...e-form/src/lists/clear-list-state.test.ts b/...clear-list-state/clear-list-state.test.ts → ...e-form/src/lists/clear-list-state.test.ts
diff --git a/.../src/clear-list-state/clear-list-state.ts → ...antine-form/src/lists/clear-list-state.ts b/.../src/clear-list-state/clear-list-state.ts → ...antine-form/src/lists/clear-list-state.ts
diff --git a/src/mantine-form/src/lists/index.ts b/src/mantine-form/src/lists/index.ts
@@ -0,0 +1,3 @@
+export { clearListState } from './clear-list-state';
+export { changeErrorIndices } from './change-error-indices';
+export { reorderErrors } from './reorder-errors';
diff --git a/src/mantine-form/src/lists/reorder-errors.test.ts b/src/mantine-form/src/lists/reorder-errors.test.ts
@@ -0,0 +1,18 @@
+import { reorderErrors } from './reorder-errors';
+
+describe('@mantine/form/reorder-errors', () => {
+  it('reorders errors at given path', () => {
+    expect(reorderErrors('a', { from: 2, to: 0 }, { 'a.0': true })).toStrictEqual({
+      'a.2': true,
+    });
+    expect(reorderErrors('a', { from: 2, to: 0 }, { 'a.0': true, 'a.2': 'Error' })).toStrictEqual({
+      'a.0': 'Error',
+      'a.2': true,
+    });
+  });
+
+  it('returns unchanged object if path does not exist', () => {
+    const errors = { 'a.0': true };
+    expect(reorderErrors('c', { from: 1, to: 2 }, errors)).toStrictEqual(errors);
+  });
+});
diff --git a/src/mantine-form/src/lists/reorder-errors.ts b/src/mantine-form/src/lists/reorder-errors.ts
@@ -0,0 +1,30 @@
+import { ReorderPayload } from '../types';
+
+export function reorderErrors<T>(path: unknown, { from, to }: ReorderPayload, errors: T): T {
+  const oldKeyStart = `${path}.${from}`;
+  const newKeyStart = `${path}.${to}`;
+
+  const clone = { ...errors };
+  Object.keys(errors).every((key) => {
+    let oldKey;
+    let newKey;
+    if (key.startsWith(oldKeyStart)) {
+      oldKey = key;
+      newKey = key.replace(oldKeyStart, newKeyStart);
+    }
+    if (key.startsWith(newKeyStart)) {
+      oldKey = key.replace(newKeyStart, oldKeyStart);
+      newKey = key;
+    }
+    if (oldKey && newKey) {
+      const value1 = clone[oldKey];
+      const value2 = clone[newKey];
+      value2 === undefined ? delete clone[oldKey] : (clone[oldKey] = value2);
+      value1 === undefined ? delete clone[newKey] : (clone[newKey] = value1);
+      return false;
+    }
+    return true;
+  });
+
+  return clone;
+}
diff --git a/src/mantine-form/src/tests/insertListItem.test.ts b/src/mantine-form/src/tests/insertListItem.test.ts
@@ -37,4 +37,36 @@ describe('@mantine/form/insertListItem', () => {
       a: [{ b: 1 }, { b: 2 }, { b: 3 }, { b: 4 }, { b: 5 }],
     });
   });
+
+  it('updates errors of associated fields when list item is inserted', () => {
+    const hook = renderHook(() =>
+      useForm({
+        initialValues: {
+          name: '',
+          a: [{ b: 1 }, { b: 2 }, { b: 3 }],
+        },
+        initialErrors: {
+          name: 'name-error',
+          'a.0.b': 'error-1',
+          'a.1.b': 'error-2',
+          'a.2.b': 'error-3',
+        },
+      })
+    );
+
+    expect(hook.result.current.errors).toStrictEqual({
+      name: 'name-error',
+      'a.0.b': 'error-1',
+      'a.1.b': 'error-2',
+      'a.2.b': 'error-3',
+    });
+
+    act(() => hook.result.current.insertListItem('a', { b: 3 }, 1));
+    expect(hook.result.current.errors).toStrictEqual({
+      name: 'name-error',
+      'a.0.b': 'error-1',
+      'a.2.b': 'error-2',
+      'a.3.b': 'error-3',
+    });
+  });
 });
diff --git a/src/mantine-form/src/tests/removeListItem.test.ts b/src/mantine-form/src/tests/removeListItem.test.ts
@@ -37,7 +37,7 @@ describe('@mantine/form/removeListItem', () => {
     });
   });
 
-  it('clears errors of associated fields when list item is removed', () => {
+  it('updates errors of associated fields when list item is removed', () => {
     const hook = renderHook(() =>
       useForm({
         initialValues: {
@@ -63,6 +63,8 @@ describe('@mantine/form/removeListItem', () => {
     act(() => hook.result.current.removeListItem('a', 1));
     expect(hook.result.current.errors).toStrictEqual({
       name: 'name-error',
+      'a.0.b': 'error-1',
+      'a.1.b': 'error-3',
     });
   });
 });
diff --git a/src/mantine-form/src/use-form.ts b/src/mantine-form/src/use-form.ts
@@ -1,35 +1,35 @@
-import { useState, useCallback, useRef } from 'react';
+import { useCallback, useRef, useState } from 'react';
 import isEqual from 'fast-deep-equal';
 import { getInputOnChange } from './get-input-on-change';
-import { setPath, reorderPath, insertPath, getPath, removePath } from './paths';
+import { getPath, insertPath, removePath, reorderPath, setPath } from './paths';
 import { filterErrors } from './filter-errors';
-import { validateValues, validateFieldValue, shouldValidateOnChange } from './validate';
+import { shouldValidateOnChange, validateFieldValue, validateValues } from './validate';
 import { getStatus } from './get-status';
-import { clearListState } from './clear-list-state';
+import { changeErrorIndices, clearListState, reorderErrors } from './lists';
 import {
-  UseFormReturnType,
-  UseFormInput,
-  SetErrors,
+  _TransformValues,
   ClearErrors,
+  ClearFieldDirty,
+  ClearFieldError,
+  GetFieldStatus,
+  GetInputProps,
+  GetTransformedValues,
+  InsertListItem,
+  IsValid,
+  OnReset,
+  OnSubmit,
+  RemoveListItem,
+  ReorderListItem,
   Reset,
+  ResetDirty,
+  SetErrors,
   SetFieldError,
   SetFieldValue,
   SetValues,
-  ReorderListItem,
-  RemoveListItem,
-  InsertListItem,
-  ClearFieldError,
+  UseFormInput,
+  UseFormReturnType,
   Validate,
   ValidateField,
-  GetInputProps,
-  OnSubmit,
-  OnReset,
-  GetFieldStatus,
-  ResetDirty,
-  IsValid,
-  _TransformValues,
-  ClearFieldDirty,
-  GetTransformedValues,
 } from './types';
 
 export function useForm<
@@ -142,17 +142,19 @@ export function useForm<
   const reorderListItem: ReorderListItem<Values> = useCallback((path, payload) => {
     clearFieldDirty(path);
     _setValues((current) => reorderPath(path, payload, current));
+    _setErrors((errs) => reorderErrors(path, payload, errs));
   }, []);
 
   const removeListItem: RemoveListItem<Values> = useCallback((path, index) => {
     clearFieldDirty(path);
     _setValues((current) => removePath(path, index, current));
-    _setErrors((errs) => clearListState(path, errs));
+    _setErrors((errs) => changeErrorIndices(path, index, errs, -1));
   }, []);
 
   const insertListItem: InsertListItem<Values> = useCallback((path, item, index) => {
     clearFieldDirty(path);
     _setValues((current) => insertPath(path, item, index, current));
+    _setErrors((errs) => changeErrorIndices(path, index, errs, 1));
   }, []);
 
   const validate: Validate = useCallback(() => {