From ee505bae4049c86bd3c0922a01f0801e0d304e65 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Mon, 4 Nov 2024 17:31:55 +0100 Subject: [PATCH 1/3] Found the root cause! This thing re-rendered ProcessWrapper all the time (after each save) and caused all nodes to re-render and unmount (removing them in the process, and probably damaging the NodesContext). --- src/features/datamodel/useBindingSchema.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/features/datamodel/useBindingSchema.tsx b/src/features/datamodel/useBindingSchema.tsx index c470687b6..a4e0eeedc 100644 --- a/src/features/datamodel/useBindingSchema.tsx +++ b/src/features/datamodel/useBindingSchema.tsx @@ -10,7 +10,7 @@ import { } from 'src/features/applicationMetadata/appMetadataUtils'; import { DataModels } from 'src/features/datamodel/DataModelsProvider'; import { useLayoutSets } from 'src/features/form/layoutSets/LayoutSetsProvider'; -import { useLaxInstanceAllDataElements, useLaxInstanceId } from 'src/features/instance/InstanceContext'; +import { useLaxInstanceData, useLaxInstanceId } from 'src/features/instance/InstanceContext'; import { useProcessTaskId } from 'src/features/instance/useProcessTaskId'; import { useCurrentLanguage } from 'src/features/language/LanguageProvider'; import { useAllowAnonymous } from 'src/features/stateless/getAllowAnonymous'; @@ -29,17 +29,21 @@ export type AsSchema = { }; export function useCurrentDataModelGuid() { - const dataElements = useLaxInstanceAllDataElements(); const application = useApplicationMetadata(); const layoutSets = useLayoutSets(); const taskId = useProcessTaskId(); const overriddenDataModelGuid = useTaskStore((s) => s.overriddenDataModelUuid); - if (overriddenDataModelGuid) { - return overriddenDataModelGuid; - } - return getCurrentTaskDataElementId({ application, dataElements, taskId, layoutSets }); + // Instance data elements will update often (after each save), so we have to use a selector to make + // sure components don't re-render too often. + return useLaxInstanceData((data) => { + if (overriddenDataModelGuid) { + return overriddenDataModelGuid; + } + + return getCurrentTaskDataElementId({ application, dataElements: data.data, taskId, layoutSets }); + }); } type DataModelDeps = { From d8a0e3a48627867c150030587bf321c07219855e Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Tue, 5 Nov 2024 10:07:32 +0100 Subject: [PATCH 2/3] Optimizing other usages of useLaxInstanceAllDataElements() --- src/features/datamodel/DataModelsProvider.tsx | 4 +- src/features/instance/InstanceContext.tsx | 135 ++++++++++-------- src/features/validation/index.ts | 4 +- .../nodeValidation/useNodeValidation.ts | 8 +- .../AttachmentListComponent.tsx | 10 +- src/layout/Subform/index.tsx | 10 +- 6 files changed, 95 insertions(+), 76 deletions(-) diff --git a/src/features/datamodel/DataModelsProvider.tsx b/src/features/datamodel/DataModelsProvider.tsx index 92f4f0cf3..cef6b30a3 100644 --- a/src/features/datamodel/DataModelsProvider.tsx +++ b/src/features/datamodel/DataModelsProvider.tsx @@ -23,7 +23,7 @@ import { } from 'src/features/datamodel/utils'; import { useLayouts } from 'src/features/form/layout/LayoutsContext'; import { useFormDataQuery } from 'src/features/formData/useFormDataQuery'; -import { useLaxInstanceAllDataElements, useLaxInstanceDataElements } from 'src/features/instance/InstanceContext'; +import { useLaxInstanceAllDataElementsNow, useLaxInstanceDataElements } from 'src/features/instance/InstanceContext'; import { MissingRolesError } from 'src/features/instantiate/containers/MissingRolesError'; import { useIsPdf } from 'src/hooks/useIsPdf'; import { isAxiosError } from 'src/utils/isAxiosError'; @@ -136,7 +136,7 @@ function DataModelsLoader() { const layouts = useLayouts(); const defaultDataType = useCurrentDataModelName(); const isStateless = useApplicationMetadata().isStatelessApp; - const dataElements = useLaxInstanceAllDataElements(); + const dataElements = useLaxInstanceAllDataElementsNow(); // Subform const overriddenDataElement = useTaskStore((state) => state.overriddenDataModelUuid); diff --git a/src/features/instance/InstanceContext.tsx b/src/features/instance/InstanceContext.tsx index 49edb8dde..b53d4a76f 100644 --- a/src/features/instance/InstanceContext.tsx +++ b/src/features/instance/InstanceContext.tsx @@ -44,64 +44,65 @@ export type ChangeInstanceData = (callback: (instance: IInstance | undefined) => type InstanceStoreProps = Pick; -const { Provider, useMemoSelector, useSelector, useLaxMemoSelector, useHasProvider } = createZustandContext({ - name: 'InstanceContext', - required: true, - initialCreateStore: (props: InstanceStoreProps) => - createStore((set) => ({ - ...props, - instanceId: `${props.partyId}/${props.instanceGuid}`, - data: undefined, - dataSources: null, - appendDataElement: (element) => - set((state) => { - if (!state.data) { - throw new Error('Cannot append data element when instance data is not set'); - } - const next = { ...state.data, data: [...state.data.data, element] }; - return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - }), - mutateDataElement: (elementId, mutator) => - set((state) => { - if (!state.data) { - throw new Error('Cannot mutate data element when instance data is not set'); - } - const next = { - ...state.data, - data: state.data.data.map((element) => (element.id === elementId ? mutator(element) : element)), - }; - return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - }), - removeDataElement: (elementId) => - set((state) => { - if (!state.data) { - throw new Error('Cannot remove data element when instance data is not set'); - } - const next = { ...state.data, data: state.data.data.filter((element) => element.id !== elementId) }; - return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - }), - changeData: (callback) => - set((state) => { - const next = callback(state.data); - const clean = cleanUpInstanceData(next); - if (clean && !deepEqual(state.data, clean)) { +const { Provider, useMemoSelector, useSelector, useLaxMemoSelector, useHasProvider, useStore, useLaxDelayedSelector } = + createZustandContext({ + name: 'InstanceContext', + required: true, + initialCreateStore: (props: InstanceStoreProps) => + createStore((set) => ({ + ...props, + instanceId: `${props.partyId}/${props.instanceGuid}`, + data: undefined, + dataSources: null, + appendDataElement: (element) => + set((state) => { + if (!state.data) { + throw new Error('Cannot append data element when instance data is not set'); + } + const next = { ...state.data, data: [...state.data.data, element] }; return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - } - return {}; - }), - reFetch: async () => { - throw new Error('reFetch not implemented yet'); - }, - setReFetch: (reFetch) => - set({ - reFetch: async () => { - const result = await reFetch(); - set((state) => ({ ...state, data: result.data, dataSources: buildInstanceDataSources(result.data) })); - return result; - }, - }), - })), -}); + }), + mutateDataElement: (elementId, mutator) => + set((state) => { + if (!state.data) { + throw new Error('Cannot mutate data element when instance data is not set'); + } + const next = { + ...state.data, + data: state.data.data.map((element) => (element.id === elementId ? mutator(element) : element)), + }; + return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; + }), + removeDataElement: (elementId) => + set((state) => { + if (!state.data) { + throw new Error('Cannot remove data element when instance data is not set'); + } + const next = { ...state.data, data: state.data.data.filter((element) => element.id !== elementId) }; + return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; + }), + changeData: (callback) => + set((state) => { + const next = callback(state.data); + const clean = cleanUpInstanceData(next); + if (clean && !deepEqual(state.data, clean)) { + return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; + } + return {}; + }), + reFetch: async () => { + throw new Error('reFetch not implemented yet'); + }, + setReFetch: (reFetch) => + set({ + reFetch: async () => { + const result = await reFetch(); + set((state) => ({ ...state, data: result.data, dataSources: buildInstanceDataSources(result.data) })); + return result; + }, + }), + })), + }); // Also used for prefetching @see appPrefetcher.ts export function useInstanceDataQueryDef( @@ -219,8 +220,6 @@ export const useLaxInstanceId = () => useLaxInstance((state) => state.instanceId export const useLaxInstanceData = (selector: (data: IInstance) => U) => useLaxInstance((state) => (state.data ? selector(state.data) : undefined)); export const useLaxInstanceAllDataElements = () => useLaxInstance((state) => state.data?.data) ?? emptyArray; -export const useLaxInstanceDataElements = (dataType: string | undefined) => - useLaxInstance((state) => state.data?.data.filter((d) => d.dataType === dataType)) ?? emptyArray; export const useLaxInstanceStatus = () => useLaxInstance((state) => state.data?.status); export const useLaxAppendDataElement = () => useLaxInstance((state) => state.appendDataElement); export const useLaxMutateDataElement = () => useLaxInstance((state) => state.mutateDataElement); @@ -229,6 +228,24 @@ export const useLaxInstanceDataSources = () => useLaxInstance((state) => state.d export const useLaxChangeInstance = (): ChangeInstanceData | undefined => useLaxInstance((state) => state.changeData); export const useHasInstance = () => useHasProvider(); +/** Beware that in later versions, this will re-render your component after every save, as + * the backend sends us updated instance data */ +export const useLaxInstanceDataElements = (dataType: string | undefined) => + useLaxInstance((state) => state.data?.data.filter((d) => d.dataType === dataType)) ?? emptyArray; + +export type DataElementSelector = (selector: (data: IData[]) => U, deps: unknown[]) => U | typeof ContextNotProvided; +export const useLaxDataElementsSelector = (): DataElementSelector => + useLaxDelayedSelector({ + mode: 'innerSelector', + makeArgs: (state) => [state.data?.data ?? emptyArray], + }); + +/** Like useLaxInstanceAllDataElements, but will never re-render when the data changes */ +export const useLaxInstanceAllDataElementsNow = () => { + const store = useStore(); + return store.getState().data?.data ?? emptyArray; +}; + export const useStrictInstanceRefetch = () => useSelector((state) => state.reFetch); export const useStrictInstanceId = () => useSelector((state) => state.instanceId); export const useStrictAppendDataElement = () => useSelector((state) => state.appendDataElement); diff --git a/src/features/validation/index.ts b/src/features/validation/index.ts index 585d4154e..a30159663 100644 --- a/src/features/validation/index.ts +++ b/src/features/validation/index.ts @@ -1,11 +1,11 @@ import type { ApplicationMetadata } from 'src/features/applicationMetadata/types'; import type { AttachmentsSelector } from 'src/features/attachments/AttachmentsStorePlugin'; import type { Expression, ExprValToActual } from 'src/features/expressions/types'; +import type { DataElementSelector } from 'src/features/instance/InstanceContext'; import type { TextReference, ValidLangParam } from 'src/features/language/useLanguage'; import type { DataElementHasErrorsSelector } from 'src/features/validation/validationContext'; import type { FormDataSelector } from 'src/layout'; import type { ILayoutSets } from 'src/layout/common.generated'; -import type { IData } from 'src/types/shared'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { NodeDataSelector } from 'src/utils/layout/NodesContext'; @@ -225,7 +225,7 @@ export type ValidationDataSources = { attachmentsSelector: AttachmentsSelector; nodeDataSelector: NodeDataSelector; applicationMetadata: ApplicationMetadata; - dataElements: IData[]; + dataElementsSelector: DataElementSelector; layoutSets: ILayoutSets; dataElementHasErrorsSelector: DataElementHasErrorsSelector; }; diff --git a/src/features/validation/nodeValidation/useNodeValidation.ts b/src/features/validation/nodeValidation/useNodeValidation.ts index 4e63545ab..f9d7af244 100644 --- a/src/features/validation/nodeValidation/useNodeValidation.ts +++ b/src/features/validation/nodeValidation/useNodeValidation.ts @@ -5,7 +5,7 @@ import { useAttachmentsSelector } from 'src/features/attachments/hooks'; import { DataModels } from 'src/features/datamodel/DataModelsProvider'; import { useLayoutSets } from 'src/features/form/layoutSets/LayoutSetsProvider'; import { FD } from 'src/features/formData/FormDataWrite'; -import { useLaxInstanceAllDataElements } from 'src/features/instance/InstanceContext'; +import { useLaxDataElementsSelector } from 'src/features/instance/InstanceContext'; import { useCurrentLanguage } from 'src/features/language/LanguageProvider'; import { Validation } from 'src/features/validation/validationContext'; import { implementsValidateComponent, implementsValidateEmptyField } from 'src/layout'; @@ -85,7 +85,7 @@ function useValidationDataSources(): ValidationDataSources { const currentLanguage = useCurrentLanguage(); const nodeSelector = NodesInternal.useNodeDataSelector(); const applicationMetadata = useApplicationMetadata(); - const dataElements = useLaxInstanceAllDataElements(); + const dataElementsSelector = useLaxDataElementsSelector(); const layoutSets = useLayoutSets(); const dataElementHasErrorsSelector = Validation.useDataElementHasErrorsSelector(); @@ -97,7 +97,7 @@ function useValidationDataSources(): ValidationDataSources { currentLanguage, nodeDataSelector: nodeSelector, applicationMetadata, - dataElements, + dataElementsSelector, layoutSets, dataElementHasErrorsSelector, }), @@ -108,7 +108,7 @@ function useValidationDataSources(): ValidationDataSources { currentLanguage, nodeSelector, applicationMetadata, - dataElements, + dataElementsSelector, layoutSets, dataElementHasErrorsSelector, ], diff --git a/src/layout/AttachmentList/AttachmentListComponent.tsx b/src/layout/AttachmentList/AttachmentListComponent.tsx index 398a2eb8f..f81f70845 100644 --- a/src/layout/AttachmentList/AttachmentListComponent.tsx +++ b/src/layout/AttachmentList/AttachmentListComponent.tsx @@ -1,8 +1,8 @@ -import React, { useMemo } from 'react'; +import React from 'react'; import { AltinnAttachment } from 'src/components/atoms/AltinnAttachment'; import { useApplicationMetadata } from 'src/features/applicationMetadata/ApplicationMetadataProvider'; -import { useLaxInstanceAllDataElements } from 'src/features/instance/InstanceContext'; +import { useLaxInstanceData } from 'src/features/instance/InstanceContext'; import { useLaxProcessData } from 'src/features/instance/ProcessContext'; import { ComponentStructureWrapper } from 'src/layout/ComponentStructureWrapper'; import { DataTypeReference, filterDisplayPdfAttachments, getDisplayAttachments } from 'src/utils/attachmentsUtils'; @@ -15,12 +15,12 @@ export type IAttachmentListProps = PropsFromGenericComponent<'AttachmentList'>; const emptyDataTypeArray: IDataType[] = []; export function AttachmentListComponent({ node }: IAttachmentListProps) { - const instanceData = useLaxInstanceAllDataElements(); const currentTaskId = useLaxProcessData()?.currentTask?.elementId; const dataTypes = useApplicationMetadata().dataTypes ?? emptyDataTypeArray; const { dataTypeIds, textResourceBindings } = useNodeItem(node); - const attachments = useMemo(() => { + const attachments = useLaxInstanceData((data) => { + const instanceData = data.data ?? []; const allowedTypes = new Set(dataTypeIds ?? []); const includePdf = allowedTypes.has(DataTypeReference.RefDataAsPdf) || allowedTypes.has(DataTypeReference.IncludeAll); @@ -54,7 +54,7 @@ export function AttachmentListComponent({ node }: IAttachmentListProps) { const otherAttachments = getDisplayAttachments(attachments); return [...pdfAttachments, ...otherAttachments]; - }, [currentTaskId, dataTypes, instanceData, dataTypeIds]); + }); return ( diff --git a/src/layout/Subform/index.tsx b/src/layout/Subform/index.tsx index 5c6cca1bc..2c814e843 100644 --- a/src/layout/Subform/index.tsx +++ b/src/layout/Subform/index.tsx @@ -86,7 +86,7 @@ export class Subform extends SubformDef implements ValidateComponent<'Subform'>, node: LayoutNode<'Subform'>, { applicationMetadata, - dataElements, + dataElementsSelector, nodeDataSelector, layoutSets, dataElementHasErrorsSelector, @@ -107,8 +107,8 @@ export class Subform extends SubformDef implements ValidateComponent<'Subform'>, const validations: ComponentValidation[] = []; - const elements = dataElements.filter((x) => x.dataType === targetType); - const numDataElements = elements?.length ?? 0; + const elements = dataElementsSelector((d) => d.filter((x) => x.dataType === targetType), [targetType]); + const numDataElements = Array.isArray(elements) ? elements.length : 0; const { minCount, maxCount } = dataTypeDefinition; if (minCount > 0 && numDataElements < minCount) { @@ -129,7 +129,9 @@ export class Subform extends SubformDef implements ValidateComponent<'Subform'>, }); } - const subformIdsWithError = elements?.map((dE) => dE.id).filter((id) => dataElementHasErrorsSelector(id)); + const subformIdsWithError = Array.isArray(elements) + ? elements?.map((dE) => dE.id).filter((id) => dataElementHasErrorsSelector(id)) + : []; if (subformIdsWithError?.length) { const validation: SubformValidation = { subformDataElementIds: subformIdsWithError, From a40e0fa219add85e582f7184f0e919a526eaecbe Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Tue, 5 Nov 2024 11:18:44 +0100 Subject: [PATCH 3/3] Lax was not lax --- src/features/instance/InstanceContext.tsx | 128 ++++++++++++---------- 1 file changed, 69 insertions(+), 59 deletions(-) diff --git a/src/features/instance/InstanceContext.tsx b/src/features/instance/InstanceContext.tsx index b53d4a76f..f5e8d01a0 100644 --- a/src/features/instance/InstanceContext.tsx +++ b/src/features/instance/InstanceContext.tsx @@ -44,65 +44,72 @@ export type ChangeInstanceData = (callback: (instance: IInstance | undefined) => type InstanceStoreProps = Pick; -const { Provider, useMemoSelector, useSelector, useLaxMemoSelector, useHasProvider, useStore, useLaxDelayedSelector } = - createZustandContext({ - name: 'InstanceContext', - required: true, - initialCreateStore: (props: InstanceStoreProps) => - createStore((set) => ({ - ...props, - instanceId: `${props.partyId}/${props.instanceGuid}`, - data: undefined, - dataSources: null, - appendDataElement: (element) => - set((state) => { - if (!state.data) { - throw new Error('Cannot append data element when instance data is not set'); - } - const next = { ...state.data, data: [...state.data.data, element] }; +const { + Provider, + useMemoSelector, + useSelector, + useLaxMemoSelector, + useHasProvider, + useLaxStore, + useLaxDelayedSelector, +} = createZustandContext({ + name: 'InstanceContext', + required: true, + initialCreateStore: (props: InstanceStoreProps) => + createStore((set) => ({ + ...props, + instanceId: `${props.partyId}/${props.instanceGuid}`, + data: undefined, + dataSources: null, + appendDataElement: (element) => + set((state) => { + if (!state.data) { + throw new Error('Cannot append data element when instance data is not set'); + } + const next = { ...state.data, data: [...state.data.data, element] }; + return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; + }), + mutateDataElement: (elementId, mutator) => + set((state) => { + if (!state.data) { + throw new Error('Cannot mutate data element when instance data is not set'); + } + const next = { + ...state.data, + data: state.data.data.map((element) => (element.id === elementId ? mutator(element) : element)), + }; + return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; + }), + removeDataElement: (elementId) => + set((state) => { + if (!state.data) { + throw new Error('Cannot remove data element when instance data is not set'); + } + const next = { ...state.data, data: state.data.data.filter((element) => element.id !== elementId) }; + return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; + }), + changeData: (callback) => + set((state) => { + const next = callback(state.data); + const clean = cleanUpInstanceData(next); + if (clean && !deepEqual(state.data, clean)) { return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - }), - mutateDataElement: (elementId, mutator) => - set((state) => { - if (!state.data) { - throw new Error('Cannot mutate data element when instance data is not set'); - } - const next = { - ...state.data, - data: state.data.data.map((element) => (element.id === elementId ? mutator(element) : element)), - }; - return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - }), - removeDataElement: (elementId) => - set((state) => { - if (!state.data) { - throw new Error('Cannot remove data element when instance data is not set'); - } - const next = { ...state.data, data: state.data.data.filter((element) => element.id !== elementId) }; - return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - }), - changeData: (callback) => - set((state) => { - const next = callback(state.data); - const clean = cleanUpInstanceData(next); - if (clean && !deepEqual(state.data, clean)) { - return { ...state, data: next, dataSources: buildInstanceDataSources(next) }; - } - return {}; - }), - reFetch: async () => { - throw new Error('reFetch not implemented yet'); - }, - setReFetch: (reFetch) => - set({ - reFetch: async () => { - const result = await reFetch(); - set((state) => ({ ...state, data: result.data, dataSources: buildInstanceDataSources(result.data) })); - return result; - }, - }), - })), - }); + } + return {}; + }), + reFetch: async () => { + throw new Error('reFetch not implemented yet'); + }, + setReFetch: (reFetch) => + set({ + reFetch: async () => { + const result = await reFetch(); + set((state) => ({ ...state, data: result.data, dataSources: buildInstanceDataSources(result.data) })); + return result; + }, + }), + })), +}); // Also used for prefetching @see appPrefetcher.ts export function useInstanceDataQueryDef( @@ -242,7 +249,10 @@ export const useLaxDataElementsSelector = (): DataElementSelector => /** Like useLaxInstanceAllDataElements, but will never re-render when the data changes */ export const useLaxInstanceAllDataElementsNow = () => { - const store = useStore(); + const store = useLaxStore(); + if (store === ContextNotProvided) { + return emptyArray; + } return store.getState().data?.data ?? emptyArray; };