fix: address CodeRabbit review findings

mattinannt · mattinannt · commit c0d4dca01f74 · 2026-03-28T15:10:27.000+01:00
diff --git a/packages/react-native/src/components/survey-web-view.tsx b/packages/react-native/src/components/survey-web-view.tsx
@@ -326,6 +326,20 @@ const renderHtml = (
 ): string => {
   const surveyScriptUrl = getSurveyScriptUrl(options.appUrl);
 
+  if (!surveyScriptUrl) {
+    return `
+  <!doctype html>
+  <html>
+    <meta name="viewport" content="initial-scale=1.0, maximum-scale=1.0">
+    <head>
+      <title>Formbricks WebView Survey</title>
+    </head>
+    <body style="overflow: hidden; height: 100vh; margin: 0;">
+    </body>
+  </html>
+  `;
+  }
+
   return `
   <!doctype html>
   <html>
diff --git a/packages/react-native/src/components/tests/survey-script-url.test.ts b/packages/react-native/src/components/tests/survey-script-url.test.ts
@@ -2,8 +2,8 @@ import { describe, expect, test } from "vitest";
 import { getSurveyScriptUrl } from "@/components/utils/survey-script-url";
 
 describe("getSurveyScriptUrl()", () => {
-  test("builds the default local development script URL", () => {
-    expect(getSurveyScriptUrl()).toBe("http://localhost:3000/js/surveys.umd.cjs");
+  test("returns null when appUrl is missing", () => {
+    expect(getSurveyScriptUrl()).toBeNull();
   });
 
   test("builds the script URL from a root-hosted appUrl", () => {
@@ -30,9 +30,11 @@ describe("getSurveyScriptUrl()", () => {
     );
   });
 
-  test("rejects non-http protocols", () => {
-    expect(() => getSurveyScriptUrl("file:///tmp/formbricks")).toThrow(
-      "Formbricks appUrl must use http or https",
-    );
+  test("returns null for non-http protocols", () => {
+    expect(getSurveyScriptUrl("file:///tmp/formbricks")).toBeNull();
+  });
+
+  test("returns null for invalid URLs", () => {
+    expect(getSurveyScriptUrl("not-a-url")).toBeNull();
   });
 });
diff --git a/packages/react-native/src/components/utils/survey-script-url.ts b/packages/react-native/src/components/utils/survey-script-url.ts
@@ -1,14 +1,24 @@
-export const getSurveyScriptUrl = (appUrl?: string): string => {
-  const url = new URL(appUrl ?? "http://localhost:3000");
-
-  if (url.protocol !== "http:" && url.protocol !== "https:") {
-    throw new Error("Formbricks appUrl must use http or https");
+export const getSurveyScriptUrl = (appUrl?: string): string | null => {
+  if (!appUrl) {
+    return null;
   }
 
-  const basePath = url.pathname.endsWith("/") ? url.pathname : `${url.pathname}/`;
-  url.pathname = `${basePath}js/surveys.umd.cjs`;
-  url.search = "";
-  url.hash = "";
+  try {
+    const url = new URL(appUrl);
+
+    if (url.protocol !== "http:" && url.protocol !== "https:") {
+      return null;
+    }
 
-  return url.toString();
+    const basePath = url.pathname.endsWith("/")
+      ? url.pathname
+      : `${url.pathname}/`;
+    url.pathname = `${basePath}js/surveys.umd.cjs`;
+    url.search = "";
+    url.hash = "";
+
+    return url.toString();
+  } catch {
+    return null;
+  }
 };
diff --git a/packages/react-native/src/lib/common/command-queue.ts b/packages/react-native/src/lib/common/command-queue.ts
@@ -1,13 +1,16 @@
 /* eslint-disable no-console -- we need to log global errors */
 import { checkSetup } from "@/lib/common/setup";
 import { wrapThrowsAsync } from "@/lib/common/utils";
-import type { Result } from "@/types/error";
+import { okVoid, type Result } from "@/types/error";
+
+type QueueCommandResult = Result<void, unknown> | void;
+type QueueCommand = (
+  ...args: unknown[]
+) => Promise<QueueCommandResult> | QueueCommandResult;
 
 export class CommandQueue {
   private readonly queue: {
-    command: (
-      ...args: unknown[]
-    ) => Promise<Result<void, unknown>> | Result<void, unknown> | Promise<void>;
+    command: QueueCommand;
     checkSetup: boolean;
     commandArgs: unknown[];
   }[] = [];
@@ -16,19 +19,12 @@ export class CommandQueue {
   private commandPromise: Promise<void> | null = null;
 
   public add<A extends unknown[]>(
-    command: (
-      ...args: A
-    ) => Promise<Result<void, unknown>> | Result<void, unknown> | Promise<void>,
+    command: (...args: A) => Promise<QueueCommandResult> | QueueCommandResult,
     shouldCheckSetup = true,
     ...args: A
   ): void {
     this.queue.push({
-      command: command as (
-        ...commandArgs: unknown[]
-      ) =>
-        | Promise<Result<void, unknown>>
-        | Result<void, unknown>
-        | Promise<void>,
+      command: command as QueueCommand,
       checkSetup: shouldCheckSetup,
       commandArgs: args,
     });
@@ -65,10 +61,9 @@ export class CommandQueue {
       }
 
       const executeCommand = async (): Promise<Result<void, unknown>> =>
-        (await currentItem.command.apply(
-          null,
-          currentItem.commandArgs,
-        )) as Result<void, unknown>;
+        normalizeQueueCommandResult(
+          await currentItem.command.apply(null, currentItem.commandArgs),
+        );
 
       const result = await wrapThrowsAsync(executeCommand)();
 
@@ -86,3 +81,17 @@ export class CommandQueue {
     }
   }
 }
+
+const normalizeQueueCommandResult = (
+  value: QueueCommandResult,
+): Result<void, unknown> => {
+  if (!value) {
+    return okVoid();
+  }
+
+  if ("ok" in value && typeof value.ok === "boolean") {
+    return value;
+  }
+
+  return okVoid();
+};
diff --git a/packages/react-native/src/lib/common/tests/command-queue.test.ts b/packages/react-native/src/lib/common/tests/command-queue.test.ts
@@ -86,6 +86,24 @@ describe("CommandQueue", () => {
     expect(cmd).toHaveBeenCalledTimes(1);
   });
 
+  test("treats Promise<void> commands as successful", async () => {
+    const consoleErrorSpy = vi
+      .spyOn(console, "error")
+      .mockImplementation(() => undefined);
+    const cmd = vi.fn(async (): Promise<void> => {
+      await delayedResult(undefined, 10);
+    });
+
+    vi.mocked(checkSetup).mockReturnValue({ ok: true, data: undefined });
+
+    queue.add(cmd, true);
+    await queue.wait();
+
+    expect(cmd).toHaveBeenCalledTimes(1);
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
+    consoleErrorSpy.mockRestore();
+  });
+
   test("logs errors if a command throws or returns error", async () => {
     // Spy on console.error to see if it's called
     const consoleErrorSpy = vi
diff --git a/packages/react-native/src/types/response.test.ts b/packages/react-native/src/types/response.test.ts
@@ -0,0 +1,28 @@
+import { describe, expect, test } from "vitest";
+import { ZResponseData, ZResponseUpdate } from "@/types/response";
+
+describe("response schemas", () => {
+  test("accepts nested string records in response data", () => {
+    const result = ZResponseData.safeParse({
+      simple: "value",
+      nested: {
+        key: "value",
+      },
+    });
+
+    expect(result.success).toBe(true);
+  });
+
+  test("accepts nested string records in response updates", () => {
+    const result = ZResponseUpdate.safeParse({
+      finished: true,
+      data: {
+        nested: {
+          key: "value",
+        },
+      },
+    });
+
+    expect(result.success).toBe(true);
+  });
+});
diff --git a/packages/react-native/src/types/response.ts b/packages/react-native/src/types/response.ts
@@ -26,9 +26,10 @@ export interface TResponseUpdate {
   endingId?: string | null;
 }
 
+const ZNestedResponseData = z.record(z.string(), z.string());
 export const ZResponseData = z.record(
   z.string(),
-  z.union([z.string(), z.number(), z.array(z.string())]),
+  z.union([z.string(), z.number(), z.array(z.string()), ZNestedResponseData]),
 );
 export const ZResponseVariables = z.record(
   z.string(),
