tatethurston / nextjs-routes

Type safe routing for Next.js
MIT License
571 stars 23 forks source link

Make hardcoded part a template? #45

Closed OnkelTem closed 2 years ago

OnkelTem commented 2 years ago

Hi!

I was glad to find this module, and at first, it worked pretty well. But when I started to build our code, which is a part of a bigger thing, I ran into a problem with that pathname is too strict and doesn't allow other strings, which may occur outside of the project src files.

E.g. this:

    ...(router.pathname === "/dashboard" && ...

makes an error:

Type error: This condition will always return 'false' since the types '"/404" | "/[id]" | "/" | ... and '"/dashboard"' have no overlap.

I can only edit the generated file now like this, to allow for string checks:

export interface NextRouter<P extends Pathname = Pathname>
    extends Omit<Router, "push" | "replace"> {
    pathname: P | string; /// <<<<<< ` | string` is added HERE
    route: P;
    query: QueryForPathname[P];
    ...
  }

but it's gonna be rewritten.

I'd propose to use an overridable template instead. What do you think?

tatethurston commented 2 years ago

Can you help me understand where dashboard is coming from? Why is dashboard outside of the project source files instead of being a nextjs page?

tatethurston commented 2 years ago

In the general case, all of a user's routes will be defined in their next application (and anchor tags can be used for any external links, because next's routing isn't offering any benefits for external routes). Losing type safety isn't an acceptable outcome.

Using generics wouldn't give anything additional over a cast (you're losing the same type safety with both approaches), so my recommendation would be to cast to string here:

    ...(router.pathname as string) === "/dashboard" && ...
OnkelTem commented 2 years ago

Well, the reason why it happens is that our app consists of different npm packages, that are developed by different teams. Then those packages are integrated into one application.

Thanks for the proposition, Tate! Let me try this approach.

tatethurston commented 2 years ago

Are these npm packages exporting Nextjs pages? I’m curious how the Nextjs router fits in here in the broader context.

OnkelTem commented 2 years ago

I'm not familiar with this system enough yet. But I'll try to update this issue once I learn it more.