Skip to content

fix(router): treat /application as outside base="/app" - #572

Merged
molefrog merged 22 commits into
molefrog:mainfrom
damiengrandi:fix/base-prefix-boundary
Sep 14, 2026
Merged

molefrog merged 22 commits into
molefrog:mainfrom
damiengrandi:fix/base-prefix-boundary

Conversation

@damiengrandi

@damiengrandi damiengrandi commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Description

Router with base="/app" treated /application as in-base (lication) because relativePath used a string prefix (indexOf === 0), not a path segment.

It now matches only when the path is the base, or continues with / / %2F (decodeURI leaves %2F encoded, so /app%2Fusers still yields %2Fusers).

To reproduce

Save as repro.mjs at the repo root, then run without this PR:

node repro.mjs

import { relativePath } from "./packages/wouter/src/paths.js";

const cases = [
  {
    base: "/app",
    path: "/app/users",
    expected: "/users",
    note: "real child of /app — should stay in-base",
  },
  {
    base: "/app",
    path: "/application",
    expected: "~/application",
    note: "different path that only shares the /app prefix",
  },
  {
    base: "/app",
    path: "/apple",
    expected: "~/apple",
    note: "same prefix collision",
  },
  {
    base: "/MyApp",
    path: "/MyOtherApp",
    expected: "~/MyOtherApp",
    note: "already OK — not a string prefix",
  },
];

for (const { base, path, expected, note } of cases) {
  const got = relativePath(base, path);
  const ok = got === expected;

  console.log(ok ? "ok  " : "FAIL");
  console.log(`  base=${base}  path=${path}`);
  console.log(`  expected ${JSON.stringify(expected)}`);
  console.log(`  got      ${JSON.stringify(got)}`);
  console.log(`  ${note}`);
  console.log("");
}

2/4 fail

To test the fix

  • Run bun test packages/wouter/test/path-normalization.test.tsx
  • Execute node repro.mjs again: no fail

Screenshots

bun test after the fix:

image

molefrog and others added 21 commits September 5, 2026 13:47
Target ES2022, require React 18.2 and TypeScript 5.2, and retain Preact 10 support. Remove obsolete compatibility code, fix optional parameter caching, and improve route and navigation declarations.

Add framework and type compatibility matrices, SSR coverage, and dependency-inclusive size measurements. Verified runtime tests, all supported React versions, strict type consumers, lint, formatting, package exports, and bundle limits.
@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Copy Markdown
Owner

Thanks, the fix is correct and brings Router base in line with how <Route nest> already matches (regexparam's loose (?=$|\/) lookahead). Full suite, lint and size pass here.

One request before merging: drop the %2f branch. %2F is a literal slash inside a single segment, so /app%2Fusers is not a child of /app, and regexparam already treats it that way. The extra branch also costs ~30 B in a file with 69 B of headroom. This keeps the bundle at its current size:

return !base ||
  (path.toLowerCase() + "/").startsWith(base.toLowerCase() + "/")
  ? path.slice(base.length) || "/"
  : "~" + path;

Then flip the existing expectation to ["/app", "/app%2Fusers", "~/app%2Fusers"], which still verifies that %2F is not decoded. Tests, lint and size are green with this applied.


Generated by Claude Code

@molefrog

Copy link
Copy Markdown
Owner

Hey @damiengrandi thank you for your PR. sorry for the AI-generated reply, I just asked Claude to review it. And I think it's a good point and we should drop that check. Could you please update it? I'd be happy to merge this once this fix lands.

Alexey.

@damiengrandi

Copy link
Copy Markdown
Contributor Author

Hi @molefrog, it's done!

@molefrog
molefrog changed the base branch from v3 to main September 14, 2026 20:54
@molefrog
molefrog merged commit ba780cd into molefrog:main Sep 14, 2026
10 checks passed
@molefrog

Copy link
Copy Markdown
Owner

Published in v3.11.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants