#6065: fix rendering of user-defined bricks (#6067)

src/blocks/errors.ts

@@ -23,6 +23,7 @@ import { BusinessError, CancelError } from "@/errors/businessErrors";
 import { type MessageContext } from "@/types/loggerTypes";
 import { type RegistryId } from "@/types/registryTypes";
 import { type Schema } from "@/types/schemaTypes";
+import { type BrickArgs, type BrickArgsContext } from "@/types/runtimeTypes";
 
 export class PipelineConfigurationError extends BusinessError {
   override name = "PipelineConfigurationError";
@@ -45,16 +46,16 @@ export class HeadlessModeError extends Error {
 
   public readonly blockId: RegistryId;
 
-  public readonly args: unknown;
+  public readonly args: BrickArgs;
 
-  public readonly ctxt: unknown;
+  public readonly ctxt: BrickArgsContext;
 
   public readonly loggerContext: MessageContext;
 
   constructor(
     blockId: RegistryId,
-    args: unknown, // BlockArg
-    ctxt: unknown, // BrickArgsContext
+    args: BrickArgs,
+    ctxt: BrickArgsContext,
     loggerContext: MessageContext
   ) {
     super(`${blockId} is a renderer`);

src/blocks/transformers/brickFactory.test.ts

@@ -33,6 +33,9 @@ import Run from "@/blocks/transformers/controlFlow/Run";
 import { GetPageState } from "@/blocks/effects/pageState";
 import { cloneDeep } from "lodash";
 import { extraEmptyModStateContext } from "@/runtime/extendModVariableContext";
+import { setContext } from "@/testUtils/detectPageMock";
+
+setContext("contentScript");
 
 beforeEach(() => {
   blockRegistry.clear();

src/blocks/transformers/brickFactory.ts

@@ -15,7 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { BrickABC, type Brick } from "@/types/brickTypes";
+import { type Brick, BrickABC } from "@/types/brickTypes";
 import { readerFactory } from "@/blocks/readers/factory";
 import {
   type Schema as ValidatorSchema,
@@ -35,25 +35,47 @@ import {
   type ApiVersion,
   type BrickArgs,
   type BrickOptions,
+  validateBrickArgsContext,
 } from "@/types/runtimeTypes";
 import { type Schema, type UiSchema } from "@/types/schemaTypes";
-import {
-  type Metadata,
-  type RegistryId,
-  type SemVerString,
-} from "@/types/registryTypes";
+import { type Metadata, type SemVerString } from "@/types/registryTypes";
 import { type UnknownObject } from "@/types/objectTypes";
 import { inputProperties } from "@/helpers";
-
 import { isPipelineExpression } from "@/utils/expressionUtils";
+import { isContentScript } from "webext-detect-page";
+import { getTopLevelFrame } from "webext-messenger";
+import { uuidv4 } from "@/types/helpers";
+import { isSpecificError } from "@/errors/errorHelpers";
+import { HeadlessModeError } from "@/blocks/errors";
+import BackgroundLogger from "@/telemetry/BackgroundLogger";
+import { runHeadlessPipeline } from "@/contentScript/messenger/api";
 
 type BrickDefinition = {
+  /**
+   * The runtime version to use when running the Brick.
+   */
   apiVersion?: ApiVersion;
+
+  /**
+   * User-defined brick kind.
+   */
   kind: "component";
+
+  /**
+   * Registry package metadata.
+   */
   metadata: Metadata;
-  defaultOptions: Record<string, string>;
+
+  /**
+   * The wrapped bricks.
+   */
   pipeline: BrickConfig | BrickPipeline;
+
+  /**
+   * JSON Schema for brick inputs.
+   */
   inputSchema: Schema;
+
   /**
    * An optional RJSF uiSchema for inputs.
    *
@@ -63,18 +85,21 @@ type BrickDefinition = {
    */
   uiSchema?: UiSchema;
 
+  /**
+   * An optional JSON Output Schema for the brick.
+   */
   outputSchema?: Schema;
 
   /**
    * The default output key to use for the brick
    * @since 1.7.34
    */
   defaultOutputKey?: string;
-
-  // Mapping from `key` -> `serviceId`
-  services?: Record<string, RegistryId>;
 };
 
+/**
+ * Throw an error if the brick definition is invalid with respect to the brick meta-schema.
+ */
 function validateBrickDefinition(
   component: unknown
 ): asserts component is BrickDefinition {
@@ -97,9 +122,7 @@ function validateBrickDefinition(
 /**
  * A non-native (i.e., non-JS) Brick. Typically defined in YAML/JSON.
  */
-class ExternalBlock extends BrickABC {
-  public readonly component: BrickDefinition;
-
+class UserDefinedBrick extends BrickABC {
   readonly apiVersion: ApiVersion;
 
   readonly inputSchema: Schema;
@@ -108,9 +131,10 @@ class ExternalBlock extends BrickABC {
 
   readonly version: SemVerString;
 
-  constructor(component: BrickDefinition) {
+  constructor(public readonly component: BrickDefinition) {
     const { id, name, description, icon, version } = component.metadata;
     super(id, name, description, icon);
+    // Fall back to v1 for backward compatability
     this.apiVersion = component.apiVersion ?? "v1";
     this.component = component;
     this.inputSchema = this.component.inputSchema;
@@ -240,21 +264,62 @@ class ExternalBlock extends BrickABC {
     // Blocks only have inputs, they can't pick up free variables from the environment
     const initialValues: InitialValues = {
       input: argsWithClosures,
-      // OptionsArgs are set at the blueprint level. For composite bricks, the options should be passed in
-      // as part of the brick inputs
+      // OptionsArgs are set at the blueprint level. For user-defined bricks, are passed via brick inputs
       optionsArgs: undefined,
       // Services are passed as inputs to the brick
       serviceContext: undefined,
       root: options.root,
     };
 
-    return reducePipeline(this.component.pipeline, initialValues, {
-      logger: options.logger,
-      headless: options.headless,
-      // The component uses its declared version of the runtime API, regardless of what version of the runtime
-      // is used to call the component
-      ...apiVersionOptions(this.component.apiVersion),
-    });
+    if (isContentScript()) {
+      // Already in the contentScript, run the pipeline directly.
+      return reducePipeline(this.component.pipeline, initialValues, {
+        logger: options.logger,
+        headless: options.headless,
+        // The component uses its declared version of the runtime API, regardless of what version of the runtime
+        // is used to call the component
+        ...apiVersionOptions(this.component.apiVersion),
+      });
+    }
+
+    // The brick is being run as a renderer from a modal or a sidebar. Run the logic in the contentScript and return the
+    // renderer. The caller can't run the whole brick in the contentScript because renderers can return React
+    // Components which can't be serialized across messenger boundaries.
+
+    // TODO: call top-level contentScript directly after https://github.com/pixiebrix/webext-messenger/issues/72
+    const topLevelFrame = await getTopLevelFrame();
+
+    try {
+      return await runHeadlessPipeline(topLevelFrame, {
+        nonce: uuidv4(),
+        // OptionsArgs are set at the blueprint level. For user-defined bricks, are passed via brick inputs
+        context: validateBrickArgsContext({
+          "@input": argsWithClosures,
+          "@options": {},
+        }),
+        pipeline: castArray(this.component.pipeline),
+        options: apiVersionOptions(this.apiVersion),
+        messageContext: options.logger.context,
+        meta: {
+          // Don't trace within user-defined bricks
+          extensionId: options.logger.context.extensionId,
+          branches: [],
+          runId: null,
+        },
+      });
+    } catch (error) {
+      if (isSpecificError(error, HeadlessModeError)) {
+        const continuation = error;
+        const renderer = await blockRegistry.lookup(continuation.blockId);
+        return renderer.run(continuation.args, {
+          ...options,
+          ctxt: continuation.ctxt,
+          logger: new BackgroundLogger(continuation.loggerContext),
+        });
+      }
+
+      throw error;
+    }
   }
 }
 
@@ -271,5 +336,5 @@ export function fromJS(component: UnknownObject): Brick {
   }
 
   validateBrickDefinition(component);
-  return new ExternalBlock(component);
+  return new UserDefinedBrick(component);
 }

src/components/documentBuilder/render/ButtonElement.tsx

@@ -18,7 +18,7 @@
 import React, { useContext, useState } from "react";
 import { type BrickPipeline } from "@/blocks/types";
 import AsyncButton, { type AsyncButtonProps } from "@/components/AsyncButton";
-import { runEffectPipeline } from "@/contentScript/messenger/api";
+import { runHeadlessPipeline } from "@/contentScript/messenger/api";
 import { uuidv4 } from "@/types/helpers";
 import DocumentContext from "@/components/documentBuilder/render/DocumentContext";
 import { type Except } from "type-fest";
@@ -68,7 +68,7 @@ const ButtonElement: React.FC<ButtonElementProps> = ({
     const topLevelFrame = await getTopLevelFrame();
 
     try {
-      await runEffectPipeline(topLevelFrame, {
+      await runHeadlessPipeline(topLevelFrame, {
         nonce: uuidv4(),
         context: ctxt,
         pipeline: onClick,

src/contentScript/messenger/api.ts

@@ -73,7 +73,7 @@ export const cancelSelect = getMethod("CANCEL_SELECT_ELEMENT");
 export const selectElement = getMethod("SELECT_ELEMENT");
 
 export const runRendererPipeline = getMethod("RUN_RENDERER_PIPELINE");
-export const runEffectPipeline = getMethod("RUN_EFFECT_PIPELINE");
+export const runHeadlessPipeline = getMethod("RUN_HEADLESS_PIPELINE");
 export const runMapArgs = getMethod("RUN_MAP_ARGS");
 
 export const getPageState = getMethod("GET_PAGE_STATE");

src/contentScript/messenger/registration.ts

@@ -64,7 +64,7 @@ import {
   selectElement,
 } from "@/contentScript/pageEditor/elementPicker";
 import {
-  runEffectPipeline,
+  runHeadlessPipeline,
   runMapArgs,
   runRendererPipeline,
 } from "@/contentScript/pipelineProtocol";
@@ -130,7 +130,7 @@ declare global {
     SELECT_ELEMENT: typeof selectElement;
 
     RUN_RENDERER_PIPELINE: typeof runRendererPipeline;
-    RUN_EFFECT_PIPELINE: typeof runEffectPipeline;
+    RUN_HEADLESS_PIPELINE: typeof runHeadlessPipeline;
     RUN_MAP_ARGS: typeof runMapArgs;
 
     NOTIFY_INFO: typeof notify.info;
@@ -195,7 +195,7 @@ export default function registerMessenger(): void {
     SELECT_ELEMENT: selectElement,
 
     RUN_RENDERER_PIPELINE: runRendererPipeline,
-    RUN_EFFECT_PIPELINE: runEffectPipeline,
+    RUN_HEADLESS_PIPELINE: runHeadlessPipeline,
     RUN_MAP_ARGS: runMapArgs,
 
     NOTIFY_INFO: notify.info,

src/contentScript/pipelineProtocol.ts

@@ -102,20 +102,23 @@ export async function runRendererPipeline({
   throw new BusinessError("Pipeline does not include a renderer");
 }
 
-export async function runEffectPipeline({
+/**
+ * Run a pipeline in headless mode.
+ */
+export async function runHeadlessPipeline({
   pipeline,
   context,
   options,
   meta,
   messageContext,
-}: RunPipelineParams): Promise<void> {
+}: RunPipelineParams): Promise<unknown> {
   expectContext("contentScript");
 
   if (meta.extensionId == null) {
-    throw new Error("runEffectPipeline requires meta.extensionId");
+    throw new Error("runHeadlessPipeline requires meta.extensionId");
   }
 
-  await reducePipeline(
+  return reducePipeline(
     pipeline,
     {
       input: context["@input"] ?? {},

src/runtime/pipelineTests/component.test.ts

@@ -27,6 +27,9 @@ import {
 
 import { fromJS } from "@/blocks/transformers/brickFactory";
 import { validateSemVerString } from "@/types/helpers";
+import { setContext } from "@/testUtils/detectPageMock";
+
+setContext("contentScript");
 
 beforeEach(() => {
   blockRegistry.clear();

src/runtime/reducePipeline.ts

@@ -44,7 +44,10 @@ import {
   throwIfInvalidInput,
 } from "@/runtime/runtimeUtils";
 import ConsoleLogger from "@/utils/ConsoleLogger";
-import { type ResolvedBrickConfig } from "@/runtime/runtimeTypes";
+import {
+  type ResolvedBrickConfig,
+  unsafeAssumeValidArg,
+} from "@/runtime/runtimeTypes";
 import { type RunBlock } from "@/contentScript/runBlockTypes";
 import { resolveBlockConfig } from "@/blocks/registry";
 import { isObject } from "@/utils";
@@ -570,7 +573,8 @@ export async function runBlock(
 
     throw new HeadlessModeError(
       block.id,
-      props.args,
+      // Call to throwIfInvalidInput above ensures args are valid for the brick
+      unsafeAssumeValidArg(props.args),
       props.context,
       logger.context
     );

src/sidebar/PanelBody.tsx

@@ -160,8 +160,7 @@ const PanelBody: React.FunctionComponent<{
         console.debug("Running panel body for panel payload", payload);
 
         const block = await blockRegistry.lookup(blockId);
-        // In the future, the renderer brick should run in the contentScript, not the panel frame
-        // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/1939
+
         const body = await block.run(unsafeAssumeValidArg(args), {
           ctxt,
           root: null,

src/sidebar/RendererComponent.test.tsx

@@ -25,27 +25,29 @@ import { screen } from "shadow-dom-testing-library";
 import { act } from "@testing-library/react";
 import { SubmitPanelAction } from "@/blocks/errors";
 import ConsoleLogger from "@/utils/ConsoleLogger";
-import { runEffectPipeline } from "@/contentScript/messenger/api";
+import { runHeadlessPipeline } from "@/contentScript/messenger/api";
 
 jest.mock("@/contentScript/messenger/api", () => ({
-  runEffectPipeline: jest.fn().mockRejectedValue(new Error("not implemented")),
+  runHeadlessPipeline: jest
+    .fn()
+    .mockRejectedValue(new Error("not implemented")),
 }));
 
-const runEffectPipelineMock = runEffectPipeline as jest.MockedFunction<
-  typeof runEffectPipeline
+const runHeadlessPipelineMock = runHeadlessPipeline as jest.MockedFunction<
+  typeof runHeadlessPipeline
 >;
 
 describe("RendererComponent", () => {
   beforeEach(() => {
-    runEffectPipelineMock.mockReset();
+    runHeadlessPipelineMock.mockReset();
   });
 
   test("provide onAction to document renderer", async () => {
     const runId = uuidv4();
     const extensionId = uuidv4();
     const onAction = jest.fn();
 
-    runEffectPipelineMock.mockRejectedValue(
+    runHeadlessPipelineMock.mockRejectedValue(
       new SubmitPanelAction("submit", { foo: "bar" })
     );