tatethurston / nextjs-routes

Type safe routing for Next.js
MIT License
557 stars 21 forks source link

useRouter `push` function miss the property `pathname` #106

Closed ChristoRibeiro closed 1 year ago

ChristoRibeiro commented 1 year ago

Great lib and great work on that!

I'm using the nextjs-routes v1.0.4 and in the README example :

import { useRouter } from "next/link";

const router = useRouter();
router.push({ pathname: "/foos/[foo]", query: { foo: "test" } });
  1. the import should be from "next/router"
  2. the property pathname from router.push is not available but query does.

Can you confirm @tatethurston ?

ChristoRibeiro commented 1 year ago

The core.ts#L230 should get this change?

push(
-  url: { query?: { [key: string]: string | string[] | undefined } },
+   url: {
+     pathname: Route,
+     query?: { [key: string]: string | string[] | undefined }
+  }, 
  as?: string,
  options?: TransitionOptions
): Promise<boolean>;
ChristoRibeiro commented 1 year ago

Looks like <Link href={{ ... }} miss too the pathname property.

tatethurston commented 1 year ago

The core.ts#L230 should get this change?

push(
-  url: { query?: { [key: string]: string | string[] | undefined } },
+   url: {
+     pathname: Route,
+     query?: { [key: string]: string | string[] | undefined }
+  }, 
  as?: string,
  options?: TransitionOptions
): Promise<boolean>;

No -- push is overloaded, if you look at the first function definition push already accepts pathname: https://github.com/tatethurston/nextjs-routes/blob/main/src/core.ts#L220

Link also already accepts pathname.

Here's an example with both working on v1.0.4: https://stackblitz.com/edit/nextjs-88s7s8?file=pages/route-a.tsx

tatethurston commented 1 year ago

the import should be from "next/router"

Thanks for noticing this, I've updated the readme.

ChristoRibeiro commented 1 year ago

So how can I use push with both pathname and query?

For now I can only use pathname or query. Not both of them at same time.

My use case:

// go to this URL:
// /product-data?id=123&sort=desc&groupBy=name
router.push({
  pathname: "/product-data",
  query: { 
    id: "123",
    sort: "desc",
    groupBy: "name"
  }
})
tatethurston commented 1 year ago

@ChristoRibeiro You can use both pathname and query, see the example I referenced previously: https://stackblitz.com/edit/nextjs-88s7s8?file=pages%2Froute-a.tsx,pages%2Findex.tsx

      <button
        onClick={() =>
          router.push({
            pathname: '/',
            query: {
              id: '123',
              sort: 'desc',
              groupBy: 'name',
            },
          })
        }
      >

Could you share the error you're encountering?

ChristoRibeiro commented 1 year ago

If I use:

An example with push:

CleanShot 2022-11-28 at 13 43 58

I guess it's not the correct behaviour as mentioned in the documentation @tatethurston?

ChristoRibeiro commented 1 year ago

I updated to Nextjs 13 and it work now 🙌