AllBlocks map

pixiebrix · Oct 12, 2021 · a0e32bd · a0e32bd
1 parent e3e4edd
commit a0e32bd
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 117 deletions.
diff --git a/src/devTools/editor/hooks/usePipelineField.ts b/src/devTools/editor/hooks/usePipelineField.ts
@@ -17,40 +17,34 @@
 
 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<string | number, unknown>;
 
 const pipelineBlocksFieldName = "extension.blockPipeline";
 
 function usePipelineField(
-  allBlocks: IBlock[]
+  allBlocks: BlocksMap
 ): {
   blockPipeline: BlockPipeline;
   blockPipelineErrors: PipelineErrors;
   errorTraceEntry: TraceError;
 } {
   const errorTraceEntry = useSelector(selectTraceError);
 
-  useEffect(() => {
-    clearOutputKeyValidatorValidatorCache();
-  }, [allBlocks]);
-
   const validatePipelineBlocks = useCallback(
-    async (pipeline: BlockPipeline): Promise<void | PipelineErrors> => {
+    (pipeline: BlockPipeline): void | PipelineErrors => {
       const formikErrors: Record<string, unknown> = {};
 
-      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
@@ -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<NodeId>(FOUNDATION_NODE_ID);
   const activeBlockIndex = useMemo(() => {

diff --git a/src/devTools/editor/tabs/editTab/editTabTypes.ts 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
@@ -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<string, unknown> = {};
-    await outputKeyValidator(pipelineErrors, pipelineFactory(), []);
+    outputKeyValidator(pipelineErrors, pipelineFactory(), {});
 
     expect(pipelineErrors).toEqual({});
   });
 
   test("returns when pipeline is empty", async () => {
     const pipelineErrors: Record<string, unknown> = {};
-    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,62 +100,18 @@ 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(
         "Must start with a letter and only include letters and numbers."
       );
     }
   );
-
-  describe("sequential calls", () => {
-    test("validates different pipelines", async () => {
-      const allBlocks = [blockFactory()];
-
-      const pipelineErrors1: Record<string, unknown> = {};
-      const pipeline1 = pipelineFactory({
-        outputKey: "validOutputKey" as OutputKey,
-      });
-      await outputKeyValidator(pipelineErrors1, pipeline1, allBlocks);
-      expect(pipelineErrors1).toEqual({});
-
-      const pipelineErrors2: Record<string, unknown> = {};
-      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<string, unknown> = {};
-      const allBlocks1: IBlock[] = [];
-      await outputKeyValidator(pipelineErrors1, pipeline, allBlocks1);
-      // AllBlocks empty - no validation
-      expect(pipelineErrors1).toEqual({});
-
-      const pipelineErrors2: Record<string, unknown> = {};
-      const allBlocks2 = [
-        blockFactory({
-          effect: jest.fn(),
-        } as any),
-      ];
-      await outputKeyValidator(pipelineErrors2, pipeline, allBlocks2);
-      // Effects have empty OutputKey
-      expect(pipelineErrors1).toEqual({});
-
-      const pipelineErrors3: Record<string, unknown> = {};
-      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
@@ -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<string, unknown>,
   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)) {