fix: add basePath support in mountApp() (#3395)

fry69 · web-flow · commit 3010e27097be · 2025-09-12T10:06:06.000+02:00
Fix for mountApp() with the new basePath handling

---
Generated PR message:

I successfully investigated and implemented a fix for mountApp() to make
it compatible with basePath handling. Here's what I found and fixed:

  The Problem

The original mountApp() implementation completely ignored the inner
app's basePath. When mounting an app like this:

```ts
  const innerApp = new App({ basePath: "/api" })
    .get("/users", () =&gt; new Response("users"));

  const app = new App()
    .mountApp("/v1", innerApp);
```

The route would become /v1/users instead of the expected /v1/api/users
because the inner app's /api basePath was lost.

  Root Cause

The issue was in the mountApp() method at packages/fresh/src/app.ts:333.
The original code only used:

```ts
  pattern: mergePath(path, cmd.pattern, true)
```

This only considered the mount path (/v1) and the command pattern
(/users), but ignored app.config.basePath (/api).

  The Fix

I modified the mountApp() method to properly handle the inner app's
basePath:

```ts
  // Apply the inner app's basePath if it exists
  let effectivePattern = cmd.pattern;
  if (app.config.basePath) {
    effectivePattern = mergePath(app.config.basePath, cmd.pattern, false);
  }

  const clone = {
    ...cmd,
    pattern: mergePath(path, effectivePattern, true),
    includeLastSegment: cmd.pattern === "/" || cmd.includeLastSegment,
  };
```

The key insight was to only apply the basePath transformation when the
inner app actually has a basePath (not empty), which preserves backward
  compatibility with existing code that doesn't use basePath.

  Test Coverage

  I added comprehensive test cases covering:

1. Inner app with basePath: /v1 + /api basePath + /users route =
/v1/api/users
2. Main app with basePath + mounted app: /main basePath + /sub mount +
/data route = /main/sub/data
3. Both main and inner basePath: /main + /services + /api/v2 + /users =
/main/services/api/v2/users

  Backward Compatibility

  The fix maintains full backward compatibility:
  - ✅ All existing mountApp() tests pass
  - ✅ All existing basePath tests pass
  - ✅ New functionality works as expected

  Now the three basePath mechanisms work correctly together:

  1. Vite base config → Static asset URLs
  2. App basePath option → Route matching (now works with mountApp!)
  3. External mounting → Framework integration

The solution correctly handles the scenario you described where
vite.config.ts, main.ts, and external mounting all work together
seamlessly.
diff --git a/packages/fresh/src/app.ts b/packages/fresh/src/app.ts
@@ -328,9 +328,15 @@ export class App<State> {
       const cmd = app.#commands[i];
 
       if (cmd.type !== CommandType.App && cmd.type !== CommandType.NotFound) {
+        // Apply the inner app's basePath if it exists
+        let effectivePattern = cmd.pattern;
+        if (app.config.basePath) {
+          effectivePattern = mergePath(app.config.basePath, cmd.pattern, false);
+        }
+
         const clone = {
           ...cmd,
-          pattern: mergePath(path, cmd.pattern, true),
+          pattern: mergePath(path, effectivePattern, true),
           includeLastSegment: cmd.pattern === "/" || cmd.includeLastSegment,
         };
         this.#commands.push(clone);
diff --git a/packages/fresh/src/app_test.tsx b/packages/fresh/src/app_test.tsx
@@ -777,3 +777,75 @@ Deno.test("App - ctx.render() without app wrapper or layout", async () => {
   const res = await server.get("/");
   expect(await res.text()).toContain("<body>index<");
 });
+
+Deno.test("App - .mountApp() with basePath", async () => {
+  const innerApp = new App({ basePath: "/api" })
+    .get("/users", () => new Response("users"));
+
+  const app = new App()
+    .get("/", () => new Response("home"))
+    .mountApp("/v1", innerApp);
+
+  const server = new FakeServer(app.handler());
+
+  let res = await server.get("/");
+  expect(await res.text()).toEqual("home");
+
+  // Fixed behavior: inner app's basePath is now applied
+  res = await server.get("/v1/api/users");
+  expect(await res.text()).toEqual("users");
+
+  // Routes without the full basePath should not work
+  res = await server.get("/v1/users");
+  expect(res.status).toEqual(404);
+});
+
+Deno.test("App - .mountApp() with main app basePath", async () => {
+  const innerApp = new App()
+    .get("/data", () => new Response("data"));
+
+  const app = new App({ basePath: "/main" })
+    .get("/", () => new Response("home"))
+    .mountApp("/sub", innerApp);
+
+  const server = new FakeServer(app.handler());
+
+  let res = await server.get("/main");
+  expect(await res.text()).toEqual("home");
+
+  res = await server.get("/main/sub/data");
+  expect(await res.text()).toEqual("data");
+
+  // Should not work without main basePath
+  res = await server.get("/sub/data");
+  expect(res.status).toEqual(404);
+});
+
+Deno.test("App - .mountApp() with both main and inner basePath", async () => {
+  const innerApp = new App({ basePath: "/api/v2" })
+    .get("/users", () => new Response("users"))
+    .get("/posts", () => new Response("posts"));
+
+  const app = new App({ basePath: "/main" })
+    .get("/", () => new Response("home"))
+    .mountApp("/services", innerApp);
+
+  const server = new FakeServer(app.handler());
+
+  let res = await server.get("/main");
+  expect(await res.text()).toEqual("home");
+
+  // Both basePaths should be applied: main basePath + mount path + inner basePath
+  res = await server.get("/main/services/api/v2/users");
+  expect(await res.text()).toEqual("users");
+
+  res = await server.get("/main/services/api/v2/posts");
+  expect(await res.text()).toEqual("posts");
+
+  // Partial paths should not work
+  res = await server.get("/services/api/v2/users");
+  expect(res.status).toEqual(404);
+
+  res = await server.get("/main/services/users");
+  expect(res.status).toEqual(404);
+});
