diff --git a/src/devTools/editor/hooks/usePipelineField.ts b/src/devTools/editor/hooks/usePipelineField.ts index b91a33f1f0..0d354cd4c6 100644 --- a/src/devTools/editor/hooks/usePipelineField.ts +++ b/src/devTools/editor/hooks/usePipelineField.ts @@ -17,24 +17,22 @@ import { useSelector } from "react-redux"; import { selectTraceError } from "@/devTools/editor/slices/runtimeSelectors"; -import { useCallback, useEffect } from "react"; +import { useCallback } from "react"; import { BlockConfig, BlockPipeline } from "@/blocks/types"; import { useField, useFormikContext, setNestedObjectValues } from "formik"; import { TraceError } from "@/telemetry/trace"; import { useAsyncEffect } from "use-async-effect"; -import outputKeyValidator, { - clearOutputKeyValidatorValidatorCache, -} from "@/devTools/editor/validators/outputKeyValidator"; -import { IBlock } from "@/core"; +import outputKeyValidator from "@/devTools/editor/validators/outputKeyValidator"; import traceErrorValidator from "@/devTools/editor/validators/traceErrorValidator"; import { isEmpty } from "lodash"; +import { BlocksMap } from "@/devTools/editor/tabs/editTab/editTabTypes"; export type PipelineErrors = string | Record; const pipelineBlocksFieldName = "extension.blockPipeline"; function usePipelineField( - allBlocks: IBlock[] + allBlocks: BlocksMap ): { blockPipeline: BlockPipeline; blockPipelineErrors: PipelineErrors; @@ -42,15 +40,11 @@ function usePipelineField( } { const errorTraceEntry = useSelector(selectTraceError); - useEffect(() => { - clearOutputKeyValidatorValidatorCache(); - }, [allBlocks]); - const validatePipelineBlocks = useCallback( - async (pipeline: BlockPipeline): Promise => { + (pipeline: BlockPipeline): void | PipelineErrors => { const formikErrors: Record = {}; - await outputKeyValidator(formikErrors, pipeline, allBlocks ?? []); + outputKeyValidator(formikErrors, pipeline, allBlocks); traceErrorValidator(formikErrors, errorTraceEntry, pipeline); return isEmpty(formikErrors) ? undefined : formikErrors; diff --git a/src/devTools/editor/tabs/editTab/EditTab.tsx b/src/devTools/editor/tabs/editTab/EditTab.tsx index c1374ed95f..bcfa4d43d6 100644 --- a/src/devTools/editor/tabs/editTab/EditTab.tsx +++ b/src/devTools/editor/tabs/editTab/EditTab.tsx @@ -29,7 +29,7 @@ import { BlockType, defaultBlockConfig, getType } from "@/blocks/util"; import { useAsyncState } from "@/hooks/common"; import blockRegistry from "@/blocks/registry"; import { compact, isEmpty } from "lodash"; -import { IBlock, OutputKey, RegistryId, UUID } from "@/core"; +import { IBlock, OutputKey, UUID } from "@/core"; import { produce } from "immer"; import EditorNodeConfigPanel from "@/devTools/editor/tabs/editTab/editorNodeConfigPanel/EditorNodeConfigPanel"; import styles from "./EditTab.module.scss"; @@ -48,14 +48,7 @@ import useExtensionTrace from "@/devTools/editor/hooks/useExtensionTrace"; import FoundationDataPanel from "@/devTools/editor/tabs/editTab/dataPanel/FoundationDataPanel"; import { produceExcludeUnusedDependencies } from "@/components/fields/schemaFields/ServiceField"; import usePipelineField from "@/devTools/editor/hooks/usePipelineField"; - -type BlocksMap = Record< - RegistryId, - { - block: IBlock; - type: BlockType; - } ->; +import { BlocksMap } from "./editTabTypes"; const blockConfigTheme: ThemeProps = { layout: "horizontal", @@ -100,7 +93,7 @@ const EditTab: React.FC<{ blockPipeline, blockPipelineErrors, errorTraceEntry, - } = usePipelineField(Object.values(allBlocks).map(({ block }) => block)); + } = usePipelineField(allBlocks); const [activeNodeId, setActiveNodeId] = useState(FOUNDATION_NODE_ID); const activeBlockIndex = useMemo(() => { diff --git a/src/devTools/editor/tabs/editTab/editTabTypes.ts b/src/devTools/editor/tabs/editTab/editTabTypes.ts new file mode 100644 index 0000000000..02bf3aa51b --- /dev/null +++ b/src/devTools/editor/tabs/editTab/editTabTypes.ts @@ -0,0 +1,10 @@ +import { BlockType } from "@/blocks/util"; +import { IBlock, RegistryId } from "@/core"; + +export type BlocksMap = Record< + RegistryId, + { + block: IBlock; + type: BlockType; + } +>; diff --git a/src/devTools/editor/validators/outputKeyValidator.test.ts b/src/devTools/editor/validators/outputKeyValidator.test.ts index c1d657957b..f78167d555 100644 --- a/src/devTools/editor/validators/outputKeyValidator.test.ts +++ b/src/devTools/editor/validators/outputKeyValidator.test.ts @@ -16,27 +16,27 @@ */ import { BlockType } from "@/blocks/util"; -import { IBlock, OutputKey } from "@/core"; +import { OutputKey } from "@/core"; import { blockFactory, pipelineFactory } from "@/tests/factories"; -import outputKeyValidator, { - clearOutputKeyValidatorValidatorCache, -} from "./outputKeyValidator"; +import outputKeyValidator from "./outputKeyValidator"; describe("outputKeyValidator", () => { - afterEach(() => { - clearOutputKeyValidatorValidatorCache(); - }); - test("returns when no blocks given", async () => { const pipelineErrors: Record = {}; - await outputKeyValidator(pipelineErrors, pipelineFactory(), []); + outputKeyValidator(pipelineErrors, pipelineFactory(), {}); expect(pipelineErrors).toEqual({}); }); test("returns when pipeline is empty", async () => { const pipelineErrors: Record = {}; - await outputKeyValidator(pipelineErrors, [], [{} as IBlock]); + const block = blockFactory(); + outputKeyValidator(pipelineErrors, [], { + [block.id]: { + block, + type: null, + }, + }); expect(pipelineErrors).toEqual({}); }); @@ -53,11 +53,16 @@ describe("outputKeyValidator", () => { const block = blockFactory({ [blockProperty]: jest.fn(), }); - await outputKeyValidator(pipelineErrors, pipeline, [block]); + outputKeyValidator(pipelineErrors, pipeline, { + [block.id]: { + block, + type: blockType, + }, + }); expect(pipelineErrors[0]).toBeUndefined(); expect((pipelineErrors[1] as any).outputKey).toBe( - `OutputKey must be empty for ${blockType} block.` + `OutputKey must be empty for "${blockType}" block.` ); } ); @@ -74,7 +79,12 @@ describe("outputKeyValidator", () => { const block = blockFactory({ [blockProperty]: jest.fn(), }); - await outputKeyValidator(pipelineErrors, pipeline, [block]); + outputKeyValidator(pipelineErrors, pipeline, { + [block.id]: { + block, + type: blockType, + }, + }); expect(pipelineErrors[0]).toBeUndefined(); expect((pipelineErrors[1] as any).outputKey).toBe( @@ -90,7 +100,13 @@ describe("outputKeyValidator", () => { const pipeline = pipelineFactory(); pipeline[0].outputKey = "validOutputKey" as OutputKey; pipeline[1].outputKey = invalidOutputKey as OutputKey; - await outputKeyValidator(pipelineErrors, pipeline, [blockFactory()]); + const block = blockFactory(); + outputKeyValidator(pipelineErrors, pipeline, { + [block.id]: { + block, + type: null, + }, + }); expect(pipelineErrors[0]).toBeUndefined(); expect((pipelineErrors[1] as any).outputKey).toBe( @@ -98,54 +114,4 @@ describe("outputKeyValidator", () => { ); } ); - - describe("sequential calls", () => { - test("validates different pipelines", async () => { - const allBlocks = [blockFactory()]; - - const pipelineErrors1: Record = {}; - const pipeline1 = pipelineFactory({ - outputKey: "validOutputKey" as OutputKey, - }); - await outputKeyValidator(pipelineErrors1, pipeline1, allBlocks); - expect(pipelineErrors1).toEqual({}); - - const pipelineErrors2: Record = {}; - const pipeline2 = pipelineFactory({ - outputKey: "not valid OutputKey" as OutputKey, - }); - await outputKeyValidator(pipelineErrors2, pipeline2, allBlocks); - expect(pipelineErrors2[0]).toBeDefined(); - expect(pipelineErrors2[1]).toBeDefined(); - }); - - test("validates with different blocks", async () => { - const pipeline = pipelineFactory(); - - const pipelineErrors1: Record = {}; - const allBlocks1: IBlock[] = []; - await outputKeyValidator(pipelineErrors1, pipeline, allBlocks1); - // AllBlocks empty - no validation - expect(pipelineErrors1).toEqual({}); - - const pipelineErrors2: Record = {}; - const allBlocks2 = [ - blockFactory({ - effect: jest.fn(), - } as any), - ]; - await outputKeyValidator(pipelineErrors2, pipeline, allBlocks2); - // Effects have empty OutputKey - expect(pipelineErrors1).toEqual({}); - - const pipelineErrors3: Record = {}; - const allBlocks3 = [blockFactory()]; - // Must clear validator's cache when `allBlocks` changes - clearOutputKeyValidatorValidatorCache(); - await outputKeyValidator(pipelineErrors3, pipeline, allBlocks3); - // Expect "OutputKey is required" error - expect(pipelineErrors3[0]).toBeDefined(); - expect(pipelineErrors3[1]).toBeDefined(); - }); - }); }); diff --git a/src/devTools/editor/validators/outputKeyValidator.ts b/src/devTools/editor/validators/outputKeyValidator.ts index 5fb3a86ba1..8a4c03bdc6 100644 --- a/src/devTools/editor/validators/outputKeyValidator.ts +++ b/src/devTools/editor/validators/outputKeyValidator.ts @@ -16,11 +16,10 @@ */ import { BlockPipeline } from "@/blocks/types"; -import { BlockType, getType } from "@/blocks/util"; -import { IBlock } from "@/core"; +import { BlockType } from "@/blocks/util"; import { joinName } from "@/utils"; -import { memoize, set } from "lodash"; -import hash from "object-hash"; +import { isEmpty, set } from "lodash"; +import { BlocksMap } from "@/devTools/editor/tabs/editTab/editTabTypes"; const outputKeyRegex = /^[A-Za-z][\dA-Za-z]*$/; @@ -35,49 +34,28 @@ function setOutputKeyError( set(pipelineErrors, propertyNameInPipeline, errorMessage); } -async function getPipelineBlockTypes( - pipeline: BlockPipeline, - allBlocks: IBlock[] -) { - return Promise.all( - pipeline - .map(({ id }) => allBlocks.find((block) => block.id === id)) - .map(async (block) => (block ? getType(block) : null)) - ); -} - -// Note: allBlocks is not included in cache key, hence the cache must be cleared when allBlocks changes -const memoizeGetPipelineBlockTypes = memoize( - getPipelineBlockTypes, - (pipeline: BlockPipeline) => hash(pipeline.map((x) => x.id)) -); -export function clearOutputKeyValidatorValidatorCache() { - memoizeGetPipelineBlockTypes.cache.clear(); -} - -async function outputKeyValidator( +function outputKeyValidator( pipelineErrors: Record, pipeline: BlockPipeline, - allBlocks: IBlock[] + allBlocks: BlocksMap ) { // No blocks, no validation - if (pipeline.length === 0 || allBlocks.length === 0) { + if (pipeline.length === 0 || isEmpty(allBlocks)) { return; } - const blockTypes = await memoizeGetPipelineBlockTypes(pipeline, allBlocks); - for (let blockIndex = 0; blockIndex !== pipeline.length; ++blockIndex) { let errorMessage: string; + // eslint-disable-next-line security/detect-object-injection const pipelineBlock = pipeline[blockIndex]; - const blockType = blockTypes[blockIndex]; + const blockType = allBlocks[pipelineBlock.id]?.type; if (blockTypesWithEmptyOutputKey.includes(blockType)) { if (!pipelineBlock.outputKey) { continue; } - errorMessage = `OutputKey must be empty for ${blockType} block.`; + errorMessage = `OutputKey must be empty for "${blockType}" block.`; } else if (!pipelineBlock.outputKey) { errorMessage = "This field is required."; } else if (outputKeyRegex.test(pipelineBlock.outputKey)) {