fix batch of open issues against main

- useSubmission retry/clear no longer stub out when a submission exists (#504)
- disposing an older owner no longer unregisters a newer action with the same URL (#542)
- useBeforeLeave listeners observe preventDefault from other listeners (#530)
- <A> active state ignores trailing slashes (#532)
- useCurrentMatches returns a copy to protect router state (#516)
- static path segments keep RFC 3986 pchar characters literal so +foo/@user match (#559, #509)
- consecutive synchronous setSearchParams calls compose via pending navigation target (#547)

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: ryansolid

.changeset/open-issue-audit-fixes.md

@@ -0,0 +1,13 @@
+---
+"@solidjs/router": patch
+---
+
+Fix a batch of long-standing bugs:
+
+- `useSubmission().retry` was always a no-op due to an operator-precedence bug (#504)
+- disposing an older owner no longer unregisters a newer action bound to the same URL, which caused forms to fall through to native submission after revalidation (#542)
+- `useBeforeLeave` listeners now observe `defaultPrevented` set by other listeners (#530)
+- `<A>` active state now ignores trailing slashes on `href` (#532)
+- `useCurrentMatches` returns a copy so user mutation can't corrupt router state (#516)
+- static path segments no longer percent-encode RFC 3986 pchar characters (`+`, `@`, `:`, `$`, `&`, `,`, `;`, `=`), so routes like `+foo` or `@user` match the browser's raw pathname (#559, #509)
+- consecutive synchronous `setSearchParams` calls now compose: the merge applies to the in-flight navigation target instead of the stale committed location (#547)

src/components.tsx

@@ -50,8 +50,9 @@ export function A(props: AnchorProps) {
   const isActive = createMemo(() => {
     const to_ = to();
     if (to_ === undefined) return [false, false];
-    const path = normalizePath(to_.split(/[?#]/, 1)[0]).toLowerCase();
-    const loc = decodeURI(normalizePath(location.pathname).toLowerCase());
+    // trailing slashes are ignored so `/route` and `/route/` share active state
+    const path = normalizePath(to_.split(/[?#]/, 1)[0]).toLowerCase().replace(/\/$/, "");
+    const loc = decodeURI(normalizePath(location.pathname).toLowerCase().replace(/\/$/, ""));
     return [props.end ? path === loc : loc.startsWith(path + "/") || loc === path, path === loc];
   });
 

src/data/action.ts

@@ -53,7 +53,7 @@ export function useSubmission<T extends Array<any>, U, V>(
     {},
     {
       get(_, property) {
-        if ((submissions.length === 0 && property === "clear") || property === "retry")
+        if (submissions.length === 0 && (property === "clear" || property === "retry"))
           return () => {};
         return submissions[submissions.length - 1]?.[property as keyof Submission<T, U>];
       }
@@ -165,7 +165,10 @@ function toAction<T extends Array<any>, U, V = T>(fn: Function, url: string): Ac
   (fn as any).url = url;
   if (!isServer) {
     actions.set(url, fn as Action<T, U, V>);
-    getOwner() && onCleanup(() => actions.delete(url));
+    // Only remove the registration if it still belongs to this instance —
+    // a re-created action (e.g. a new `.with()` binding after revalidation)
+    // may have registered itself under the same URL since.
+    getOwner() && onCleanup(() => actions.get(url) === fn && actions.delete(url));
   }
   return fn as Action<T, U, V>;
 }

src/lifecycle.ts

@@ -20,7 +20,14 @@ export function createBeforeLeave(): BeforeLeaveLifecycle {
     };
     for (const l of listeners)
       l.listener({
-        ...e,
+        to,
+        options,
+        // delegate to the shared event so later listeners' preventDefault
+        // calls are observable from earlier listeners
+        get defaultPrevented() {
+          return e.defaultPrevented;
+        },
+        preventDefault: e.preventDefault,
         from: l.location,
         retry: (force?: boolean) => {
           force && (ignore = true);

src/routing.ts

@@ -179,7 +179,11 @@ export const useMatch = <S extends string>(path: () => S, matchFilters?: MatchFi
  * const breadcrumbs = createMemo(() => matches().map(m => m.route.info.breadcrumb))
  * ```
  */
-export const useCurrentMatches = () => useRouter().matches;
+export const useCurrentMatches = () => {
+  const router = useRouter();
+  // return a copy so user mutations (eg. `.reverse()`) can't corrupt router state
+  return () => router.matches().slice();
+};
 
 /**
  * Retrieves a reactive, store-like object containing the current route path parameters as defined in the Route.
@@ -225,11 +229,20 @@ export const useSearchParams = <T extends SearchParams>(): [
   Partial<T>,
   (params: SetSearchParams, options?: Partial<NavigateOptions>) => void
 ] => {
-  const location = useLocation();
+  const router = useRouter();
+  const location = router.location;
   const navigate = useNavigate();
   const setSearchParams = (params: SetSearchParams, options?: Partial<NavigateOptions>) => {
-    const searchString = untrack(() => mergeSearchString(location.search, params) + location.hash);
-    navigate(searchString, {
+    const to = untrack(() => {
+      // merge onto the in-flight navigation target (if any) so consecutive
+      // synchronous calls compose instead of the later one winning
+      const pending = router.pendingTarget && new URL(router.pendingTarget.value, mockBase);
+      const pathname = pending ? pending.pathname : location.pathname;
+      const search = pending ? pending.search : location.search;
+      const hash = pending ? pending.hash : location.hash;
+      return pathname + mergeSearchString(search, params) + hash;
+    });
+    navigate(to, {
       scroll: false,
       resolve: false,
       ...options
@@ -274,6 +287,13 @@ export const useBeforeLeave = (listener: (e: BeforeLeaveEventArgs) => void) => {
   onCleanup(s);
 };
 
+// Encodes a static path segment like `encodeURIComponent`, but leaves RFC 3986
+// pchar characters (sub-delims / ":" / "@") literal, matching how browsers
+// report them in `location.pathname`. Non-ASCII characters (eg. CJK paths) are
+// still percent-encoded exactly as before, since browsers encode those too.
+const encodeSegment = (s: string) =>
+  encodeURIComponent(s).replace(/%(2B|40|3A|24|26|2C|3B|3D)/g, m => decodeURIComponent(m));
+
 export function createRoutes(routeDef: RouteDefinition, base: string = ""): RouteDescription[] {
   const { component, preload, load, children, info } = routeDef;
   const isLeaf = !children || (Array.isArray(children) && !children.length);
@@ -292,7 +312,7 @@ export function createRoutes(routeDef: RouteDefinition, base: string = ""): Rout
       pattern = pattern
         .split("/")
         .map((s: string) => {
-          return s.startsWith(":") || s.startsWith("*") ? s : encodeURIComponent(s);
+          return s.startsWith(":") || s.startsWith("*") ? s : encodeSegment(s);
         })
         .join("/");
       acc.push({
@@ -534,6 +554,9 @@ export function createRouterContext(
     location,
     params,
     isRouting,
+    get pendingTarget() {
+      return lastTransitionTarget;
+    },
     renderPath,
     parsePath,
     navigatorFactory,

src/types.ts

@@ -176,6 +176,8 @@ export interface RouterContext {
   params: Params;
   navigatorFactory: NavigatorFactory;
   isRouting: () => boolean;
+  /** The target of the in-flight navigation transition, if any. Not reactive. */
+  readonly pendingTarget?: LocationChange;
   matches: () => RouteMatch[];
   renderPath(path: string): string;
   parsePath(str: string): string;

test/data/action.spec.ts

@@ -5,7 +5,8 @@ import {
   useAction,
   useSubmission,
   useSubmissions,
-  actions
+  actions,
+  type Action
 } from "../../src/data/action.js";
 import type { RouterContext } from "../../src/types.js";
 import { createMockRouter } from "../helpers.js";
@@ -85,6 +86,42 @@ describe("action", () => {
     expect(actions.get(testAction.url)).toBe(testAction);
   });
 
+  test("disposing an older owner should not unregister a newer action with the same URL", () => {
+    const base = action(async (id: string, data: string) => `${id}: ${data}`, "cleanup-test");
+
+    let disposeFirst!: () => void;
+    let first!: ReturnType<typeof base.with>;
+    createRoot(dispose => {
+      disposeFirst = dispose;
+      first = base.with("same-args");
+    });
+
+    let second!: ReturnType<typeof base.with>;
+    createRoot(() => {
+      second = base.with("same-args");
+    });
+
+    expect(first.url).toBe(second.url);
+    expect(actions.get(second.url)).toBe(second);
+
+    // the older owner disposing must not delete the newer registration
+    disposeFirst();
+    expect(actions.get(second.url)).toBe(second);
+  });
+
+  test("disposing the current owner should unregister its action", () => {
+    let dispose!: () => void;
+    let registered!: Action<[], string>;
+    createRoot(d => {
+      dispose = d;
+      registered = action(async () => "result", "self-cleanup-test");
+    });
+
+    expect(actions.get(registered.url)).toBe(registered);
+    dispose();
+    expect(actions.has(registered.url)).toBe(false);
+  });
+
   test("should support `.with` method for currying arguments", () => {
     const baseAction = action(async (prefix: string, data: string) => {
       return `${prefix}: ${data}`;
@@ -302,6 +339,34 @@ describe("useSubmission", () => {
     });
   });
 
+  test("retry and clear should invoke the underlying submission when one exists", () => {
+    return createRoot(() => {
+      const testAction = action(async () => "result", "retry-test");
+      const retry = vi.fn();
+      const clear = vi.fn();
+
+      mockRouterContext.submissions[1](submissions => [
+        ...submissions,
+        {
+          input: ["data"],
+          url: testAction.url,
+          result: "result",
+          error: undefined,
+          pending: false,
+          clear,
+          retry
+        }
+      ]);
+
+      const submission = useSubmission(testAction);
+      (submission.retry as () => void)();
+      (submission.clear as () => void)();
+
+      expect(retry).toHaveBeenCalledTimes(1);
+      expect(clear).toHaveBeenCalledTimes(1);
+    });
+  });
+
   test("should filter submissions when filter function provided", () => {
     return createRoot(() => {
       const testAction = action(async (data: string) => data, "filter-submission-test");

test/lifecycle.spec.ts

@@ -0,0 +1,51 @@
+import { vi } from "vitest";
+import { createBeforeLeave } from "../src/lifecycle.js";
+import type { BeforeLeaveEventArgs, Location } from "../src/types.js";
+
+const mockLocation = { pathname: "/", search: "", hash: "", query: {}, state: null, key: "" } as unknown as Location;
+
+describe("createBeforeLeave", () => {
+  test("confirm returns false when a listener prevents default", () => {
+    const beforeLeave = createBeforeLeave();
+    beforeLeave.subscribe({
+      listener: e => e.preventDefault(),
+      location: mockLocation,
+      navigate: vi.fn()
+    });
+
+    expect(beforeLeave.confirm("/next")).toBe(false);
+  });
+
+  test("earlier listeners observe preventDefault called by later listeners", () => {
+    const beforeLeave = createBeforeLeave();
+    let captured!: BeforeLeaveEventArgs;
+
+    beforeLeave.subscribe({
+      listener: e => (captured = e),
+      location: mockLocation,
+      navigate: vi.fn()
+    });
+    beforeLeave.subscribe({
+      listener: e => e.preventDefault(),
+      location: mockLocation,
+      navigate: vi.fn()
+    });
+
+    beforeLeave.confirm("/next");
+    expect(captured.defaultPrevented).toBe(true);
+  });
+
+  test("unsubscribe removes the listener", () => {
+    const beforeLeave = createBeforeLeave();
+    const listener = vi.fn();
+    const unsubscribe = beforeLeave.subscribe({
+      listener,
+      location: mockLocation,
+      navigate: vi.fn()
+    });
+
+    unsubscribe();
+    expect(beforeLeave.confirm("/next")).toBe(true);
+    expect(listener).not.toHaveBeenCalled();
+  });
+});

test/route.spec.ts

@@ -563,4 +563,21 @@ describe("createBranches should", () => {
 
     expect(branchPaths).toEqual(["/root/%E3%81%BB%E3%81%92/:ふが/*ぴよ"]);
   });
+
+  test(`not encode RFC 3986 pchar characters in static segments`, () => {
+    // browsers keep sub-delims / ":" / "@" literal in location.pathname,
+    // so encoding them made these routes unmatchable (#559, #509)
+    const route = createRoute({ path: "+foo/@user/a=b" });
+
+    expect(route.pattern).toBe("/+foo/@user/a=b");
+    expect(route.matcher("/+foo/@user/a=b")).not.toBeNull();
+    expect(route.matcher("/%2Bfoo/@user/a=b")).toBeNull();
+  });
+
+  test(`still encode non-pchar characters in static segments`, () => {
+    const route = createRoute({ path: "foo bar/ほげ" });
+
+    expect(route.pattern).toBe("/foo%20bar/%E3%81%BB%E3%81%92");
+    expect(route.matcher("/foo%20bar/%E3%81%BB%E3%81%92")).not.toBeNull();
+  });
 });

test/search-params.spec.tsx

@@ -0,0 +1,71 @@
+import { render } from "solid-js/web";
+import { MemoryRouter, Route, useSearchParams, useNavigate, useLocation } from "../src/index.js";
+import type { Location, Navigator } from "../src/index.js";
+import { awaitPromise } from "./helpers.js";
+
+describe("useSearchParams", () => {
+  test("two synchronous setSearchParams calls both apply", async () => {
+    let set!: ReturnType<typeof useSearchParams>[1];
+    let params!: ReturnType<typeof useSearchParams>[0];
+
+    const Index = () => {
+      [params, set] = useSearchParams();
+      return null;
+    };
+
+    const dispose = render(
+      () => (
+        <MemoryRouter>
+          <Route path="/" component={Index} />
+        </MemoryRouter>
+      ),
+      document.body
+    );
+
+    try {
+      set({ a: "1" });
+      set({ b: "2" });
+      await awaitPromise();
+      expect(params.a).toBe("1");
+      expect(params.b).toBe("2");
+    } finally {
+      document.body.innerHTML = "";
+      dispose();
+    }
+  });
+
+  test("setSearchParams during a pending navigation applies to the target route", async () => {
+    let set!: ReturnType<typeof useSearchParams>[1];
+    let navigate!: Navigator;
+    let location!: Location;
+
+    const Index = () => {
+      [, set] = useSearchParams();
+      navigate = useNavigate();
+      location = useLocation();
+      return null;
+    };
+
+    const dispose = render(
+      () => (
+        <MemoryRouter>
+          <Route path="/" component={Index} />
+          <Route path="/other" component={() => null} />
+        </MemoryRouter>
+      ),
+      document.body
+    );
+
+    try {
+      await awaitPromise();
+      navigate("/other", { scroll: false });
+      set({ a: "1" });
+      await awaitPromise();
+      expect(location.pathname).toBe("/other");
+      expect(location.search).toBe("?a=1");
+    } finally {
+      document.body.innerHTML = "";
+      dispose();
+    }
+  });
+});