Merge pull request #7733 from maiieul/fix-link-and-redirect-wrong-url

Author: shairez

.changeset/every-socks-beg.md

@@ -0,0 +1,5 @@
+---
+'@builder.io/qwik-city': patch
+---
+
+FIX: link/useNavigate with query params don't override loader/middleware redirect with query params anymore.

packages/qwik-city/src/buildtime/build-layout.unit.ts

@@ -3,7 +3,7 @@ import { assert, testAppSuite } from '../utils/test-suite';
 const test = testAppSuite('Build Layout');
 
 test('total layouts', ({ ctx: { layouts } }) => {
-  assert.equal(layouts.length, 11, JSON.stringify(layouts, null, 2));
+  assert.equal(layouts.length, 12, JSON.stringify(layouts, null, 2));
 });
 
 test('nested named layout', ({ assertLayout }) => {

packages/qwik-city/src/runtime/src/qwik-city-component.tsx

@@ -383,8 +383,7 @@ export const QwikCityProvider = component$<QwikCityProps>((props) => {
         const pageModule = contentModules[contentModules.length - 1] as PageModule;
 
         // Restore search params unless it's a redirect
-        const isRedirect = navType === 'form' && !isSamePath(trackUrl, prevUrl);
-        if (navigation.dest.search && !isRedirect) {
+        if (navigation.dest.search && !!isSamePath(trackUrl, prevUrl)) {
           trackUrl.search = navigation.dest.search;
         }
 
@@ -430,7 +429,8 @@ export const QwikCityProvider = component$<QwikCityProps>((props) => {
             (navigation.scroll &&
               (!navigation.forceReload || !isSamePath(trackUrl, prevUrl)) &&
               (navType === 'link' || navType === 'popstate')) ||
-            isRedirect
+            // Action might have responded with a redirect.
+            (navType === 'form' && !isSamePath(trackUrl, prevUrl))
           ) {
             // Mark next DOM render to scroll.
             (document as any).__q_scroll_restore__ = () =>

starters/apps/qwikcity-test/src/routes/issue-route-with-search-params-and-action-redirect-without-search-params/action-redirect-without-search-params-target/index.tsx

@@ -0,0 +1,15 @@
+import { useLocation } from "@builder.io/qwik-city";
+import { component$ } from "@builder.io/qwik";
+
+export default component$(() => {
+  const location = useLocation();
+
+  return (
+    <div>
+      <h1>
+        Should <strong>not</strong> have searchParams
+      </h1>
+      <pre>{JSON.stringify(location.url.searchParams.get("id"))}</pre>
+    </div>
+  );
+});

starters/apps/qwikcity-test/src/routes/issue-route-with-search-params-and-action-redirect-without-search-params/action-redirect-without-search-params/index.tsx

@@ -0,0 +1,20 @@
+import { routeAction$, Form } from "@builder.io/qwik-city";
+import { component$ } from "@builder.io/qwik";
+
+export const useAction = routeAction$((_, event) => {
+  throw event.redirect(
+    302,
+    "/qwikcity-test/action-redirect-without-search-params-target/",
+  );
+});
+
+export default component$(() => {
+  const action = useAction();
+
+  return (
+    <Form action={action}>
+      <h1>Should have searchParams on the url</h1>
+      <button type="submit">Submit</button>
+    </Form>
+  );
+});

starters/apps/qwikcity-test/src/routes/issue7732/a/index.tsx

@@ -0,0 +1,5 @@
+import { component$ } from "@builder.io/qwik";
+
+export default component$(() => {
+  return <div>empty default route A</div>;
+});

starters/apps/qwikcity-test/src/routes/issue7732/b/index.tsx

@@ -0,0 +1,10 @@
+import { component$ } from "@builder.io/qwik";
+import type { RequestHandler } from "@builder.io/qwik-city";
+
+export const onGet: RequestHandler<void> = ({ redirect }) => {
+  throw redirect(302, `/qwikcity-test/issue7732/c/?redirected=true`);
+};
+
+export default component$(() => {
+  return <div>B route with redirect</div>;
+});

starters/apps/qwikcity-test/src/routes/issue7732/c/index.tsx

@@ -0,0 +1,5 @@
+import { component$ } from "@builder.io/qwik";
+
+export default component$(() => {
+  return <div>route C should have redirected=true params</div>;
+});

starters/apps/qwikcity-test/src/routes/issue7732/layout.tsx

@@ -0,0 +1,25 @@
+import { component$, Slot } from "@builder.io/qwik";
+import { Link } from "@builder.io/qwik-city";
+
+export default component$(() => {
+  return (
+    <>
+      <div style={{ display: "flex", gap: "10px" }}>
+        <Link href="/issue7732/a/" id="issue7732-link-a">
+          A
+        </Link>
+
+        <Link
+          href="/qwikcity-test/issue7732/b/?shouldOverrideRedirect=no"
+          id="issue7732-link-b"
+        >
+          B
+        </Link>
+        <Link href="/issue7732/c/" id="issue7732-link-c">
+          C
+        </Link>
+      </div>
+      <Slot />
+    </>
+  );
+});

starters/e2e/qwikcity/nav.spec.ts

@@ -1,3 +1,4 @@
+import { $ } from "bun";
 import {
   assertPage,
   getScrollHeight,
@@ -447,7 +448,30 @@ test.describe("actions", () => {
         await expect(result).toHaveText("3");
       }
     });
-
+    test("issue7732 link/useNavigate with query params should not override loader/middleware redirect with query params", async ({
+      page,
+    }) => {
+      await page.goto("/qwikcity-test/issue7732/a/");
+      const link = page.locator("#issue7732-link-b");
+      await link.click();
+      await expect(page).toHaveURL(
+        "/qwikcity-test/issue7732/c/?redirected=true",
+      );
+    });
+    // TODO: Fix this test (currently not working because the action redirect adds a `/q-data.json` at the end of the path)
+    test.fixme(
+      "action with redirect without query params in a route with query param should redirect to route without query params",
+      async ({ page }) => {
+        await page.goto(
+          "/qwikcity-test/action-redirect-without-search-params/?test=test",
+        );
+        const button = page.locator("button");
+        await button.click();
+        await page.waitForURL(
+          "/qwikcity-test/action-redirect-without-search-params-target/",
+        );
+      },
+    );
     test("media in home page", async ({ page }) => {
       await page.goto("/qwikcity-test/");