#7649: introduce platform protocol to define clear platform boundary and decouple bricks from messenger/webext APIs

eslint-local-rules/persistBackgroundData.txt

@@ -72,14 +72,14 @@
 ./src/contentScript/context.ts
 ./src/contentScript/contextMenus.ts
 ./src/contentScript/elementReference.ts
-./src/contentScript/ephemeralFormProtocol.ts
+./src/contentScript/formController.ts
 ./src/contentScript/ephemeralPanelController.tsx
 ./src/contentScript/lifecycle.ts
 ./src/contentScript/messenger/api.ts
 ./src/contentScript/messenger/runBrickTypes.ts
 ./src/contentScript/pageEditor/elementPicker.ts
 ./src/contentScript/pageEditor/types.ts
-./src/contentScript/pageState.ts
+./src/contentScript/stateController.ts
 ./src/contentScript/performanceMonitoring.ts
 ./src/contentScript/ready.ts
 ./src/contentScript/sidebarController.tsx

jest.config.js

@@ -20,7 +20,6 @@ const config = {
   testEnvironment: "./src/testUtils/FixJsdomEnvironment.js",
   modulePaths: ["/src"],
   moduleFileExtensions: ["ts", "tsx", "js", "jsx", "yaml", "yml", "json"],
-  testPathIgnorePatterns: ["<rootDir>/selenium/"],
   modulePathIgnorePatterns: ["<rootDir>/headers.json", "<rootDir>/dist/"],
   transform: {
     "^.+\\.[jt]sx?$": "@swc/jest",

src/analysis/analysisVisitors/formBrickAnalysis.ts

@@ -16,11 +16,14 @@
  */
 
 import { AnalysisVisitorABC } from "./baseAnalysisVisitors";
-import { type BrickConfig, type BrickPosition } from "@/bricks/types";
+import {
+  type BrickConfig,
+  type BrickPosition,
+  PipelineFlavor,
+} from "@/bricks/types";
 import { type VisitBlockExtra } from "@/bricks/PipelineVisitor";
 import { FORM_MODAL_ID } from "@/pageEditor/fields/FormModalOptions";
 import { AnnotationType } from "@/types/annotationTypes";
-import { PipelineFlavor } from "@/pageEditor/pageEditorTypes";
 
 class FormBrickAnalysis extends AnalysisVisitorABC {
   get id() {

src/analysis/analysisVisitors/renderersAnalysis.ts

@@ -16,12 +16,15 @@
  */
 
 import { AnalysisVisitorWithResolvedBricksABC } from "./baseAnalysisVisitors";
-import { type BrickConfig, type BrickPosition } from "@/bricks/types";
+import {
+  type BrickConfig,
+  type BrickPosition,
+  PipelineFlavor,
+} from "@/bricks/types";
 import {
   nestedPosition,
   type VisitPipelineExtra,
 } from "@/bricks/PipelineVisitor";
-import { PipelineFlavor } from "@/pageEditor/pageEditorTypes";
 import { AnnotationType } from "@/types/annotationTypes";
 
 export const MULTIPLE_RENDERERS_ERROR_MESSAGE =

src/background/activateBrowserActionIcon.test.ts

@@ -24,6 +24,7 @@ import MockAdapter from "axios-mock-adapter";
 import { browserAction } from "@/mv3/api";
 
 jest.mock("@/mv3/api", () => ({
+  isMV3: jest.fn().mockReturnValue(false),
   browserAction: {
     setIcon: jest.fn(),
   },
@@ -35,7 +36,7 @@ describe("activateBrowserActionIcon", () => {
 
   const drawImageMock = jest.fn();
   const getImageDataMock = jest.fn().mockReturnValue("image data");
-  const getContextmock = jest.fn().mockReturnValue({
+  const getContextMock = jest.fn().mockReturnValue({
     drawImage: drawImageMock,
     getImageData: getImageDataMock,
   });
@@ -55,7 +56,7 @@ describe("activateBrowserActionIcon", () => {
   };
   // @ts-expect-error -- No need to mock the whole class for the test
   global.OffscreenCanvas = class {
-    getContext = getContextmock;
+    getContext = getContextMock;
   };
 
   URL.createObjectURL = jest.fn();
@@ -107,7 +108,7 @@ describe("activateBrowserActionIcon", () => {
       const result = await blobToImageData(blob);
 
       expect(result).toBe("image data");
-      expect(getContextmock).toHaveBeenCalledWith("2d");
+      expect(getContextMock).toHaveBeenCalledWith("2d");
       expect(drawImageMock).toHaveBeenCalledWith(
         expect.any(Image),
         0,

src/background/contextMenus.ts

@@ -35,20 +35,16 @@ import {
   type ResolvedModComponent,
 } from "@/types/modComponentTypes";
 import { allSettled, memoizeUntilSettled } from "@/utils/promiseUtils";
+import type { SelectionMenuOptions } from "@/platform/platformProtocol";
+import { ContextError } from "@/errors/genericErrors";
+import { selectEventData } from "@/telemetry/deployments";
 
 const MENU_PREFIX = "pixiebrix-";
 
 // This constant must be high enough to give Chrome time to inject the content script. ensureContentScript can take
 // >= 1 seconds because it also waits for the content script to be ready
 const CONTEXT_SCRIPT_INSTALL_MS = 5000;
 
-type SelectionMenuOptions = {
-  extensionId: UUID;
-  title: string;
-  contexts: Menus.ContextType[];
-  documentUrlPatterns: string[];
-};
-
 /**
  * Return a unique context menu item id for the given extension id.
  * @param extensionId
@@ -193,8 +189,16 @@ export async function preloadContextMenus(
       resolved.extensionPointId,
     );
     if (extensionPoint instanceof ContextMenuStarterBrickABC) {
-      await extensionPoint.ensureMenu(
+      await extensionPoint.registerMenuItem(
         definition as unknown as ResolvedModComponent<ContextMenuConfig>,
+        () => {
+          throw new ContextError(
+            "Context menu was preloaded, but no handler was registered",
+            {
+              context: selectEventData(resolved),
+            },
+          );
+        },
       );
     }
   });

src/bricks/PipelineExpressionVisitor.ts

@@ -15,14 +15,17 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { type BrickConfig, type BrickPosition } from "@/bricks/types";
+import {
+  type BrickConfig,
+  type BrickPosition,
+  PipelineFlavor,
+} from "@/bricks/types";
 import { type Expression } from "@/types/runtimeTypes";
 import PipelineVisitor, {
   nestedPosition,
   type VisitBlockExtra,
 } from "./PipelineVisitor";
 import { type DocumentElement } from "@/components/documentBuilder/documentBuilderTypes";
-import { PipelineFlavor } from "@/pageEditor/pageEditorTypes";
 import { isExpression, isPipelineExpression } from "@/utils/expressionUtils";
 import { joinPathParts } from "@/utils/formUtils";
 
@@ -94,7 +97,7 @@ abstract class PipelineExpressionVisitor extends PipelineVisitor {
       }
 
       default: {
-        return PipelineFlavor.AllBlocks;
+        return PipelineFlavor.AllBricks;
       }
     }
   }

src/bricks/PipelineVisitor.ts

@@ -15,7 +15,11 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { type BrickConfig, type BrickPosition } from "@/bricks/types";
+import {
+  type BrickConfig,
+  type BrickPosition,
+  PipelineFlavor,
+} from "@/bricks/types";
 import { type UUID } from "@/types/stringTypes";
 import { type TypedBrickPair } from "@/bricks/registry";
 import { DocumentRenderer } from "@/bricks/renderers/document";
@@ -26,7 +30,6 @@ import {
   getSubPipelineFlavor,
 } from "@/bricks/blockFilterHelpers";
 import { type StarterBrickType } from "@/types/starterBrickTypes";
-import { PipelineFlavor } from "@/pageEditor/pageEditorTypes";
 import { PIPELINE_BLOCKS_FIELD_NAME } from "@/pageEditor/consts";
 import { isPipelineExpression } from "@/utils/expressionUtils";
 import { joinPathParts } from "@/utils/formUtils";
@@ -175,7 +178,7 @@ class PipelineVisitor {
   ): void {
     const flavor = extra?.extensionPointType
       ? getRootPipelineFlavor(extra.extensionPointType)
-      : PipelineFlavor.AllBlocks;
+      : PipelineFlavor.AllBricks;
     this.visitPipeline(ROOT_POSITION, pipeline, { flavor });
   }
 }

src/bricks/blockFilterHelpers.ts

@@ -17,7 +17,6 @@
 
 import { type TypedBrickPair } from "@/bricks/registry";
 import { type StarterBrickType } from "@/types/starterBrickTypes";
-import { PipelineFlavor } from "@/pageEditor/pageEditorTypes";
 import { stubTrue } from "lodash";
 import { DocumentRenderer } from "@/bricks/renderers/document";
 import { type BrickType } from "@/runtime/runtimeTypes";
@@ -27,6 +26,7 @@ import TourStepTransformer from "@/bricks/transformers/tourStep/tourStep";
 import { ErrorEffect } from "@/bricks/effects/error";
 import { CancelEffect } from "@/bricks/effects/cancel";
 import CommentEffect from "@/bricks/effects/comment";
+import { PipelineFlavor } from "@/bricks/types";
 
 const PANEL_TYPES = ["actionPanel", "panel"];
 
@@ -82,7 +82,7 @@ export function getSubPipelineFlavor(
 export function makeIsBlockAllowedForPipeline(
   pipelineFlavor: PipelineFlavor,
 ): IsBlockAllowedPredicate {
-  if (pipelineFlavor === PipelineFlavor.AllBlocks) {
+  if (pipelineFlavor === PipelineFlavor.AllBricks) {
     return stubTrue;
   }
 

src/bricks/effects/AddQuickBarAction.test.ts

@@ -16,23 +16,15 @@
  */
 
 import AddQuickBarAction from "@/bricks/effects/AddQuickBarAction";
-import quickbarRegistry from "@/components/quickBar/quickBarRegistry";
 import { unsafeAssumeValidArg } from "@/runtime/runtimeTypes";
 import ConsoleLogger from "@/utils/ConsoleLogger";
 import { uuidv4, validateRegistryId } from "@/types/helpers";
 import { brickOptionsFactory } from "@/testUtils/factories/runtimeFactories";
+import { platformMock } from "@/testUtils/platformMock";
 
 const brick = new AddQuickBarAction();
 
-jest.mock("@/components/quickBar/quickBarRegistry", () => ({
-  __esModule: true,
-  default: {
-    addAction: jest.fn(),
-    knownGeneratorRootIds: new Set<string>(),
-  },
-}));
-
-const addActionMock = jest.mocked(quickbarRegistry.addAction);
+const platform = platformMock;
 
 const logger = new ConsoleLogger({
   extensionId: uuidv4(),
@@ -41,7 +33,7 @@ const logger = new ConsoleLogger({
 
 describe("AddQuickBarAction", () => {
   beforeEach(() => {
-    addActionMock.mockReset();
+    jest.clearAllMocks();
   });
 
   test("is root aware", async () => {
@@ -58,12 +50,13 @@ describe("AddQuickBarAction", () => {
     await brick.run(
       unsafeAssumeValidArg({ title: "test" }),
       brickOptionsFactory({
+        platform,
         logger,
         root: document,
         abortSignal: abortController.signal,
       }),
     );
-    expect(addActionMock).toHaveBeenCalledWith({
+    expect(platform.quickBar.addAction).toHaveBeenCalledWith({
       id: expect.toBeString(),
       extensionPointId: logger.context.extensionPointId,
       extensionId: logger.context.extensionId,

src/bricks/effects/AddQuickBarAction.tsx

@@ -17,7 +17,6 @@
 
 import { validateRegistryId } from "@/types/helpers";
 import { propertiesToSchema } from "@/validators/generic";
-import quickBarRegistry from "@/components/quickBar/quickBarRegistry";
 import Icon from "@/icons/Icon";
 import React from "react";
 import {
@@ -29,6 +28,9 @@ import { type CustomAction } from "@/components/quickBar/quickbarTypes";
 import { type IconConfig } from "@/types/iconTypes";
 import { type Schema } from "@/types/schemaTypes";
 import { EffectABC } from "@/types/bricks/effectTypes";
+import type { BrickConfig } from "@/bricks/types";
+import type { PlatformCapability } from "@/platform/capabilities";
+import { uniq } from "lodash";
 
 type ActionConfig = {
   /**
@@ -85,6 +87,15 @@ class AddQuickBarAction extends EffectABC {
     return true;
   }
 
+  override async getRequiredCapabilities(
+    _config: BrickConfig,
+  ): Promise<PlatformCapability[]> {
+    return uniq([
+      ...(await super.getRequiredCapabilities(_config)),
+      "quickBar",
+    ]);
+  }
+
   inputSchema: Schema = propertiesToSchema(
     {
       title: {
@@ -126,8 +137,10 @@ class AddQuickBarAction extends EffectABC {
       // Be explicit about the default priority if non is provided
       priority = DEFAULT_PRIORITY,
     }: BrickArgs<ActionConfig>,
-    { root, logger, runPipeline, abortSignal }: BrickOptions,
+    { root, logger, runPipeline, abortSignal, platform }: BrickOptions,
   ): Promise<void> {
+    const { quickBar } = platform;
+
     // The runtime checks the abortSignal for each brick. But check here too to avoid flickering in the Quick Bar
     if (abortSignal.aborted) {
       return;
@@ -146,7 +159,7 @@ class AddQuickBarAction extends EffectABC {
       extensionPointId: logger.context.extensionPointId,
       extensionId: logger.context.extensionId,
       // Can only provide a parent if the parent exists
-      parent: quickBarRegistry.knownGeneratorRootIds.has(parentId)
+      parent: quickBar.knownGeneratorRootIds.has(parentId)
         ? parentId
         : undefined,
       name: title,
@@ -176,7 +189,7 @@ class AddQuickBarAction extends EffectABC {
       },
     };
 
-    quickBarRegistry.addAction(action);
+    quickBar.addAction(action);
   }
 }
 

src/bricks/effects/AddTextCommand.ts

@@ -24,13 +24,11 @@ import type {
 } from "@/types/runtimeTypes";
 import { type Schema } from "@/types/schemaTypes";
 import { EffectABC } from "@/types/bricks/effectTypes";
-import {
-  commandRegistry,
-  initCommandController,
-} from "@/contentScript/commandPopover/commandController";
+import { initCommandController } from "@/contentScript/commandPopover/commandController";
 import { BusinessError } from "@/errors/businessErrors";
 import { getSettingsState } from "@/store/settings/settingsStorage";
 import { validateOutputKey } from "@/runtime/runtimeTypes";
+import type { PlatformCapability } from "@/platform/capabilities";
 
 type CommandArgs = {
   /**
@@ -88,9 +86,13 @@ class AddTextCommand extends EffectABC {
     ["shortcut", "title", "generate"],
   );
 
+  override async getRequiredCapabilities(): Promise<PlatformCapability[]> {
+    return ["commandPopover"];
+  }
+
   async effect(
     { shortcut, title, generate: generatePipeline }: BrickArgs<CommandArgs>,
-    { logger, runPipeline, abortSignal }: BrickOptions,
+    { logger, runPipeline, platform, abortSignal }: BrickOptions,
   ): Promise<void> {
     // The runtime checks the abortSignal for each brick. But check here too to avoid flickering in the popover
     if (abortSignal?.aborted) {
@@ -104,7 +106,7 @@ class AddTextCommand extends EffectABC {
     // Counter to keep track of the action run number for tracing
     let counter = 0;
 
-    commandRegistry.register({
+    platform.commandPopover.register({
       componentId: logger.context.extensionId,
       // Trim leading slash to be resilient to user input
       shortcut: shortcut.replace(/^\//, ""),