Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make useExpressionDataSources and useValidationDataSources cheaper #2720

Merged
merged 31 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ae21210
optimize hook usage in expression data sources
bjosttveit Nov 7, 2024
d97e439
add shallow object memo
bjosttveit Nov 7, 2024
e9132ac
some more incremental improvements
bjosttveit Nov 8, 2024
93c3c45
reduce ref hooks
bjosttveit Nov 8, 2024
a63a008
Merge branch 'main' into fix/perf#6
bjosttveit Nov 8, 2024
307dc07
trick react into finding host sibling faster :'(
bjosttveit Nov 8, 2024
5752991
Merge branch 'main' into fix/perf#6
bjosttveit Nov 12, 2024
94cdff0
multi-delayed-selector
bjosttveit Nov 13, 2024
c34025a
delayed selector class implementation
bjosttveit Nov 13, 2024
baa16e2
gather all selectors in expressiondatasrouces
bjosttveit Nov 13, 2024
6112e64
clean up
bjosttveit Nov 13, 2024
e40d79b
fix expression tests
bjosttveit Nov 13, 2024
fbd0b4d
fix memory leak (forgot to call unsubscribe)
bjosttveit Nov 14, 2024
16e6dd1
quickfix useNodes is undefined
bjosttveit Nov 14, 2024
94b91d6
Merge branch 'main' into fix/perf#6
bjosttveit Nov 14, 2024
bf34404
fix stale initial data when changing task
bjosttveit Nov 15, 2024
f0f0180
fix queued requests not getting committed after adding or removing no…
bjosttveit Nov 15, 2024
b28a697
fix stale initial data when changing layout in cypress
bjosttveit Nov 15, 2024
3855903
fix stale validations when changing layouts in cypress, no clue why t…
bjosttveit Nov 15, 2024
35287bb
use shared hook for remove node
bjosttveit Nov 15, 2024
3c89bde
Update GeneratorDataSources.tsx
bjosttveit Nov 15, 2024
2600561
Storing the nodes object (LayoutPages) outside the NodesContext zusta…
Nov 15, 2024
04bf917
Merge branch 'main' into fix/perf#6
Nov 18, 2024
3d53163
Fixes after merge from main
Nov 18, 2024
a5e78c0
apply optimizations for regular useExpressionDataSources and add bett…
bjosttveit Nov 18, 2024
a9f56a7
handle case where store is subscribed to and updates before uSES has …
bjosttveit Nov 18, 2024
45c82bf
Rewriting SubformWrapper to ensure FormProvider is not fully re-rende…
Nov 18, 2024
417d952
separate setDataElementId from setInitialData and remove annoying wor…
bjosttveit Nov 18, 2024
2cc1219
Making sure node.item is set again in case it disappears because of r…
Nov 18, 2024
0f7b07a
Merge remote-tracking branch 'origin/fix/perf#6' into fix/perf#6
Nov 18, 2024
453f319
Merge branch 'main' into fix/perf#6
bjosttveit Nov 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/core/contexts/hookContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React, { createContext, useContext } from 'react';
import type { PropsWithChildren } from 'react';

type HookContextProps = {
[name: string]: () => unknown;
};

/**
* This will store the result of any hook in a context and return a new hook for reading the context value.
* Why is this useful? React always reruns hooks on render, and if the hook does expensive calculations and
* is used a lot (e.g. for every node in the node generator), this will make things much faster,
* as reading from a plain context is very cheap. It is also much cheaper than subscribing directly to
* a zustand store. You shouldn't bother storing the result of another simple context here, as it will
* not be any faster.
*/
export function createHookContext<P extends HookContextProps>(
props: P,
): {
Provider: React.FC<PropsWithChildren>;
hooks: P;
} {
const data = Object.entries(props).map(([name, useHook]) => {
const Context = createContext<unknown>(null as unknown);
Context.displayName = name;
const useCtx = () => useContext(Context);
const Provider = ({ children }: PropsWithChildren) => {
const value = useHook();
return <Context.Provider value={value}>{children}</Context.Provider>;
};
return {
name,
useCtx,
Provider,
};
});

return {
Provider: ({ children }: PropsWithChildren) => (
<>
{data.reduce(
(innerProviders, { Provider }) => (
<Provider>{innerProviders}</Provider>
),
<>{children}</>,
)}
</>
),
hooks: Object.fromEntries(data.map(({ name, useCtx }) => [name, useCtx])) as P,
};
}
24 changes: 23 additions & 1 deletion src/core/contexts/zustandContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { StoreApi } from 'zustand';
import { ContextNotProvided, createContext } from 'src/core/contexts/context';
import { SelectorStrictness, useDelayedSelector } from 'src/hooks/delayedSelectors';
import type { CreateContextProps } from 'src/core/contexts/context';
import type { DSConfig, DSMode, DSReturn } from 'src/hooks/delayedSelectors';
import type { DSConfig, DSMode, DSProps, DSReturn } from 'src/hooks/delayedSelectors';

type ExtractFromStoreApi<T> = T extends StoreApi<infer U> ? Exclude<U, void> : never;

Expand Down Expand Up @@ -165,6 +165,26 @@ export function createZustandContext<Store extends StoreApi<Type>, Type = Extrac
deps,
});

const useDSProps = <Mode extends DSMode<Type>>(
mode: Mode,
deps?: unknown[],
): DSProps<DSConfig<Type, Mode, SelectorStrictness.throwWhenNotProvided>> => ({
store: useCtx(),
strictness: SelectorStrictness.throwWhenNotProvided,
mode,
deps,
});

const useLaxDSProps = <Mode extends DSMode<Type>>(
mode: Mode,
deps?: unknown[],
): DSProps<DSConfig<Type, Mode, SelectorStrictness.returnWhenNotProvided>> => ({
store: useLaxCtx(),
strictness: SelectorStrictness.returnWhenNotProvided,
mode,
deps,
});

return {
Provider: MyProvider,
useSelector,
Expand All @@ -175,6 +195,8 @@ export function createZustandContext<Store extends StoreApi<Type>, Type = Extrac
useLaxSelector,
useDelayedSelector: useDS,
useLaxDelayedSelector: useLaxDS,
useDelayedSelectorProps: useDSProps,
useLaxDelayedSelectorProps: useLaxDSProps,
useHasProvider,
useStore: useCtx,
useLaxStore: useLaxCtx,
Expand Down
30 changes: 20 additions & 10 deletions src/features/attachments/AttachmentsStorePlugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import type {
UploadedAttachment,
} from 'src/features/attachments/index';
import type { BackendValidationIssue } from 'src/features/validation';
import type { DSConfig, DSProps } from 'src/hooks/delayedSelectors';
import type { IDataModelBindingsList, IDataModelBindingsSimple } from 'src/layout/common.generated';
import type { CompWithBehavior } from 'src/layout/layout';
import type { IData } from 'src/types/shared';
Expand Down Expand Up @@ -83,6 +84,7 @@ export interface AttachmentsStorePluginConfig {

useAttachments: (node: FileUploaderNode) => IAttachment[];
useAttachmentsSelector: () => AttachmentsSelector;
useAttachmentsSelectorProps: () => DSProps<DSConfig>;
useWaitUntilUploaded: () => (node: FileUploaderNode, attachment: TemporaryAttachment) => Promise<IData | false>;

useHasPendingAttachments: () => boolean;
Expand Down Expand Up @@ -387,18 +389,15 @@ export class AttachmentsStorePlugin extends NodeDataPlugin<AttachmentsStorePlugi
useAttachmentsSelector() {
return store.useDelayedSelector({
mode: 'simple',
selector: (node: LayoutNode) => (state) => {
const nodeData = state.nodeData[node.id];
if (!nodeData) {
return emptyArray;
}
if (nodeData && 'attachments' in nodeData) {
return Object.values(nodeData.attachments).sort(sortAttachmentsByName);
}
return emptyArray;
},
selector: attachmentSelector,
}) satisfies AttachmentsSelector;
},
useAttachmentsSelectorProps() {
return store.useDelayedSelectorProps({
mode: 'simple',
selector: attachmentSelector,
});
},
useWaitUntilUploaded() {
const zustandStore = store.useStore();
const waitFor = useWaitForState<IData | false, NodesContext>(zustandStore);
Expand Down Expand Up @@ -496,6 +495,17 @@ function useAttachmentsUploadMutation() {
return useMutation(options);
}

const attachmentSelector = (node: LayoutNode) => (state: NodesContext) => {
const nodeData = state.nodeData[node.id];
if (!nodeData) {
return emptyArray;
}
if (nodeData && 'attachments' in nodeData) {
return Object.values(nodeData.attachments).sort(sortAttachmentsByName);
}
return emptyArray;
};

function useAttachmentsAddTagMutation() {
const { doAttachmentAddTag } = useAppMutations();
const instanceId = useLaxInstanceId();
Expand Down
3 changes: 2 additions & 1 deletion src/features/dataLists/useDataListQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { SortDirection } from '@digdir/design-system-react/dist/types/compo
import type { UseQueryResult } from '@tanstack/react-query';

import { useAppQueries } from 'src/core/contexts/AppQueriesProvider';
import { DataModels } from 'src/features/datamodel/DataModelsProvider';
import { FD } from 'src/features/formData/FormDataWrite';
import { useLaxInstanceId } from 'src/features/instance/InstanceContext';
import { useCurrentLanguage } from 'src/features/language/LanguageProvider';
Expand All @@ -29,7 +30,7 @@ export const useDataListQuery = (
const { fetchDataList } = useAppQueries();
const selectedLanguage = useCurrentLanguage();
const instanceId = useLaxInstanceId();
const mappingResult = FD.useMapping(mapping);
const mappingResult = FD.useMapping(mapping, DataModels.useDefaultDataType());
const { pageSize, pageNumber, sortColumn, sortDirection } = filter || {};

const url = getDataListsUrl({
Expand Down
62 changes: 42 additions & 20 deletions src/features/datamodel/DataModelsProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,20 @@ function initialCreateStore() {
writableDataTypes: null,
initialData: {},
dataElementIds: {},
initialValidations: null,
schemas: {},
schemaLookup: {},
expressionValidationConfigs: {},
error: null,

setDataTypes: (allDataTypes, writableDataTypes, defaultDataType, layoutSetId) => {
set(() => ({ allDataTypes, writableDataTypes, defaultDataType, layoutSetId }));
set(() => ({
allDataTypes,
writableDataTypes,
defaultDataType,
layoutSetId,
initialData: {},
dataElementIds: {},
}));
},
setInitialData: (dataType, initialData, dataElementId) => {
set((state) => ({
Expand Down Expand Up @@ -120,7 +126,7 @@ function initialCreateStore() {
}));
}

const { Provider, useSelector, useMemoSelector, useLaxMemoSelector, useSelectorAsRef } = createZustandContext({
const { Provider, useSelector, useLaxSelector, useSelectorAsRef } = createZustandContext({
name: 'DataModels',
required: true,
initialCreateStore,
Expand Down Expand Up @@ -187,24 +193,39 @@ function DataModelsLoader() {
setDataTypes(allValidDataTypes, writableDataTypes, defaultDataType, layoutSetId);
}, [applicationMetadata, defaultDataType, isStateless, layouts, setDataTypes, dataElements, layoutSetId]);

const initialDataDone = useSelector((state) => Object.keys(state.initialData));
const schemaDone = useSelector((state) => Object.keys(state.schemas));
const validationConfigDone = useSelector((state) => Object.keys(state.expressionValidationConfigs));

// We should load form data and schema for all referenced data models, schema is used for dataModelBinding validation which we want to do even if it is readonly
// We only need to load expression validation config for data types that are not readonly. Additionally, backend will error if we try to validate a model we are not supposed to
return (
<>
{allDataTypes?.map((dataType) => (
<React.Fragment key={dataType}>
{allDataTypes
?.filter((dataType) => !initialDataDone.includes(dataType))
.map((dataType) => (
<LoadInitialData
key={dataType}
dataType={dataType}
overrideDataElement={dataType === overriddenDataType ? overriddenDataElement : undefined}
/>
<LoadSchema dataType={dataType} />
</React.Fragment>
))}
{writableDataTypes?.map((dataType) => (
<React.Fragment key={dataType}>
<LoadExpressionValidationConfig dataType={dataType} />
</React.Fragment>
))}
))}
{allDataTypes
?.filter((dataType) => !schemaDone.includes(dataType))
.map((dataType) => (
<LoadSchema
key={dataType}
dataType={dataType}
/>
))}
{writableDataTypes
?.filter((dataType) => !validationConfigDone.includes(dataType))
.map((dataType) => (
<LoadExpressionValidationConfig
key={dataType}
dataType={dataType}
/>
))}
</>
);
}
Expand Down Expand Up @@ -325,12 +346,13 @@ const emptyArray = [];
export const DataModels = {
useFullStateRef: () => useSelectorAsRef((state) => state),

useLaxDefaultDataType: () => useLaxMemoSelector((state) => state.defaultDataType),
useDefaultDataType: () => useSelector((state) => state.defaultDataType),
useLaxDefaultDataType: () => useLaxSelector((state) => state.defaultDataType),

// The following hooks use emptyArray if the value is null, so cannot be used to determine whether or not the datamodels are finished loading
useReadableDataTypes: () => useMemoSelector((state) => state.allDataTypes ?? emptyArray),
useLaxReadableDataTypes: () => useLaxMemoSelector((state) => state.allDataTypes ?? emptyArray),
useWritableDataTypes: () => useMemoSelector((state) => state.writableDataTypes ?? emptyArray),
useReadableDataTypes: () => useSelector((state) => state.allDataTypes ?? emptyArray),
useLaxReadableDataTypes: () => useLaxSelector((state) => state.allDataTypes ?? emptyArray),
useWritableDataTypes: () => useSelector((state) => state.writableDataTypes ?? emptyArray),

useDataModelSchema: (dataType: string) => useSelector((state) => state.schemas[dataType]),

Expand All @@ -348,12 +370,12 @@ export const DataModels = {
useSelector((state) => state.expressionValidationConfigs[dataType]),

useDefaultDataElementId: () =>
useMemoSelector((state) => (state.defaultDataType ? state.dataElementIds[state.defaultDataType] : null)),
useSelector((state) => (state.defaultDataType ? state.dataElementIds[state.defaultDataType] : null)),

useDataElementIdForDataType: (dataType: string) => useMemoSelector((state) => state.dataElementIds[dataType]),
useDataElementIdForDataType: (dataType: string) => useSelector((state) => state.dataElementIds[dataType]),

useGetDataElementIdForDataType: () => {
const dataElementIds = useMemoSelector((state) => state.dataElementIds);
const dataElementIds = useSelector((state) => state.dataElementIds);
return useCallback((dataType: string) => dataElementIds[dataType], [dataElementIds]);
},

Expand Down
4 changes: 3 additions & 1 deletion src/features/expressions/shared-functions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { useExternalApis } from 'src/features/externalApi/useExternalApi';
import { fetchApplicationMetadata, fetchProcessState } from 'src/queries/queries';
import { renderWithNode } from 'src/test/renderWithProviders';
import { useEvalExpression } from 'src/utils/layout/generator/useEvalExpression';
import { useExpressionDataSources } from 'src/utils/layout/useExpressionDataSources';
import type { SharedTestFunctionContext } from 'src/features/expressions/shared';
import type { ExprValToActualOrExpr } from 'src/features/expressions/types';
import type { ExternalApisResult } from 'src/features/externalApi/useExternalApi';
Expand All @@ -25,7 +26,8 @@ import type { LayoutNode } from 'src/utils/layout/LayoutNode';
jest.mock('src/features/externalApi/useExternalApi');

function ExpressionRunner({ node, expression }: { node: LayoutNode; expression: ExprValToActualOrExpr<ExprVal.Any> }) {
const result = useEvalExpression(ExprVal.Any, node, expression, null);
const dataSources = useExpressionDataSources();
const result = useEvalExpression(ExprVal.Any, node, expression, null, dataSources);
return (
<>
<div data-testid='expr-result'>{JSON.stringify(result)}</div>
Expand Down
Loading
Loading