From 7d028e271d359f09d1b27b9122845cf1da47a4f9 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Tue, 10 Sep 2024 00:45:52 +0200
Subject: [PATCH] frontend-plugin-api: refuse output override without factory

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 .../src/wiring/createExtension.test.ts        | 30 ++++++++++++++++++-
 .../src/wiring/createExtension.ts             |  9 ++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts
index ee79c277d0..271415fda4 100644
--- a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts
+++ b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts
@@ -588,7 +588,7 @@ describe('createExtension', () => {
         },
       });
 
-      // @ts-expect-error - this should fail because string output should be merged?
+      // @ts-expect-error
       const override2 = testExtension.override({
         output: [numberDataRef],
         factory(_, { inputs }) {
@@ -740,6 +740,34 @@ describe('createExtension', () => {
       ).toBe(undefined);
     });
 
+    it('should complain when overriding with incompatible output', () => {
+      const testExtension = createExtension({
+        namespace: 'test',
+        attachTo: { id: 'root', input: 'blob' },
+        output: [stringDataRef],
+        factory() {
+          return [stringDataRef('0')];
+        },
+      });
+
+      // @ts-expect-error - override output is incompatible with factory
+      const override = testExtension.override({
+        output: [numberDataRef],
+        factory() {
+          return [stringDataRef('1')];
+        },
+      });
+      expect(override).toBeDefined();
+
+      expect(() =>
+        testExtension.override({
+          output: [numberDataRef],
+        }),
+      ).toThrowErrorMatchingInlineSnapshot(
+        `"Refused to override output without also overriding factory"`,
+      );
+    });
+
     it('should be able to override input values', () => {
       const outputRef = createExtensionDataRef<unknown>().with({
         id: 'output',
diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts
index 1dcc2fef6f..d6f339ef6d 100644
--- a/packages/frontend-plugin-api/src/wiring/createExtension.ts
+++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts
@@ -430,6 +430,15 @@ export function createExtension<
         UFactoryOutput
       >;
 
+      // TODO(Rugvip): Making this a type check would be optimal, but it seems
+      //               like it's tricky to add that and still have the type
+      //               inference work correctly for the factory output.
+      if (overrideOptions.output && !overrideOptions.factory) {
+        throw new Error(
+          'Refused to override output without also overriding factory',
+        );
+      }
+
       return createExtension({
         kind: newOptions.kind,
         namespace: newOptions.namespace,