ethanniser / next-typesafe-url

Fully typesafe, JSON serializable, and zod validated URL search params, dynamic route params, and routing for NextJS.
https://next-typesafe-url.dev
MIT License
362 stars 16 forks source link

fixes import order ( functional fix ) #87

Closed balibintang closed 6 months ago

balibintang commented 6 months ago

The location of the imports causes the package not to work properly.

Before:

image

After:

image

Have set it to match whats seen in the example packages.

ethanniser commented 6 months ago

This directly reverts #85, which is fine if it actually did break things. But I'm just curious because someone else quite intentionally made this change.

balibintang commented 6 months ago

Im not really sure to be completely honest, but this is what i managed to do to fix it. With the current version the package was completely non functional. It only worked for routes, but broke for query and search params

ethanniser commented 6 months ago

I'm out of town but that's good enough for me

If it breaks again we'll see just lmk

chungweileong94 commented 6 months ago

So I'm currently facing a different issue that is related to both this and #85.

Issue describe

Possible better solution

We can dynamically import the type inline, like

interface DynamicRouter {
   '/items/[id]': InferRoute<import('./src/app/items/[id]/routeType')>;
}

I can submit a PR to fix the issue, but mind if @balibintang could share a small repro? As I'm not sure if my solution will work in your case.

ethanniser commented 5 months ago

sorry this never went through you didnt add a changeset and I forgot to check just published 4.0.5 lmk if things are fixed or not

lelabo-m commented 5 months ago

@balibintang could you share your version of next and which package manager you use, if you don't mind? I proposed the changes because I had the opposite behavior, so I would like to understand what happened.

lelabo-m commented 5 months ago

Ok, I had time to go back in. @chungweileong94 is right on this issue. Credit to him for the solution. I missed this part the first time reading about ambient d.ts files rules and convention and my solution was only half-baked. I came to the same conclusion than him playing again with the code, and saw his recommendation afterward.

I took the initiative to create a PR (#91) again, and you are all welcome to check my implementation of the changes (still new to this). Tested on my side the cli, and the generation seems fine, the current version of the package seems to play correctly with it, but I don't have many routes yet.

I hope this time, the issues are resolved.

balibintang commented 5 months ago

Sorry guys i was on holiday, but just checked out the changes and tested it and it seems to still be working on my end too! Thanks everyone for the contributions :)