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

Removing more node traversal usages #3009

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions src/codegen/ComponentConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ export class ComponentConfig {
import: 'TraversalRestriction',
from: 'src/utils/layout/useNodeTraversal',
});
const LayoutNode = new CG.import({ import: 'LayoutNode', from: 'src/utils/layout/LayoutNode' });

const claimChildrenBody = childrenPlugins.map((plugin) =>
`${pluginRef(plugin)}.claimChildren({
Expand All @@ -429,7 +428,7 @@ export class ComponentConfig {
);

const isChildHiddenBody = childrenPlugins.map(
(plugin) => `${pluginRef(plugin)}.isChildHidden(state as any, childNode)`,
(plugin) => `${pluginRef(plugin)}.isChildHidden(state as any, childId)`,
);

additionalMethods.push(
Expand All @@ -439,7 +438,7 @@ export class ComponentConfig {
`pickDirectChildren(state: ${NodeData}<'${this.type}'>, restriction?: ${TraversalRestriction}) {
return [${pickDirectChildrenBody.join(', ')}];
}`,
`isChildHidden(state: ${NodeData}<'${this.type}'>, childNode: ${LayoutNode}) {
`isChildHidden(state: ${NodeData}<'${this.type}'>, childId: string) {
return [${isChildHiddenBody.join(', ')}].some((h) => h);
}`,
);
Expand All @@ -463,6 +462,8 @@ export class ComponentConfig {
const baseState: ${BaseNodeData}<'${this.type}'> = {
type: 'node',
pageKey: props.pageKey,
parentId: props.parentId,
depth: props.depth,
item: undefined,
layout: props.item,
hidden: undefined,
Expand Down
105 changes: 38 additions & 67 deletions src/components/form/Form.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useState } from 'react';
import React, { useEffect } from 'react';
import { Helmet } from 'react-helmet-async';

import { Flex } from 'src/app-components/Flex/Flex';
Expand Down Expand Up @@ -26,18 +26,17 @@ import {
import { useOnFormSubmitValidation } from 'src/features/validation/callbacks/onFormSubmitValidation';
import { useTaskErrors } from 'src/features/validation/selectors/taskErrors';
import { useCurrentView, useNavigatePage, useStartUrl } from 'src/hooks/useNavigatePage';
import { getComponentCapabilities } from 'src/layout';
import { GenericComponentById } from 'src/layout/GenericComponent';
import { extractBottomButtons } from 'src/utils/formLayout';
import { getPageTitle } from 'src/utils/getPageTitle';
import { NodesInternal, useGetPage, useNode } from 'src/utils/layout/NodesContext';
import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal';
import { NodesInternal, useNode } from 'src/utils/layout/NodesContext';
import type { NavigateToNodeOptions } from 'src/features/form/layout/NavigateToNode';
import type { AnyValidation, BaseValidation, NodeRefValidation } from 'src/features/validation';
import type { NodeData } from 'src/utils/layout/types';

interface FormState {
hasRequired: boolean;
mainIds: string[] | undefined;
mainIds: string[];
errorReportIds: string[];
formErrors: NodeRefValidation<AnyValidation<'error'>>[];
taskErrors: BaseValidation<'error'>[];
Expand All @@ -54,14 +53,7 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined
const appName = useAppName();
const appOwner = useAppOwner();
const { langAsString } = useLanguage();
const [formState, setFormState] = useState<FormState>({
hasRequired: false,
mainIds: undefined,
errorReportIds: [],
formErrors: [],
taskErrors: [],
});
const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = formState;
const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = useFormState(currentPageId);
const requiredFieldsMissing = NodesInternal.usePageHasVisibleRequiredValidations(currentPageId);

useRedirectToStoredPage();
Expand All @@ -86,15 +78,6 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined
return <FormFirstPage />;
}

if (mainIds === undefined) {
return (
<>
<ErrorProcessing setFormState={setFormState} />
<Loader reason='form-ids' />
</>
);
}

const hasSetCurrentPageId = langAsString(currentPageId) !== currentPageId;

if (!hasSetCurrentPageId) {
Expand All @@ -114,7 +97,6 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined
<Helmet>
<title>{`${getPageTitle(appName, hasSetCurrentPageId ? langAsString(currentPageId) : undefined, appOwner)}`}</title>
</Helmet>
<ErrorProcessing setFormState={setFormState} />
{hasRequired && (
<MessageBanner
error={requiredFieldsMissing}
Expand Down Expand Up @@ -213,64 +195,53 @@ function useSetExpandedWidth() {
}

const emptyArray = [];
interface ErrorProcessingProps {
setFormState: React.Dispatch<React.SetStateAction<FormState>>;
}

function nodeDataIsRequired(n: NodeData) {
const item = n.item;
return !!(item && 'required' in item && item.required === true);
}

/**
* Instead of re-rendering the entire Form component when any of this changes, we just report the
* state to the parent component.
*/
function ErrorProcessing({ setFormState }: ErrorProcessingProps) {
const currentPageId = useCurrentView();
const page = useGetPage(currentPageId);
const traversalSelector = useNodeTraversalSelector();
function useFormState(currentPageId: string | undefined): FormState {
const { formErrors, taskErrors } = useTaskErrors();
const hasErrors = Boolean(formErrors.length) || Boolean(taskErrors.length);

const topLevelNodeIds = traversalSelector(
(traverser) => {
if (!page) {
return emptyArray;
}
const [mainIds, errorReportIds] = NodesInternal.useMemoSelector((state) => {
const topLevelNodeData = Object.values(state.nodeData).filter(
(node) => node.pageKey === currentPageId && node.parentId === undefined,
);

const all = traverser.with(page).children();
return all.map((n) => n.id);
},
[currentPageId],
);
if (!hasErrors) {
return [topLevelNodeData.map((node) => node.layout.id), emptyArray];
}

const hasRequired = traversalSelector(
(traverser) => {
if (!page) {
return false;
const toMainLayout: string[] = [];
const toErrorReport: string[] = [];

for (const node of topLevelNodeData.reverse()) {
const capabilities = getComponentCapabilities(node.layout.type);
const isButtonLike =
node.layout.type === 'ButtonGroup' || (capabilities.renderInButtonGroup && node.layout.type !== 'Custom');
if (isButtonLike && toMainLayout.length === 0) {
toErrorReport.push(node.layout.id);
} else {
toMainLayout.push(node.layout.id);
}
return traverser.with(page).flat((n) => n.type === 'node' && nodeDataIsRequired(n)).length > 0;
},
[currentPageId],
);
}

const { formErrors, taskErrors } = useTaskErrors();
const hasErrors = Boolean(formErrors.length) || Boolean(taskErrors.length);
return [toMainLayout.reverse(), toErrorReport.reverse()];
});

const [mainIds, errorReportIds] = traversalSelector(
(traverser) => {
if (!hasErrors || !page) {
return [topLevelNodeIds, emptyArray];
}
return extractBottomButtons(traverser.with(page).children());
},
[currentPageId, hasErrors],
const hasRequired = NodesInternal.useSelector((state) =>
Object.values(state.nodeData).some((node) => node.pageKey === currentPageId && nodeDataIsRequired(node)),
);

useEffect(() => {
setFormState({ hasRequired, mainIds, errorReportIds, formErrors, taskErrors });
}, [setFormState, hasRequired, mainIds, errorReportIds, formErrors, taskErrors]);

return null;
return {
hasRequired,
mainIds,
errorReportIds,
formErrors,
taskErrors,
};
}

function HandleNavigationFocusComponent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import { ExprVal } from 'src/features/expressions/types';
import { ExprValidation } from 'src/features/expressions/validation';
import { useNavigationParam } from 'src/features/routing/AppRoutingContext';
import comboboxClasses from 'src/styles/combobox.module.css';
import { useNodes } from 'src/utils/layout/NodesContext';
import { NodesInternal, useNodes } from 'src/utils/layout/NodesContext';
import { useExpressionDataSources } from 'src/utils/layout/useExpressionDataSources';
import { useNodeTraversal, useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal';
import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal';
import type { ExprConfig, Expression, ExprFunctionName } from 'src/features/expressions/types';
import type { LayoutNode } from 'src/utils/layout/LayoutNode';
import type { LayoutPage } from 'src/utils/layout/LayoutPage';
Expand Down Expand Up @@ -76,8 +76,11 @@ export const ExpressionPlayground = () => {

const traversalSelector = useNodeTraversalSelector();

const componentOptions = useNodeTraversal((t) =>
t.allNodes().map((n) => ({ label: n.id, value: `${n.page.pageKey}|${n.id}` })),
const componentOptions = NodesInternal.useShallowSelector((state) =>
Object.values(state.nodeData).map((nodeData) => ({
label: nodeData.layout.id,
value: `${nodeData.pageKey}|${nodeData.layout.id}`,
})),
);

useEffect(() => {
Expand Down
12 changes: 7 additions & 5 deletions src/features/devtools/components/NodeInspector/NodeInspector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ import { SplitView } from 'src/features/devtools/components/SplitView/SplitView'
import { useDevToolsStore } from 'src/features/devtools/data/DevToolsStore';
import { useCurrentView } from 'src/hooks/useNavigatePage';
import { implementsAnyValidation } from 'src/layout';
import { useGetPage, useNode } from 'src/utils/layout/NodesContext';
import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal';
import { NodesInternal, useNode } from 'src/utils/layout/NodesContext';

export const NodeInspector = () => {
const pageKey = useCurrentView();
const currentPage = useGetPage(pageKey);
const selectedId = useDevToolsStore((state) => state.nodeInspector.selectedNodeId);
const selectedNode = useNode(selectedId);
const children = useNodeTraversal((t) => (currentPage ? t.with(currentPage).children() : undefined));
const children = NodesInternal.useShallowSelector((state) =>
Object.values(state.nodeData)
.filter((data) => data.pageKey === pageKey && data.parentId === undefined) // Find top-level nodes
.map((data) => data.layout.id),
);
const setSelected = useDevToolsStore((state) => state.actions.nodeInspectorSet);
const focusLayoutInspector = useDevToolsStore((state) => state.actions.focusLayoutInspector);

Expand All @@ -32,7 +34,7 @@ export const NodeInspector = () => {
>
<div className={reusedClasses.container}>
<NodeHierarchy
nodeIds={children?.map((c) => c.id) ?? []}
nodeIds={children?.map((id) => id) ?? []}
selected={selectedId}
onClick={setSelected}
/>
Expand Down
15 changes: 11 additions & 4 deletions src/features/options/effects/EffectPreselectedOptionIndex.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { useRef } from 'react';
import { useEffect, useRef, useState } from 'react';

import { useSetOptions } from 'src/features/options/useGetOptions';
import { GeneratorInternal } from 'src/utils/layout/generator/GeneratorContext';
import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext';
import { Hidden, NodesInternal, NodesReadiness } from 'src/utils/layout/NodesContext';
import type { IOptionInternal } from 'src/features/options/castOptionsToStrings';
import type { OptionsValueType } from 'src/features/options/useGetOptions';
import type { IDataModelBindingsOptionsSimple } from 'src/layout/common.generated';
Expand All @@ -21,6 +21,8 @@ interface Props {
*/
export function EffectPreselectedOptionIndex({ preselectedOption, valueType, options }: Props) {
const node = GeneratorInternal.useParent() as LayoutNode<CompWithBehavior<'canHaveOptions'>>;
const nodeStore = NodesInternal.useStore();
const [force, setForceUpdate] = useState(0);
const isNodeHidden = Hidden.useIsHidden(node);
const hasSelectedInitial = useRef(false);
const item = GeneratorInternal.useIntermediateItem() as CompIntermediate<CompWithBehavior<'canHaveOptions'>>;
Expand All @@ -30,12 +32,17 @@ export function EffectPreselectedOptionIndex({ preselectedOption, valueType, opt
const shouldSelectOptionAutomatically =
!hasValue && !hasSelectedInitial.current && preselectedOption !== undefined && isNodeHidden !== true;

NodesInternal.useEffectWhenReady(() => {
useEffect(() => {
if (shouldSelectOptionAutomatically) {
if (nodeStore.getState().readiness !== NodesReadiness.Ready) {
requestAnimationFrame(() => setForceUpdate((prev) => prev + 1));
return;
}

setData([preselectedOption.value]);
hasSelectedInitial.current = true;
}
}, [preselectedOption, shouldSelectOptionAutomatically, setData]);
}, [preselectedOption, shouldSelectOptionAutomatically, setData, nodeStore, node.id, force]);

return null;
}
15 changes: 12 additions & 3 deletions src/features/options/effects/EffectRemoveStaleValues.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { useEffect, useState } from 'react';

import deepEqual from 'fast-deep-equal';

import { useSetOptions } from 'src/features/options/useGetOptions';
import { useAsRef } from 'src/hooks/useAsRef';
import { GeneratorInternal } from 'src/utils/layout/generator/GeneratorContext';
import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext';
import { Hidden, NodesInternal, NodesReadiness } from 'src/utils/layout/NodesContext';
import type { IOptionInternal } from 'src/features/options/castOptionsToStrings';
import type { OptionsValueType } from 'src/features/options/useGetOptions';
import type { IDataModelBindingsOptionsSimple } from 'src/layout/common.generated';
Expand All @@ -24,6 +26,8 @@ interface Props {
export function EffectRemoveStaleValues({ valueType, options }: Props) {
const node = GeneratorInternal.useParent() as LayoutNode<CompWithBehavior<'canHaveOptions'>>;
const isNodeHidden = Hidden.useIsHidden(node);
const nodeStore = NodesInternal.useStore();
const [force, setForceUpdate] = useState(0);

const item = GeneratorInternal.useIntermediateItem() as CompIntermediate<CompWithBehavior<'canHaveOptions'>>;
const dataModelBindings = item.dataModelBindings as IDataModelBindingsOptionsSimple | undefined;
Expand All @@ -32,7 +36,7 @@ export function EffectRemoveStaleValues({ valueType, options }: Props) {
const optionsAsRef = useAsRef(options);
const itemsToRemove = getItemsToRemove(options, setResult.unsafeSelectedValues);

NodesInternal.useEffectWhenReady(() => {
useEffect(() => {
const { unsafeSelectedValues, setData } = setResultAsRef.current;
const options = optionsAsRef.current;
if (itemsToRemove.length === 0 || isNodeHidden || !options) {
Expand All @@ -41,9 +45,14 @@ export function EffectRemoveStaleValues({ valueType, options }: Props) {

const freshItemsToRemove = getItemsToRemove(optionsAsRef.current, unsafeSelectedValues);
if (freshItemsToRemove.length > 0 && deepEqual(freshItemsToRemove, itemsToRemove)) {
if (nodeStore.getState().readiness !== NodesReadiness.Ready) {
requestAnimationFrame(() => setForceUpdate((prev) => prev + 1));
return;
}

setData(unsafeSelectedValues.filter((v) => !itemsToRemove.includes(v)));
}
}, [isNodeHidden, itemsToRemove, optionsAsRef, setResultAsRef]);
}, [isNodeHidden, itemsToRemove, nodeStore, optionsAsRef, setResultAsRef, force]);

return null;
}
Expand Down
15 changes: 11 additions & 4 deletions src/features/options/effects/EffectStoreLabel.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { useMemo } from 'react';
import { useEffect, useMemo, useState } from 'react';

import deepEqual from 'fast-deep-equal';

import { useDataModelBindings } from 'src/features/formData/useDataModelBindings';
import { useLanguage } from 'src/features/language/useLanguage';
import { useSetOptions } from 'src/features/options/useGetOptions';
import { GeneratorInternal } from 'src/utils/layout/generator/GeneratorContext';
import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext';
import { Hidden, NodesInternal, NodesReadiness } from 'src/utils/layout/NodesContext';
import type { IOptionInternal } from 'src/features/options/castOptionsToStrings';
import type { OptionsValueType } from 'src/features/options/useGetOptions';
import type { IDataModelBindingsOptionsSimple } from 'src/layout/common.generated';
Expand All @@ -24,6 +24,8 @@ interface Props {
export function EffectStoreLabel({ valueType, options }: Props) {
const item = GeneratorInternal.useIntermediateItem() as CompIntermediate<CompWithBehavior<'canHaveOptions'>>;
const node = GeneratorInternal.useParent() as LayoutNode<CompWithBehavior<'canHaveOptions'>>;
const nodeStore = NodesInternal.useStore();
const [force, setForceUpdate] = useState(0);
const isNodeHidden = Hidden.useIsHidden(node);
const { langAsString } = useLanguage();
const dataModelBindings = item.dataModelBindings as IDataModelBindingsOptionsSimple | undefined;
Expand All @@ -42,11 +44,16 @@ export function EffectStoreLabel({ valueType, options }: Props) {
const labelsHaveChanged = !deepEqual(translatedLabels, 'label' in formData ? formData.label : undefined);
const shouldSetData = labelsHaveChanged && !isNodeHidden && dataModelBindings && 'label' in dataModelBindings;

NodesInternal.useEffectWhenReady(() => {
useEffect(() => {
if (!shouldSetData) {
return;
}

if (nodeStore.getState().readiness !== NodesReadiness.Ready) {
requestAnimationFrame(() => setForceUpdate((prev) => prev + 1));
return;
}

if (!translatedLabels || translatedLabels.length === 0) {
setValue('label', undefined);
return;
Expand All @@ -55,7 +62,7 @@ export function EffectStoreLabel({ valueType, options }: Props) {
} else {
setValue('label', translatedLabels);
}
}, [setValue, shouldSetData, translatedLabels, valueType]);
}, [nodeStore, setValue, shouldSetData, translatedLabels, valueType, force]);

return null;
}
Loading