From 278177e29708f336e923df48ada96aec12cc0972 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Thu, 14 Nov 2024 09:01:49 +0100 Subject: [PATCH] Safer way to select older data while re-generating parts of the node tree (#2715) Co-authored-by: Ole Martin Handeland --- src/features/formData/FormDataWrite.tsx | 41 ++-- .../Providers/RepeatingGroupContext.tsx | 93 ++++---- .../Providers/RepeatingGroupFocusContext.tsx | 14 +- .../Table/RepeatingGroupTableRow.tsx | 6 - src/utils/layout/NodesContext.tsx | 204 +++++++++++++----- src/utils/layout/generator/CommitQueue.tsx | 21 +- .../layout/generator/GeneratorContext.tsx | 9 +- .../layout/generator/GeneratorStages.tsx | 1 - .../plugins/RepeatingChildrenStorePlugin.tsx | 15 +- src/utils/layout/useNodeItem.ts | 102 ++------- src/utils/layout/useNodeTraversal.ts | 10 +- 11 files changed, 288 insertions(+), 228 deletions(-) diff --git a/src/features/formData/FormDataWrite.tsx b/src/features/formData/FormDataWrite.tsx index d4699d2042..4b9efb7f28 100644 --- 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 index 3f41fc1ac2..17a616204e 100644 --- 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,7 +12,7 @@ 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'; @@ -20,7 +20,6 @@ import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { BaseRow } from 'src/utils/layout/types'; interface Store { - freshRowsRef: MutableRefObject; editingAll: boolean; editingNone: boolean; editingId: string | undefined; @@ -206,15 +205,17 @@ function gotoPageForRow( } interface NewStoreProps { - freshRowsRef: MutableRefObject; - rowsRef: MutableRefObject; + 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((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 }) { - 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) { const pagination = useNodeItem(node, (i) => i.pagination); const editMode = useNodeItem(node, (i) => i.edit?.mode); - - const freshRowsRef = useRef(undefined); - const rowsRef = useNodeItemRef(node, (i) => filterByFreshRows(i.rows, freshRowsRef.current)); + const getRows = useGetFreshRows(node); return ( - {children} @@ -619,10 +605,21 @@ export function RepeatingGroupProvider({ node, children }: PropsWithChildren 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 index 0b259342c5..584e422a70 100644 --- 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,10 +66,11 @@ 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 { @@ -77,14 +78,13 @@ export function RepeatingGroupsFocusProvider({ children }: PropsWithChildren) { } } - 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 index d75b14a1e4..ea49c76ba0 100644 --- 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} @@ -339,7 +334,6 @@ export const RepeatingGroupTableRow = React.memo(function RepeatingGroupTableRow firstCellData={firstCellData} alertOnDeleteProps={alertOnDelete} langAsString={langAsString} - disabled={!isFresh} > {isEditingRow || !mobileViewSmall ? deleteButtonText : null} diff --git a/src/utils/layout/NodesContext.tsx b/src/utils/layout/NodesContext.tsx index cd88d6d19f..bee92aa962 100644 --- a/src/utils/layout/NodesContext.tsx +++ b/src/utils/layout/NodesContext.tsx @@ -30,6 +30,7 @@ import { } from 'src/features/validation/validationContext'; import { ValidationStorePlugin } from 'src/features/validation/ValidationStorePlugin'; import { SelectorStrictness, useDelayedSelector } from 'src/hooks/delayedSelectors'; +import { useAsRef } from 'src/hooks/useAsRef'; import { useCurrentView } from 'src/hooks/useNavigatePage'; import { useWaitForState } from 'src/hooks/useWaitForState'; import { getComponentDef } from 'src/layout'; @@ -113,6 +114,7 @@ export interface RemoveNodeRequest { node: LayoutNode; claim: ChildClaim; rowIndex: number | undefined; + layouts: ILayouts; } export interface SetNodePropRequest> { @@ -145,11 +147,13 @@ export type NodesContext = { nodes: LayoutPages | undefined; pagesData: PagesData; nodeData: { [key: string]: NodeData }; + prevNodeData: { [key: string]: NodeData } | undefined; // Earlier node data from before the state became non-ready childrenMap: { [key: string]: string[] | undefined }; hiddenViaRules: { [key: string]: true | undefined }; hiddenViaRulesRan: boolean; validationsProcessedLast: ValidationsProcessedLast; + layouts: ILayouts | undefined; // Used to detect if the layouts have changed stages: GeneratorStagesContext; setNodes: (nodes: LayoutPages) => void; @@ -165,7 +169,7 @@ export type NodesContext = { onSaveFinished: (result: FDSaveFinished) => void; setLatestInitialValidations: (validations: ValidationsProcessedLast['initial']) => void; - reset: (validationsProcessedLast: ValidationsProcessedLast) => void; + reset: (layouts: ILayouts, validationsProcessedLast: ValidationsProcessedLast) => void; waitForCommits: undefined | (() => Promise); setWaitForCommits: (waitForCommits: () => Promise) => void; @@ -196,6 +200,7 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre pages: {}, }, nodeData: {}, + prevNodeData: {}, childrenMap: {}, hiddenViaRules: {}, hiddenViaRulesRan: false, @@ -205,6 +210,7 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre return createStore((set) => ({ ...defaultState, + layouts: undefined, stages: createStagesStore(registry, set), markHiddenViaRule: (newState) => @@ -256,11 +262,17 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre const nodeData = { ...state.nodeData }; const childrenMap = { ...state.childrenMap }; - for (const { node, claim, rowIndex } of requests) { + for (const { node, claim, rowIndex, layouts } of requests) { if (!nodeData[node.id]) { continue; } + if (layouts !== state.layouts) { + // The layouts have changed since the request was added, so there's no need to remove the node (it was + // automatically removed when resetting the NodesContext state upon the layout change) + continue; + } + if (node.parent instanceof BaseLayoutNode && nodeData[node.parent.id]) { const additionalParentState = node.parent.def.removeChild( // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -332,7 +344,7 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre // We need to mark the data as not ready as soon as an error is added, because GeneratorErrorBoundary // may need to remove the failing node from the tree before any more node traversal can happen safely. - state.readiness = NodesReadiness.NotReady; + setReadiness({ state, target: NodesReadiness.NotReady, reason: `Error added`, mutate: true }); state.hasErrors = true; }), @@ -351,8 +363,13 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre inOrder: true, errors: undefined, }; - state.readiness = NodesReadiness.NotReady; - state.addRemoveCounter += 1; + setReadiness({ + state, + target: NodesReadiness.NotReady, + reason: `New page added`, + mutate: true, + newNodes: true, + }); }), ), setPageProps: (requests) => @@ -369,13 +386,7 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre return { pagesData: { type: 'pages', pages: pageData } }; }), markReady: (reason, readiness = NodesReadiness.Ready) => - set((state) => { - if (state.readiness !== readiness) { - generatorLog('logReadiness', `Marking state as ${readiness}: ${reason}`); - return { readiness }; - } - return {}; - }), + set((state) => setReadiness({ state, target: readiness, reason })), onSaveFinished: (result) => set((state) => { if (state.readiness !== NodesReadiness.WaitingUntilLastSaveHasProcessed) { @@ -384,6 +395,7 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre return { readiness: NodesReadiness.WaitingUntilLastSaveHasProcessed, + prevNodeData: state.nodeData, validationsProcessedLast: { ...state.validationsProcessedLast, incremental: result.validationIssues, @@ -398,8 +410,11 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre }, })), - reset: (validationsProcessedLast: ValidationsProcessedLast) => - set(() => ({ ...structuredClone(defaultState), validationsProcessedLast })), + reset: (layouts, validationsProcessedLast: ValidationsProcessedLast) => + set(() => { + generatorLog('logReadiness', 'Resetting state'); + return { ...structuredClone(defaultState), layouts, validationsProcessedLast }; + }), waitForCommits: undefined, setWaitForCommits: (waitForCommits) => set(() => ({ waitForCommits })), @@ -409,6 +424,51 @@ export function createNodesDataStore({ registry, validationsProcessedLast }: Cre .reduce((acc, val) => ({ ...acc, ...val }), {}) as ExtraFunctions), })); } + +interface SetReadinessProps { + state: NodesContext; + target: NodesReadiness; + reason: string; + newNodes?: boolean; + mutate?: boolean; +} + +/** + * Helper function to set new readiness state. Never try to set a new readiness without going through this function. + */ +export function setReadiness({ + state, + target, + reason, + newNodes = false, + mutate = false, +}: SetReadinessProps): Partial { + const toSet: Partial = {}; + if (state.readiness !== target) { + generatorLog('logReadiness', `Marking state as ${target}: ${reason}`); + toSet.readiness = target; + if (target !== NodesReadiness.Ready && state.readiness === NodesReadiness.Ready) { + // Making a copy of the nodeData from when the state was ready last, so that selectors can continue running + // with the old data until the new data is ready. This should also make sure it doesn't accidentally copy + // non-ready state if the readiness changes multiple times before becoming ready again. + toSet.prevNodeData = state.nodeData; + } else if (target === NodesReadiness.Ready) { + toSet.prevNodeData = undefined; + } + if (newNodes) { + toSet.addRemoveCounter = state.addRemoveCounter + 1; + } + } + + if (mutate) { + for (const key in toSet) { + state[key] = toSet[key]; + } + } + + return toSet; +} + const Store = createZustandContext({ name: 'Nodes', required: true, @@ -464,22 +524,22 @@ function whenReadySelector( */ const Conditionally = { useSelector: (selector: (state: NodesContext) => T): T | undefined => { - const isGenerating = GeneratorStages.useIsGenerating(); + const isGenerating = GeneratorInternal.useIsInsideGenerator(); // eslint-disable-next-line react-hooks/rules-of-hooks return isGenerating ? Store.useSelector(selector) : WhenReady.useSelector(selector); }, useMemoSelector: (selector: (state: NodesContext) => T): T | undefined => { - const isGenerating = GeneratorStages.useIsGenerating(); + const isGenerating = GeneratorInternal.useIsInsideGenerator(); // eslint-disable-next-line react-hooks/rules-of-hooks return isGenerating ? Store.useMemoSelector(selector) : WhenReady.useMemoSelector(selector); }, useLaxSelector: (selector: (state: NodesContext) => T): T | typeof ContextNotProvided => { - const isGenerating = GeneratorStages.useIsGenerating(); + const isGenerating = GeneratorInternal.useIsInsideGenerator(); // eslint-disable-next-line react-hooks/rules-of-hooks return isGenerating ? Store.useLaxSelector(selector) : WhenReady.useLaxSelector(selector); }, useLaxMemoSelector: (selector: (state: NodesContext) => T): T | typeof ContextNotProvided => { - const isGenerating = GeneratorStages.useIsGenerating(); + const isGenerating = GeneratorInternal.useIsInsideGenerator(); // eslint-disable-next-line react-hooks/rules-of-hooks return isGenerating ? Store.useLaxMemoSelector(selector) : WhenReady.useLaxMemoSelector(selector); }, @@ -517,7 +577,7 @@ export const NodesProvider = ({ children }: React.PropsWithChildren) => { function ProvideGlobalContext({ children, registry }: PropsWithChildren<{ registry: MutableRefObject }>) { const latestLayouts = useLayouts(); - const [layouts, setLayouts] = useState(latestLayouts); + const layouts = Store.useSelector((s) => s.layouts); const markNotReady = NodesInternal.useMarkNotReady(); const reset = Store.useSelector((s) => s.reset); const processedLast = Validation.useProcessedLastRef(); @@ -525,14 +585,13 @@ function ProvideGlobalContext({ children, registry }: PropsWithChildren<{ regist useEffect(() => { if (layouts !== latestLayouts) { markNotReady('new layouts'); - setLayouts(latestLayouts); - reset(processedLast.current); + reset(latestLayouts, processedLast.current); } }, [latestLayouts, layouts, markNotReady, reset, processedLast]); const layoutMap = useMemo(() => { const out: { [id: string]: CompExternal } = {}; - for (const page of Object.values(layouts)) { + for (const page of Object.values(latestLayouts)) { if (!page) { continue; } @@ -542,7 +601,7 @@ function ProvideGlobalContext({ children, registry }: PropsWithChildren<{ regist } return out; - }, [layouts]); + }, [latestLayouts]); if (layouts !== latestLayouts) { // You changed the layouts, possibly by using devtools. Hold on while we re-generate! @@ -802,20 +861,19 @@ type RetValFromNode = T extends LayoutNode * Usually, if you're looking for a specific component/node, useResolvedNode() is better. */ export function useNode(id: T): RetValFromNode { + const lastValue = useRef(NeverInitialized); const node = Store.useSelector((state) => { - if (!id) { + if (!id || !state?.nodes) { return undefined; } - if (!state?.nodes) { - return undefined; - } - - if (id instanceof BaseLayoutNode) { - return id; + if (state.readiness !== NodesReadiness.Ready && lastValue.current !== NeverInitialized) { + return lastValue.current; } - return state.nodes.findById(id); + const node = id instanceof BaseLayoutNode ? id : state.nodes.findById(id); + lastValue.current = node; + return node; }); return node as RetValFromNode; } @@ -997,11 +1055,37 @@ export const Hidden = { export type NodeDataSelector = ReturnType; export type LaxNodeDataSelector = ReturnType; -export type NodePicker = (node: N) => NodePickerReturns; +export type NodePicker = ( + node: N | string, +) => NodePickerReturns; type NodePickerReturns = NodeDataFromNode | undefined; -function selectNodeData(node: N, state: NodesContext): NodePickerReturns { - return (node ? state.nodeData[node.id] : undefined) as NodePickerReturns; +function selectNodeData( + node: N | string, + state: NodesContext, + alwaysUseFreshData = false, +): NodePickerReturns { + const source = + state.readiness === NodesReadiness.Ready || alwaysUseFreshData + ? state.nodeData + : state.prevNodeData && Object.keys(state.prevNodeData).length > 0 + ? state.prevNodeData + : state.nodeData; + + if (typeof node === 'string') { + return source[node] as NodePickerReturns; + } + + return (node ? source[node.id] : undefined) as NodePickerReturns; +} + +function getNodeData( + node: N | string, + state: NodesContext, + selector: (nodeData: NodeDataFromNode) => Out, + alwaysUseFreshData = false, +) { + return node ? selector(selectNodeData(node, state, alwaysUseFreshData) as NodeDataFromNode) : undefined; } /** @@ -1145,18 +1229,32 @@ export const NodesInternal = { node: N, selector: (nodeData: NodeDataFromNode, readiness: NodesReadiness, fullState: NodesContext) => Out, ) { - return Conditionally.useMemoSelector((s) => - node && s.nodeData[node.id] ? selector(s.nodeData[node.id] as NodeDataFromNode, s.readiness, s) : undefined, - ) as N extends undefined ? Out | undefined : Out; + const insideGenerator = GeneratorInternal.useIsInsideGenerator(); + return Conditionally.useMemoSelector((s) => { + if (!node) { + return undefined; + } + const data = + insideGenerator && s.nodeData[node.id] + ? s.nodeData[node.id] + : s.readiness === NodesReadiness.Ready + ? s.nodeData[node.id] + : (s.prevNodeData?.[node.id] ?? s.nodeData[node.id]); + + return data ? selector(data as NodeDataFromNode, s.readiness, s) : undefined; + }) as N extends undefined ? Out | undefined : Out; }, - useNodeDataRef( + useGetNodeData( node: N, selector: (state: NodeDataFromNode) => Out, - ): React.MutableRefObject { - return Store.useSelectorAsRef( - (s) => (node && s.nodeData[node.id] ? selector(s.nodeData[node.id] as NodeDataFromNode) : undefined), - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ) as any; + ): () => Out | undefined { + const store = Store.useStore(); + const selectorRef = useAsRef(selector); + const insideGenerator = GeneratorInternal.useIsInsideGenerator(); + return useCallback( + () => getNodeData(node, store.getState(), (nodeData) => selectorRef.current(nodeData), insideGenerator), + [store, node, selectorRef, insideGenerator], + ); }, useWaitForNodeData( node: N, @@ -1179,16 +1277,20 @@ export const NodesInternal = { [waitForState, node, selector], ); }, - useNodeDataSelector: () => - Store.useDelayedSelector({ + useNodeDataSelector: () => { + const insideGenerator = GeneratorInternal.useIsInsideGenerator(); + return Store.useDelayedSelector({ mode: 'innerSelector', - makeArgs: (state) => [((node) => selectNodeData(node, state)) satisfies NodePicker], - }), - useLaxNodeDataSelector: () => - Store.useLaxDelayedSelector({ + makeArgs: (state) => [((node) => selectNodeData(node, state, insideGenerator)) satisfies NodePicker], + }); + }, + useLaxNodeDataSelector: () => { + const insideGenerator = GeneratorInternal.useIsInsideGenerator(); + return Store.useLaxDelayedSelector({ mode: 'innerSelector', - makeArgs: (state) => [((node) => selectNodeData(node, state)) satisfies NodePicker], - }), + makeArgs: (state) => [((node) => selectNodeData(node, state, insideGenerator)) satisfies NodePicker], + }); + }, useTypeFromId: (id: string) => Store.useSelector((s) => s.nodeData[id]?.layout.type), useIsAdded: (node: LayoutNode | LayoutPage | undefined) => Store.useSelector((s) => { diff --git a/src/utils/layout/generator/CommitQueue.tsx b/src/utils/layout/generator/CommitQueue.tsx index e803a0ab6a..ce9b7c1e11 100644 --- a/src/utils/layout/generator/CommitQueue.tsx +++ b/src/utils/layout/generator/CommitQueue.tsx @@ -146,7 +146,7 @@ export function SetWaitForCommits() { */ export const NodesStateQueue = { useAddNode: (req: AddNodeRequest, condition = true) => useAddToQueue('addNodes', false, req, condition), - useRemoveNode: (req: RemoveNodeRequest) => useAddToQueueOnUnmount('removeNodes', true, req), + useRemoveNode: (req: Omit) => useRemoveNode(req), // eslint-disable-next-line @typescript-eslint/no-explicit-any useSetNodeProp: (req: SetNodePropRequest, condition = true) => useAddToQueue('setNodeProps', true, req, condition), @@ -179,11 +179,11 @@ function useAddToQueue( } } -function useAddToQueueOnUnmount( - queue: T, - commitAfter: boolean, - request: RegistryCommitQueues[T][number], -) { +function useRemoveNode(request: Omit) { + // This state is intentionally not reactive, as we want to commit _what the layout was when this node was created_, + // so that we don't accidentally remove a node with the same ID from a future/different layout. + const layoutsWas = NodesStore.useStaticSelector((s) => s.layouts!); + const registry = GeneratorInternal.useRegistry(); const toCommit = registry.current.toCommit; const ref = useAsRef(request); @@ -196,14 +196,11 @@ function useAddToQueueOnUnmount( return () => { reg.toCommitCount += 1; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - toCommit[queue].push(request as any); + toCommit.removeNodes.push({ ...request, layouts: layoutsWas }); updateCommitsPendingInBody(toCommit); - if (commitAfter) { - commit(); - } + commit(); }; - }, [commit, commitAfter, queue, ref, registry, toCommit]); + }, [commit, ref, registry, toCommit, layoutsWas]); } /** diff --git a/src/utils/layout/generator/GeneratorContext.tsx b/src/utils/layout/generator/GeneratorContext.tsx index 0b0ee8cc0b..e8ef9a86f4 100644 --- a/src/utils/layout/generator/GeneratorContext.tsx +++ b/src/utils/layout/generator/GeneratorContext.tsx @@ -1,7 +1,7 @@ import React, { useMemo } from 'react'; import type { MutableRefObject, PropsWithChildren } from 'react'; -import { createContext } from 'src/core/contexts/context'; +import { ContextNotProvided, createContext } from 'src/core/contexts/context'; import type { IDataModelReference } from 'src/layout/common.generated'; import type { CompExternal, CompIntermediate, CompIntermediateExact, CompTypes, ILayouts } from 'src/layout/layout'; import type { Registry } from 'src/utils/layout/generator/GeneratorStages'; @@ -58,7 +58,7 @@ interface GeneratorContext { depth: number; // Depth is 1 for top level nodes, 2 for children of top level nodes, etc. } -const { Provider, useCtx } = createContext({ +const { Provider, useCtx, useLaxCtx } = createContext({ name: 'Generator', required: true, }); @@ -167,7 +167,10 @@ export function GeneratorGlobalProvider({ children, ...rest }: PropsWithChildren } export const GeneratorInternal = { - useIsInsideGenerator: () => useCtx().depth > 0, + useIsInsideGenerator: () => { + const ctx = useLaxCtx(); + return ctx === ContextNotProvided ? false : ctx.depth > 0; + }, useRegistry: () => useCtx().registry, useDirectMutators: () => useCtx().directMutators ?? emptyArray, useRecursiveMutators: () => useCtx().recursiveMutators ?? emptyArray, diff --git a/src/utils/layout/generator/GeneratorStages.tsx b/src/utils/layout/generator/GeneratorStages.tsx index 22afbf5ee3..bc677b4db4 100644 --- a/src/utils/layout/generator/GeneratorStages.tsx +++ b/src/utils/layout/generator/GeneratorStages.tsx @@ -388,7 +388,6 @@ function WhenTickIsSet({ children }: PropsWithChildren) { export const GeneratorStages = { useIsDoneAddingNodes: () => useIsStageAtLeast(StageAddNodes), useIsFinished: () => NodesStore.useMemoSelector((state) => state.stages.currentStage === StageFinished), - useIsGenerating: () => NodesStore.useHasProvider(), }; /** diff --git a/src/utils/layout/plugins/RepeatingChildrenStorePlugin.tsx b/src/utils/layout/plugins/RepeatingChildrenStorePlugin.tsx index 7a095d4379..5325f4b723 100644 --- a/src/utils/layout/plugins/RepeatingChildrenStorePlugin.tsx +++ b/src/utils/layout/plugins/RepeatingChildrenStorePlugin.tsx @@ -1,6 +1,6 @@ import deepEqual from 'fast-deep-equal'; -import { NodesReadiness } from 'src/utils/layout/NodesContext'; +import { NodesReadiness, setReadiness } from 'src/utils/layout/NodesContext'; import { NodeDataPlugin } from 'src/utils/layout/plugins/NodeDataPlugin'; import type { CompTypes } from 'src/layout/layout'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; @@ -72,7 +72,9 @@ export class RepeatingChildrenStorePlugin extends NodeDataPlugin { @@ -109,7 +111,9 @@ export class RepeatingChildrenStorePlugin extends NodeDataPlugin { @@ -142,7 +146,10 @@ export class RepeatingChildrenStorePlugin extends NodeDataPlugin( selector?: undefined, ): N extends undefined ? undefined : NodeItemFromNode; // eslint-disable-next-line no-redeclare -export function useNodeItem(node: never, selector: never): never { - const lastItem = useRef(undefined); - const insideGenerator = GeneratorInternal.useIsInsideGenerator(); - return NodesInternal.useNodeData(node, (data: NodeData, readiness) => { - if (insideGenerator) { - throw new Error( - 'useNodeItem() should not be used inside the node generator, it would most likely just crash when ' + - 'the item is undefined. Instead, use GeneratorInternal.useIntermediateItem() to get the item before ' + - 'expressions have run, or use a more specific selector in NodesInternal.useNodeData() which will ' + - 'make you handle the undefined item.', - ); - } - - let item: CompInternal | undefined; - if (readiness === NodesReadiness.Ready && data.item) { - item = data.item; - lastItem.current = item; - } else if (lastItem.current) { - item = lastItem.current; - } else { - // This is possibly stale state, or in the process of being updated, but it's better than failing hard. - item = data.item; - } +export function useNodeItem(node: LayoutNode | undefined, selector: never): unknown { + if (GeneratorInternal.useIsInsideGenerator()) { + throw new Error( + 'useNodeItem() should not be used inside the node generator, it would most likely just crash when ' + + 'the item is undefined. Instead, use GeneratorInternal.useIntermediateItem() to get the item before ' + + 'expressions have run, or use a more specific selector in NodesInternal.useNodeData() which will ' + + 'make you handle the undefined item.', + ); + } - if (!item) { + return NodesInternal.useNodeData(node, (data: NodeData, readiness) => { + if (!data?.item) { throw new Error( - `Node item is undefined. This should normally not happen, but might happen if you select data in new ` + - `components while the node state is in the process of being updated (readiness is '${readiness}'). `, + `Node item for '${node?.id}' is undefined. This should normally not happen, but might happen if you ` + + `select data in new components while the node state is in the process of being updated ` + + `(readiness is '${readiness}'). `, ); } // eslint-disable-next-line @typescript-eslint/no-explicit-any - return selector ? (selector as any)(item) : item; + return selector ? (selector as any)(data.item) : data.item; }); } -export function useNodeItemRef( - node: N, - selector: (item: NodeItemFromNode) => Out, -): MutableRefObject; -// eslint-disable-next-line no-redeclare -export function useNodeItemRef( - node: N, - selector?: undefined, -): MutableRefObject>; -// eslint-disable-next-line no-redeclare -export function useNodeItemRef(node: never, selector: never): never { - return NodesInternal.useNodeDataRef(node, (node: NodeData) => - // eslint-disable-next-line @typescript-eslint/no-explicit-any - selector ? (selector as any)(node.item) : node.item, - ) as never; -} - const selectNodeItem = (data: NodeData): CompInternal | undefined => data.item as CompInternal; export function useWaitForNodeItem( @@ -92,38 +62,12 @@ export function useWaitForNodeItem( const emptyArray: LayoutNode[] = []; export function useNodeDirectChildren(parent: LayoutNode, restriction?: TraversalRestriction): LayoutNode[] { - const insideGenerator = GeneratorInternal.useIsInsideGenerator(); - const lastValue = useRef(undefined); - return NodesInternal.useNodeData(parent, (nodeData, readiness, fullState) => { - if (readiness !== NodesReadiness.Ready && !insideGenerator && lastValue.current) { - return lastValue.current; - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const out = parent.def.pickDirectChildren(nodeData as any, restriction); - if (!insideGenerator) { - // If we're not inside the generator, we should make sure to only return values that make sense. - for (const child of out) { - if (!child) { - // At least one child is undefined, meaning we're in the process of adding/removing nodes. Better to return - // none than return a broken-up set of children. - return emptyArray; - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const childNodeData = fullState.nodeData[child.id] as any; - if ( - !childNodeData || - !child.def.stateIsReady(childNodeData) || - !child.def.pluginStateIsReady(childNodeData, fullState) - ) { - // At least one child is not ready, so rendering these out would be worse than pretending there are none. - return emptyArray; - } - } - } - - lastValue.current = out; - return out ?? emptyArray; - }); + return NodesInternal.useNodeData( + parent, + (nodeData) => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + parent.def.pickDirectChildren(nodeData as any, restriction) ?? emptyArray, + ); } type NodeFormData = N extends undefined diff --git a/src/utils/layout/useNodeTraversal.ts b/src/utils/layout/useNodeTraversal.ts index c5da46a07c..8cd56efce1 100644 --- a/src/utils/layout/useNodeTraversal.ts +++ b/src/utils/layout/useNodeTraversal.ts @@ -4,7 +4,7 @@ import { ContextNotProvided } from 'src/core/contexts/context'; import { BaseLayoutNode } from 'src/utils/layout/LayoutNode'; import { LayoutPage } from 'src/utils/layout/LayoutPage'; import { LayoutPages } from 'src/utils/layout/LayoutPages'; -import { NodesInternal, useNodesLax } from 'src/utils/layout/NodesContext'; +import { NodesInternal, NodesReadiness, useNodesLax } from 'src/utils/layout/NodesContext'; import type { ParentNode } from 'src/layout/layout'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { NodesContext, PageData, PagesData } from 'src/utils/layout/NodesContext'; @@ -37,6 +37,14 @@ export class TraversalTask { return this.state.pagesData as DataFrom; } + if (this.state.readiness !== NodesReadiness.Ready && this.state.prevNodeData?.[target.id]) { + return this.state.prevNodeData[target.id] as DataFrom; + } + + if (!this.state.nodeData[target.id]) { + throw new Error(`Node data for ${target.id} is missing (when matching/getting data in traversal)`); + } + return this.state.nodeData[target.id] as DataFrom; }