Safer way to select older data while re-generating parts of the node …

…tree (#2715) Co-authored-by: Ole Martin Handeland <git@olemartin.org>
Altinn · Nov 14, 2024 · 278177e · 278177e
1 parent 26909ac
commit 278177e
Show file tree

Hide file tree

Showing 11 changed files with 288 additions and 228 deletions.
diff --git a/src/features/formData/FormDataWrite.tsx b/src/features/formData/FormDataWrite.tsx
@@ -540,6 +540,23 @@ const useWaitForSave = () => {
   );
 };
 
+function getFreshNumRows(state: FormDataContext, reference: IDataModelReference | undefined) {
+  if (!reference) {
+    return 0;
+  }
+
+  const model = state.dataModels[reference.dataType];
+  if (!model) {
+    return 0;
+  }
+  const rawRows = dot.pick(reference.field, model.currentData);
+  if (!Array.isArray(rawRows) || !rawRows.length) {
+    return 0;
+  }
+
+  return rawRows.length;
+}
+
 const emptyObject = {};
 const emptyArray = [];
 
@@ -832,23 +849,15 @@ export const FD = {
    * Returns the number of rows in a repeating group. This will always be 'fresh', meaning it will update immediately
    * when a new row is added/removed.
    */
-  useFreshNumRows: (reference: IDataModelReference | undefined): number =>
-    useMemoSelector((s) => {
-      if (!reference) {
-        return 0;
-      }
+  useFreshNumRows: (ref: IDataModelReference | undefined) => useMemoSelector((s) => getFreshNumRows(s, ref)),
 
-      const model = s.dataModels[reference.dataType];
-      if (!model) {
-        return 0;
-      }
-      const rawRows = dot.pick(reference.field, model.currentData);
-      if (!Array.isArray(rawRows) || !rawRows.length) {
-        return 0;
-      }
-
-      return rawRows.length;
-    }),
+  /**
+   * Same as the above, but returns a non-reactive function you can call to check the number of rows.
+   */
+  useGetFreshNumRows: (): ((reference: IDataModelReference | undefined) => number) => {
+    const store = useStore();
+    return useCallback((reference) => getFreshNumRows(store.getState(), reference), [store]);
+  },
 
   /**
    * Get the UUID of a row in a repeating group. This will always be 'fresh', meaning it will update immediately when

diff --git a/src/layout/RepeatingGroup/Providers/RepeatingGroupContext.tsx b/src/layout/RepeatingGroup/Providers/RepeatingGroupContext.tsx
@@ -1,5 +1,5 @@
-import React, { useCallback, useEffect, useRef } from 'react';
-import type { MutableRefObject, PropsWithChildren } from 'react';
+import React, { useCallback, useEffect } from 'react';
+import type { PropsWithChildren } from 'react';
 
 import { v4 as uuidv4 } from 'uuid';
 import { createStore } from 'zustand';
@@ -12,15 +12,14 @@ import { ALTINN_ROW_ID } from 'src/features/formData/types';
 import { useOnGroupCloseValidation } from 'src/features/validation/callbacks/onGroupCloseValidation';
 import { OpenByDefaultProvider } from 'src/layout/RepeatingGroup/Providers/OpenByDefaultProvider';
 import { NodesInternal } from 'src/utils/layout/NodesContext';
-import { useNodeItem, useNodeItemRef, useWaitForNodeItem } from 'src/utils/layout/useNodeItem';
+import { useNodeItem, useWaitForNodeItem } from 'src/utils/layout/useNodeItem';
 import type { CompInternal } from 'src/layout/layout';
 import type { IGroupEditProperties } from 'src/layout/RepeatingGroup/config.generated';
 import type { RepGroupRow, RepGroupRows } from 'src/layout/RepeatingGroup/types';
 import type { LayoutNode } from 'src/utils/layout/LayoutNode';
 import type { BaseRow } from 'src/utils/layout/types';
 
 interface Store {
-  freshRowsRef: MutableRefObject<number | undefined>;
   editingAll: boolean;
   editingNone: boolean;
   editingId: string | undefined;
@@ -206,15 +205,17 @@ function gotoPageForRow(
 }
 
 interface NewStoreProps {
-  freshRowsRef: MutableRefObject<number | undefined>;
-  rowsRef: MutableRefObject<RepGroupRows>;
+  getRows: () => RepGroupRows | undefined;
   editMode: IGroupEditProperties['mode'];
   pagination: CompInternal<'RepeatingGroup'>['pagination'];
 }
 
-function newStore({ editMode, pagination, rowsRef, freshRowsRef }: NewStoreProps) {
+function newStore({ editMode, pagination, getRows }: NewStoreProps) {
+  function produce() {
+    return produceStateFromRows(getRows() ?? []);
+  }
+
   return createStore<ZustandState>((set) => ({
-    freshRowsRef,
     editingAll: editMode === 'showAll',
     editingNone: editMode === 'onlyTable',
     isFirstRender: true,
@@ -237,8 +238,8 @@ function newStore({ editMode, pagination, rowsRef, freshRowsRef }: NewStoreProps
         if (state.editingId === row.uuid || state.editingAll || state.editingNone) {
           return state;
         }
-        const { editableRows, visibleRows } = produceStateFromRows(rowsRef.current);
-        if (!editableRows.some((row) => row.uuid === row.uuid)) {
+        const { editableRows, visibleRows } = produce();
+        if (!editableRows.some((r) => r.uuid === row.uuid)) {
           return state;
         }
         const paginationState = producePaginationState(state.currentPage, pagination, visibleRows);
@@ -251,7 +252,7 @@ function newStore({ editMode, pagination, rowsRef, freshRowsRef }: NewStoreProps
         if (state.editingAll || state.editingNone) {
           return state;
         }
-        const { editableRows, visibleRows } = produceStateFromRows(rowsRef.current);
+        const { editableRows, visibleRows } = produce();
         const paginationState = producePaginationState(state.currentPage, pagination, visibleRows);
         if (state.editingId === undefined) {
           const firstRow = editableRows[0];
@@ -320,7 +321,7 @@ function newStore({ editMode, pagination, rowsRef, freshRowsRef }: NewStoreProps
 function useExtendedRepeatingGroupState(node: LayoutNode<'RepeatingGroup'>): ExtendedContext {
   const stateRef = ZStore.useSelectorAsRef((state) => state);
   const validateOnSaveRow = useNodeItem(node, (i) => i.validateOnSaveRow);
-  const groupBinding = useNodeItem(node, (i) => i.dataModelBindings.group);
+  const groupBinding = useBinding(node);
 
   const appendToList = FD.useAppendToList();
   const removeFromList = FD.useRemoveFromListCallback();
@@ -330,20 +331,22 @@ function useExtendedRepeatingGroupState(node: LayoutNode<'RepeatingGroup'>): Ext
 
   const waitForItem = useWaitForNodeItem(node);
 
-  const rowStateRef = useNodeItemRef(node, (i) => produceStateFromRows(i.rows));
-  const paginationStateRef = useNodeItemRef(node, (i) => {
-    const rowState = produceStateFromRows(i.rows);
-    return producePaginationState(stateRef.current.currentPage, i.pagination, rowState.visibleRows);
-  });
+  const pagination = useNodeItem(node, (i) => i.pagination);
+  const getRows = useGetFreshRows(node);
+  const getState = useCallback(() => produceStateFromRows(getRows() ?? []), [getRows]);
+  const getPaginationState = useCallback(
+    () => producePaginationState(stateRef.current.currentPage, pagination, getState().visibleRows),
+    [stateRef, pagination, getState],
+  );
 
   const maybeValidateRow = useCallback(() => {
     const { editingAll, editingId, editingNone } = stateRef.current;
-    const index = rowStateRef.current.editableRows.find((row) => row.uuid === editingId)?.index;
+    const index = getState().editableRows.find((row) => row.uuid === editingId)?.index;
     if (!validateOnSaveRow || editingAll || editingNone || editingId === undefined || index === undefined) {
       return Promise.resolve(false);
     }
     return onGroupCloseValidation(node, index, validateOnSaveRow);
-  }, [node, onGroupCloseValidation, rowStateRef, stateRef, validateOnSaveRow]);
+  }, [node, onGroupCloseValidation, getState, stateRef, validateOnSaveRow]);
 
   const openForEditing = useCallback(
     async (row: BaseRow) => {
@@ -403,14 +406,14 @@ function useExtendedRepeatingGroupState(node: LayoutNode<'RepeatingGroup'>): Ext
         return;
       }
 
-      const page = getPageForRow(row, paginationStateRef.current, rowStateRef.current.visibleRows);
+      const page = getPageForRow(row, getPaginationState(), getState().visibleRows);
       if (page == null) {
         return;
       }
 
       stateRef.current.changePage(page);
     },
-    [maybeValidateRow, rowStateRef, paginationStateRef, stateRef],
+    [maybeValidateRow, getPaginationState, getState, stateRef],
   );
 
   const isEditing = useCallback(
@@ -455,9 +458,10 @@ function useExtendedRepeatingGroupState(node: LayoutNode<'RepeatingGroup'>): Ext
     let attempts = 5;
     let foundVisible: boolean | undefined;
     while (foundVisible === undefined && attempts > 0) {
-      foundVisible = rowStateRef.current.visibleRows.find((row) => row.uuid === uuid)
+      const { visibleRows, hiddenRows } = getState();
+      foundVisible = visibleRows.find((row) => row.uuid === uuid)
         ? true
-        : rowStateRef.current.hiddenRows.find((row) => row.uuid === uuid)
+        : hiddenRows.find((row) => row.uuid === uuid)
           ? false
           : undefined;
       if (foundVisible === undefined && attempts > 0) {
@@ -480,13 +484,13 @@ function useExtendedRepeatingGroupState(node: LayoutNode<'RepeatingGroup'>): Ext
     appendToList,
     markNodesNotReady,
     waitForItem,
-    rowStateRef,
+    getState,
     openForEditing,
   ]);
 
   const deleteRow = useCallback(
     async (row: BaseRow) => {
-      const { deletableRows } = rowStateRef.current;
+      const { deletableRows } = getState();
       const { startDeletingRow, endDeletingRow } = stateRef.current;
       const deletableRow = deletableRows.find((r) => r.uuid === row.uuid && r.index === row.index);
       if (!deletableRow) {
@@ -510,7 +514,7 @@ function useExtendedRepeatingGroupState(node: LayoutNode<'RepeatingGroup'>): Ext
       endDeletingRow(row, false);
       return false;
     },
-    [rowStateRef, stateRef, markNodesNotReady, onBeforeRowDeletion, groupBinding, removeFromList],
+    [getState, stateRef, markNodesNotReady, onBeforeRowDeletion, groupBinding, removeFromList],
   );
 
   const isDeleting = useCallback((uuid: string) => stateRef.current.deletingIds.includes(uuid), [stateRef]);
@@ -559,20 +563,6 @@ function EffectPagination() {
   return null;
 }
 
-/**
- * The item.rows state is updated through effects in the hierarchy generated, and will always be a bit slower
- * than the source (fresh list of rows from the data model). This trick stores a ref always containing a
- * fresh list of rows we can use to filter out rows that are about to be deleted. This fixes a problem
- * where repeating group rows will 'flash' with outdated data before being removed.
- */
-function EffectSelectFreshRows({ freshRowsRef }: { freshRowsRef: MutableRefObject<number | undefined> }) {
-  const node = useRepeatingGroupNode();
-  const binding = useNodeItem(node, (i) => i.dataModelBindings.group);
-  freshRowsRef.current = FD.useFreshNumRows(binding);
-
-  return null;
-}
-
 /**
  * This function filters out rows that are about to be deleted from the rows state
  */
@@ -595,21 +585,17 @@ interface Props {
 export function RepeatingGroupProvider({ node, children }: PropsWithChildren<Props>) {
   const pagination = useNodeItem(node, (i) => i.pagination);
   const editMode = useNodeItem(node, (i) => i.edit?.mode);
-
-  const freshRowsRef = useRef<number | undefined>(undefined);
-  const rowsRef = useNodeItemRef(node, (i) => filterByFreshRows(i.rows, freshRowsRef.current));
+  const getRows = useGetFreshRows(node);
 
   return (
     <ZStore.Provider
-      freshRowsRef={freshRowsRef}
-      rowsRef={rowsRef}
+      getRows={getRows}
       pagination={pagination}
       editMode={editMode}
     >
       <ProvideTheRest node={node}>
         <EffectCloseEditing />
         <EffectPagination />
-        <EffectSelectFreshRows freshRowsRef={freshRowsRef} />
         <OpenByDefaultProvider node={node}>{children}</OpenByDefaultProvider>
       </ProvideTheRest>
     </ZStore.Provider>
@@ -619,10 +605,21 @@ export function RepeatingGroupProvider({ node, children }: PropsWithChildren<Pro
 export const useRepeatingGroup = () => ExtendedStore.useCtx();
 export const useRepeatingGroupNode = () => ExtendedStore.useCtx().node;
 
+function useBinding(node: LayoutNode<'RepeatingGroup'>) {
+  return NodesInternal.useNodeData(node, (d) => d.layout.dataModelBindings.group);
+}
+
+function useGetFreshRows(node: LayoutNode<'RepeatingGroup'>) {
+  const getFreshNumRows = FD.useGetFreshNumRows();
+  return NodesInternal.useGetNodeData(node, (d) =>
+    filterByFreshRows(d.item?.rows, getFreshNumRows(d.layout.dataModelBindings.group)),
+  );
+}
+
 export const useRepeatingGroupRowState = () => {
   const node = useRepeatingGroupNode();
-  const freshRowsRef = ZStore.useSelector((state) => state.freshRowsRef);
-  return useNodeItem(node, (i) => produceStateFromRows(filterByFreshRows(i.rows, freshRowsRef.current)));
+  const numRows = FD.useFreshNumRows(useBinding(node));
+  return useNodeItem(node, (i) => produceStateFromRows(filterByFreshRows(i.rows, numRows)));
 };
 
 export const useRepeatingGroupPagination = () => {

diff --git a/src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx b/src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx
@@ -4,7 +4,7 @@ import type { PropsWithChildren } from 'react';
 import { createContext } from 'src/core/contexts/context';
 import { useRegisterNodeNavigationHandler } from 'src/features/form/layout/NavigateToNode';
 import { useRepeatingGroup } from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext';
-import { useNodeItemRef } from 'src/utils/layout/useNodeItem';
+import { NodesInternal } from 'src/utils/layout/NodesContext';
 import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal';
 import type { LayoutNode } from 'src/utils/layout/LayoutNode';
 
@@ -40,7 +40,7 @@ export function RepeatingGroupsFocusProvider({ children }: PropsWithChildren) {
   const traversal = useNodeTraversalSelector();
 
   const { node, openForEditing, changePageToRow } = useRepeatingGroup();
-  const nodeItem = useNodeItemRef(node);
+  const getNodeItem = NodesInternal.useGetNodeData(node, (d) => d.item);
   useRegisterNodeNavigationHandler(async (targetNode) => {
     // Figure out if we are a parent of the target component, setting the targetChild to the target
     // component (or a nested repeating group containing the target component).
@@ -66,25 +66,25 @@ export function RepeatingGroupsFocusProvider({ children }: PropsWithChildren) {
       return false;
     }
 
-    const row = nodeItem.current.rows.find((r) => r?.items?.some((n) => n === targetChild));
+    const item = getNodeItem();
+    const row = item?.rows.find((r) => r?.items?.some((n) => n === targetChild));
 
     // If pagination is used, navigate to the correct page
-    if (nodeItem.current.pagination) {
+    if (item?.pagination) {
       if (row) {
         await changePageToRow(row);
       } else {
         return false;
       }
     }
 
-    if (nodeItem.current.edit?.mode === 'showAll' || nodeItem.current.edit?.mode === 'onlyTable') {
+    if (item?.edit?.mode === 'showAll' || item?.edit?.mode === 'onlyTable') {
       // We're already showing all nodes, so nothing further to do
       return true;
     }
 
     // Check if we need to open the row containing targetChild for editing.
-    const tableColSetup =
-      (nodeItem.current.tableColumns && targetChild.baseId && nodeItem.current.tableColumns[targetChild.baseId]) || {};
+    const tableColSetup = (item?.tableColumns && targetChild.baseId && item?.tableColumns[targetChild.baseId]) || {};
 
     if (tableColSetup.editInTable || tableColSetup.showInExpandedEdit === false) {
       // No need to open rows or set editIndex for components that are rendered

diff --git a/src/layout/RepeatingGroup/Table/RepeatingGroupTableRow.tsx b/src/layout/RepeatingGroup/Table/RepeatingGroupTableRow.tsx
@@ -10,7 +10,6 @@ import { ConditionalWrapper } from 'src/components/ConditionalWrapper';
 import { DeleteWarningPopover } from 'src/features/alertOnChange/DeleteWarningPopover';
 import { useAlertOnChange } from 'src/features/alertOnChange/useAlertOnChange';
 import { useDisplayDataProps } from 'src/features/displayData/useDisplayData';
-import { FD } from 'src/features/formData/FormDataWrite';
 import { Lang } from 'src/features/language/Lang';
 import { useLanguage } from 'src/features/language/useLanguage';
 import { useDeepValidationsForNode } from 'src/features/validation/selectors/deepValidationsForNode';
@@ -91,8 +90,6 @@ export const RepeatingGroupTableRow = React.memo(function RepeatingGroupTableRow
   const id = node.id;
   const group = useNodeItem(node);
   const row = group.rows.find((r) => r && r.uuid === uuid && r.index === index);
-  const freshUuid = FD.useFreshRowUuid(group.dataModelBindings?.group, index);
-  const isFresh = freshUuid === uuid;
   const rowExpressions = row?.groupExpressions;
   const editForRow = rowExpressions?.edit;
   const editForGroup = group.edit;
@@ -252,7 +249,6 @@ export const RepeatingGroupTableRow = React.memo(function RepeatingGroupTableRow
                   aria-label={`${editButtonText} ${firstCellData ?? ''}`}
                   data-testid='edit-button'
                   className={classes.tableButton}
-                  disabled={!isFresh}
                 >
                   {editButtonText}
                   {rowHasErrors ? (
@@ -286,7 +282,6 @@ export const RepeatingGroupTableRow = React.memo(function RepeatingGroupTableRow
                   firstCellData={firstCellData}
                   alertOnDeleteProps={alertOnDelete}
                   langAsString={langAsString}
-                  disabled={!isFresh}
                 >
                   {deleteButtonText}
                 </DeleteElement>
@@ -339,7 +334,6 @@ export const RepeatingGroupTableRow = React.memo(function RepeatingGroupTableRow
                   firstCellData={firstCellData}
                   alertOnDeleteProps={alertOnDelete}
                   langAsString={langAsString}
-                  disabled={!isFresh}
                 >
                   {isEditingRow || !mobileViewSmall ? deleteButtonText : null}
                 </DeleteElement>