Skip to content

Commit a66d212

Browse files
olemartinorgOle Martin Handeland
andauthored
Improving performance when having lots of components (#3552)
* Adding a lightweight commit/queue system, as performance got significantly when having many components/rows * Removing logging * Fixing pagination bugs * Refactoring to try to get RepeatingGroupTableRow to not do _everything_ and make it not re-render when any row changes * Refactoring RepeatingGroupContext to get rid of the nested context. Claude did most of this. This seems to have a drastic effect on rendering RepeatingGroup, since the context will no longer update and cause ripples in the render tree. * getRecursiveValidations() looped through every node, but that will just be slower the more nodes you get. Since this a very hot path, it's worth writing some more code and using layoutLookups to more efficiently find every child id without iteration. * Unit tests don't have react-compiler to lean on, so auto-memoization doesn't work there. We still need to memoize some things the old way. * Ugh, I had to bring back useWaitUntilReady() to make it wait for validation commits to be stored in nodeData again. This failed some tests, but luckily it can be kept a bit simpler than it used to be. * Some shared expression tests seems to fail because of too many window.logError calls. I have no idea why, but memoizing a little helped. * Cleaning up inconsistent aria labels in repeating groups * Getting deep validations didn't work exactly as expected after the rewrite, as some nested components were left out (failing a test). * It seems like the readyForPrint component isn't always rendered now, and it makes sense since the loader isn't visible after changing the layout any longer either. * Waiting until 456 is saved before closing, as the validation might not be found until a little bit after that. * Removing indexing bug workaround * Preventing re-rendering of the entire ProvideGlobalContext component, which caused performance issues. Commits now only re-render a child component that commits during useLayoutEffect() as before. * This was a sneaky one! When changing layouts, for example via cy.changeLayout(), the loader would render briefly, and umount <AutoCommit />. This would cause the triggerAutoCommit() function to no longer update the correct component from then on, ruining the auto-commit functionality and breaking stuff after it. --------- Co-authored-by: Ole Martin Handeland <[email protected]>
1 parent ee23503 commit a66d212

25 files changed

+798
-503
lines changed

src/features/attachments/StoreAttachmentsInNode.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ function StoreAttachmentsInNodeWorker() {
5353
deepEqual('attachments' in data ? data.attachments : undefined, attachments),
5454
);
5555

56-
const setNodeProp = NodesInternal.useSetNodeProp();
56+
const setNodeProp = GeneratorInternal.useSetNodeProp();
5757
useEffect(() => {
5858
!hasBeenSet && setNodeProp({ nodeId: parent.indexedId!, prop: 'attachments', value: attachments });
5959
}, [attachments, hasBeenSet, parent.indexedId, setNodeProp]);

src/features/validation/StoreValidationsInNode.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ function useStoreValidations(baseComponentId: string) {
4848
(data) => !deepEqual('validations' in data ? data.validations : undefined, validations),
4949
);
5050

51-
const setNodeProp = NodesInternal.useSetNodeProp();
51+
const setNodeProp = GeneratorInternal.useSetNodeProp();
5252
useEffect(() => {
5353
shouldSetValidations && setNodeProp({ nodeId: indexedId, prop: 'validations', value: validations });
5454
}, [indexedId, setNodeProp, shouldSetValidations, validations]);

src/features/validation/ValidationStorePlugin.tsx

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ export class ValidationStorePlugin extends NodeDataPlugin<ValidationStorePluginC
154154
const lookups = useLayoutLookups();
155155
return store.useMemoSelector((state) => {
156156
const { baseComponentId } = splitDashedKey(indexedId);
157-
return getRecursiveValidations({
157+
const output: NodeRefValidation[] = [];
158+
getRecursiveValidations({
158159
state,
159160
id: indexedId,
160161
baseId: baseComponentId,
@@ -163,7 +164,10 @@ export class ValidationStorePlugin extends NodeDataPlugin<ValidationStorePluginC
163164
includeSelf,
164165
restriction,
165166
lookups,
167+
baseToIndexedMap: makeComponentIdIndex(state),
168+
output,
166169
});
170+
return output;
167171
});
168172
},
169173
useValidationsSelector: () => {
@@ -340,38 +344,69 @@ function getValidations({
340344
}
341345

342346
interface GetDeepValidationsProps extends GetValidationsProps {
347+
output: NodeRefValidation[];
343348
includeSelf: boolean;
344349
restriction?: number | undefined;
350+
baseToIndexedMap: Map<string, string[]>;
345351
}
346352

347-
export function getRecursiveValidations(props: GetDeepValidationsProps): NodeRefValidation[] {
348-
const out: NodeRefValidation[] = [];
349-
353+
export function getRecursiveValidations(props: GetDeepValidationsProps) {
350354
if (props.includeSelf) {
351355
const nodeValidations = getValidations(props);
352356
for (const validation of nodeValidations) {
353-
out.push({ ...validation, nodeId: props.id, baseComponentId: props.baseId });
357+
props.output.push({ ...validation, nodeId: props.id, baseComponentId: props.baseId });
354358
}
355359
}
356360

357-
const children = Object.values(props.state.nodeData)
358-
.filter(
359-
(nodeData) =>
360-
nodeData.parentId === props.id && (props.restriction === undefined || props.restriction === nodeData.rowIndex),
361-
)
362-
.map((nodeData) => nodeData.id);
361+
for (const child of getChildren(props)) {
362+
getRecursiveValidations({
363+
...props,
364+
id: child.id,
365+
baseId: child.baseId,
366+
367+
// Restriction and includeSelf should only be applied to the first level (not recursively)
368+
restriction: undefined,
369+
includeSelf: true,
370+
});
371+
}
372+
}
373+
374+
function getChildren(props: GetDeepValidationsProps): { id: string; baseId: string }[] {
375+
const children: { id: string; baseId: string }[] = [];
376+
if (!props.lookups) {
377+
return children;
378+
}
379+
380+
const { depth } = splitDashedKey(props.id);
381+
const parentSuffix = depth.length ? `-${depth.join('-')}` : '';
382+
const childBaseIds = props.lookups.componentToChildren[props.baseId] ?? [];
383+
for (const childBaseId of childBaseIds) {
384+
const lookForSuffix = props.restriction === undefined ? parentSuffix : `${parentSuffix}-${props.restriction}`;
385+
const childId = `${childBaseId}${lookForSuffix}`;
386+
387+
for (const idToCheck of props.baseToIndexedMap.get(childBaseId) ?? []) {
388+
const childData = props.state.nodeData[idToCheck];
389+
if (!childData || !idToCheck.startsWith(childId)) {
390+
continue;
391+
}
392+
children.push({ id: childData.id, baseId: childData.baseId });
393+
}
394+
}
363395

364-
for (const id of children) {
365-
out.push(
366-
...getRecursiveValidations({
367-
...props,
368-
id,
396+
return children;
397+
}
369398

370-
// Restriction and includeSelf should only be applied to the first level (not recursively)
371-
restriction: undefined,
372-
includeSelf: true,
373-
}),
374-
);
399+
export function makeComponentIdIndex(state: NodesContext) {
400+
const out = new Map<string, string[]>();
401+
for (const id of Object.keys(state.nodeData)) {
402+
const data = state.nodeData[id];
403+
if (!data) {
404+
continue;
405+
}
406+
const baseId = data.baseId;
407+
const children = out.get(baseId) ?? [];
408+
children.push(id);
409+
out.set(baseId, children);
375410
}
376411

377412
return out;
Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import { useCallback } from 'react';
2-
31
import { useLayoutLookups } from 'src/features/form/layout/LayoutsContext';
42
import { getVisibilityMask } from 'src/features/validation/utils';
53
import { Validation } from 'src/features/validation/validationContext';
6-
import { getRecursiveValidations } from 'src/features/validation/ValidationStorePlugin';
4+
import { getRecursiveValidations, makeComponentIdIndex } from 'src/features/validation/ValidationStorePlugin';
75
import { useEffectEvent } from 'src/hooks/useEffectEvent';
86
import { useComponentIdMutator } from 'src/utils/layout/DataModelLocation';
97
import { NodesInternal } from 'src/utils/layout/NodesContext';
8+
import type { NodeRefValidation } from 'src/features/validation';
109
import type { AllowedValidationMasks } from 'src/layout/common.generated';
1110

1211
/**
@@ -18,14 +17,15 @@ export function useOnGroupCloseValidation() {
1817
const validating = Validation.useValidating();
1918
const nodeStore = NodesInternal.useStore();
2019
const lookups = useLayoutLookups();
21-
const idMutator = useComponentIdMutator();
20+
const idMutator = useComponentIdMutator(true);
2221

2322
/* Ensures the callback will have the latest state */
2423
const callback = useEffectEvent(
2524
(baseComponentId: string, restriction: number | undefined, masks: AllowedValidationMasks): boolean => {
2625
const mask = getVisibilityMask(masks);
2726
const state = nodeStore.getState();
28-
const nodesWithErrors = getRecursiveValidations({
27+
const errors: NodeRefValidation[] = [];
28+
getRecursiveValidations({
2929
id: idMutator(baseComponentId),
3030
baseId: baseComponentId,
3131
includeHidden: false,
@@ -35,7 +35,11 @@ export function useOnGroupCloseValidation() {
3535
mask,
3636
state,
3737
lookups,
38-
}).map((v) => v.nodeId);
38+
baseToIndexedMap: makeComponentIdIndex(state),
39+
output: errors,
40+
});
41+
42+
const nodesWithErrors = errors.map((v) => v.nodeId);
3943

4044
if (nodesWithErrors.length > 0) {
4145
setNodeVisibility(nodesWithErrors, mask);
@@ -46,11 +50,8 @@ export function useOnGroupCloseValidation() {
4650
},
4751
);
4852

49-
return useCallback(
50-
async (baseComponentId: string, restriction: number | undefined, masks: AllowedValidationMasks) => {
51-
await validating();
52-
return callback(baseComponentId, restriction, masks);
53-
},
54-
[callback, validating],
55-
);
53+
return async (baseComponentId: string, restriction: number | undefined, masks: AllowedValidationMasks) => {
54+
await validating();
55+
return callback(baseComponentId, restriction, masks);
56+
};
5657
}

src/features/validation/validationContext.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { SchemaValidation } from 'src/features/validation/schemaValidation/Schem
3333
import { hasValidationErrors, mergeFieldValidations, selectValidations } from 'src/features/validation/utils';
3434
import { useAsRef } from 'src/hooks/useAsRef';
3535
import { useWaitForState } from 'src/hooks/useWaitForState';
36+
import { NodesInternal } from 'src/utils/layout/NodesContext';
3637

3738
interface Internals {
3839
individualValidations: {
@@ -173,6 +174,7 @@ export function ValidationProvider({ children }: PropsWithChildren) {
173174
}
174175

175176
function useWaitForValidation(): WaitForValidation {
177+
const waitForNodesReady = NodesInternal.useWaitUntilReady();
176178
const waitForSave = FD.useWaitForSave();
177179
const waitForState = useWaitForState<ValidationsProcessedLast['initial'], ValidationContext & Internals>(useStore());
178180
const hasPendingAttachments = useHasPendingAttachments();
@@ -195,6 +197,7 @@ function useWaitForValidation(): WaitForValidation {
195197
await waitForAttachments((state) => !state);
196198

197199
// Wait until we've saved changed to backend, and we've processed the backend validations we got from that save
200+
await waitForNodesReady();
198201
const validationsFromSave = await waitForSave(forceSave);
199202
// If validationsFromSave is not defined, we check if initial validations are done processing
200203
await waitForState(async (state) => {
@@ -211,12 +214,14 @@ function useWaitForValidation(): WaitForValidation {
211214

212215
return false;
213216
});
217+
await waitForNodesReady();
214218
},
215219
[
216220
enabled,
217221
getCachedInitialValidations,
218222
hasWritableDataTypes,
219223
waitForAttachments,
224+
waitForNodesReady,
220225
waitForNodesToValidate,
221226
waitForSave,
222227
waitForState,

src/layout/RepeatingGroup/Container/RepeatingGroupContainer.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { AllComponentValidations } from 'src/features/validation/ComponentValida
1414
import { RepeatingGroupsEditContainer } from 'src/layout/RepeatingGroup/EditContainer/RepeatingGroupsEditContainer';
1515
import { RepeatingGroupPagination } from 'src/layout/RepeatingGroup/Pagination/RepeatingGroupPagination';
1616
import {
17-
useRepeatingGroup,
17+
RepGroupContext,
1818
useRepeatingGroupComponentId,
1919
useRepeatingGroupPagination,
2020
useRepeatingGroupRowState,
@@ -181,7 +181,8 @@ export const alignStyle = (align: ButtonPosition): React.CSSProperties => {
181181
function AddButton() {
182182
const { lang, langAsString } = useLanguage();
183183
const { triggerFocus } = useRepeatingGroupsFocusContext();
184-
const { addRow, baseComponentId } = useRepeatingGroup();
184+
const baseComponentId = useRepeatingGroupComponentId();
185+
const addRow = RepGroupContext.useAddRow();
185186
const { visibleRows } = useRepeatingGroupRowState();
186187
const { editingAll, editingNone, isEditingAnyRow, currentlyAddingRow } = useRepeatingGroupSelector((state) => ({
187188
editingAll: state.editingAll,

src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContainer.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { ALTINN_ROW_ID } from 'src/features/formData/types';
99
import { RepeatingGroupsEditContainer } from 'src/layout/RepeatingGroup/EditContainer/RepeatingGroupsEditContainer';
1010
import {
1111
RepeatingGroupProvider,
12-
useRepeatingGroup,
12+
RepGroupContext,
1313
useRepeatingGroupRowState,
1414
useRepeatingGroupSelector,
1515
} from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext';
@@ -134,7 +134,7 @@ function TestRenderer() {
134134
const editingId = useRepeatingGroupSelector((state) => state.editingId);
135135
const { visibleRows } = useRepeatingGroupRowState();
136136
const editingIndex = visibleRows.find((r) => r.uuid === editingId)?.index;
137-
const { openForEditing } = useRepeatingGroup();
137+
const openForEditing = RepGroupContext.useOpenForEditing();
138138

139139
if (editingIndex === undefined || editingId === undefined) {
140140
return (

src/layout/RepeatingGroup/EditContainer/RepeatingGroupsEditContainer.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
useRepeatingGroupEdit,
1515
} from 'src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext';
1616
import {
17-
useRepeatingGroup,
17+
RepGroupContext,
1818
useRepeatingGroupComponentId,
1919
useRepeatingGroupRowState,
2020
} from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext';
@@ -58,7 +58,10 @@ function RepeatingGroupsEditContainerInternal({
5858
}: IRepeatingGroupsEditContainer & {
5959
row: RepGroupRow;
6060
}): JSX.Element | null {
61-
const { baseComponentId, closeForEditing, deleteRow, openNextForEditing, isDeleting } = useRepeatingGroup();
61+
const baseComponentId = useRepeatingGroupComponentId();
62+
const closeForEditing = RepGroupContext.useCloseForEditing();
63+
const deleteRow = RepGroupContext.useDeleteRow();
64+
const openNextForEditing = RepGroupContext.useOpenNextForEditing();
6265
const { visibleRows } = useRepeatingGroupRowState();
6366
const childIds = RepGroupHooks.useChildIdsWithMultiPage(baseComponentId);
6467

@@ -73,6 +76,7 @@ function RepeatingGroupsEditContainerInternal({
7376

7477
const { multiPageEnabled, multiPageIndex, nextMultiPage, prevMultiPage, hasNextMultiPage, hasPrevMultiPage } =
7578
useRepeatingGroupEdit();
79+
const isDeleting = RepGroupContext.useIsDeletingRow(editId);
7680
const id = useIndexedId(baseComponentId);
7781
const rowWithExpressions = RepGroupHooks.useRowWithExpressions(baseComponentId, { uuid: row.uuid });
7882
const textsForRow = rowWithExpressions?.textResourceBindings;
@@ -127,7 +131,7 @@ function RepeatingGroupsEditContainerInternal({
127131
<Button
128132
variant='tertiary'
129133
color='danger'
130-
disabled={isDeleting(editId)}
134+
disabled={isDeleting}
131135
onClick={() => deleteRow({ index: row.index, uuid: row.uuid })}
132136
data-testid='delete-button'
133137
>

src/layout/RepeatingGroup/Pagination/RepeatingGroupPagination.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import { useLanguage } from 'src/features/language/useLanguage';
88
import { useIsMini, useIsMobile, useIsMobileOrTablet } from 'src/hooks/useDeviceWidths';
99
import classes from 'src/layout/RepeatingGroup/Pagination/RepeatingGroupPagination.module.css';
1010
import {
11-
useRepeatingGroup,
11+
RepGroupContext,
12+
useRepeatingGroupComponentId,
1213
useRepeatingGroupPagination,
1314
useRepeatingGroupRowState,
1415
} from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext';
@@ -33,7 +34,8 @@ export function RepeatingGroupPagination(props: RepeatingGroupPaginationProps) {
3334
return <RGPagination {...props} />;
3435
}
3536
function RGPagination({ inTable = true }: RepeatingGroupPaginationProps) {
36-
const { changePage, baseComponentId } = useRepeatingGroup();
37+
const baseComponentId = useRepeatingGroupComponentId();
38+
const changePage = RepGroupContext.useChangePage();
3739
const { hasPagination, rowsPerPage, currentPage, totalPages } = useRepeatingGroupPagination();
3840
const pagesWithErrors = usePagesWithErrors(rowsPerPage, baseComponentId);
3941
const isTablet = useIsMobileOrTablet();
@@ -149,6 +151,10 @@ function PaginationComponent({
149151
? `${langAsString('general.edit_alt_error')}: ${langAsString('general.page_number', [page])}`
150152
: langAsString('general.page_number', [page]);
151153

154+
if (typeof page !== 'number') {
155+
return <Pagination.Item key={itemKey} />;
156+
}
157+
152158
return (
153159
<Pagination.Item key={itemKey}>
154160
<Pagination.Button

src/layout/RepeatingGroup/Providers/OpenByDefaultProvider.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { FD } from 'src/features/formData/FormDataWrite';
99
import { ALTINN_ROW_ID } from 'src/features/formData/types';
1010
import {
1111
RepeatingGroupProvider,
12-
useRepeatingGroup,
12+
RepGroupContext,
1313
useRepeatingGroupRowState,
1414
useRepeatingGroupSelector,
1515
} from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext';
@@ -35,7 +35,7 @@ describe('openByDefault', () => {
3535
editingId: state.editingId,
3636
addingIds: state.addingIds,
3737
}));
38-
const { deleteRow } = useRepeatingGroup();
38+
const deleteRow = RepGroupContext.useDeleteRow();
3939
const { visibleRows, hiddenRows } = useRepeatingGroupRowState();
4040

4141
const data = FD.useDebouncedPick({ field: 'MyGroup', dataType: defaultDataTypeMock });

0 commit comments

Comments
 (0)