tatethurston / nextjs-routes

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

Improve Link type for better inference #111

Closed medihack closed 1 year ago

medihack commented 1 year ago

I am using Next.js with nextjs-routes together with Mantine. Mantine has a component Menu.Item where all its props are inferred by a component prop. Unfortunately, this fails when setting component={Link} as it always infers that the href of Link is only of type Query (and doesn't consider Route and StaticRoute).

Currently, Link is defined this way:

  declare function Link(
    props: PropsWithChildren<LinkProps<Route>>
  ): LinkReactElement;
  declare function Link(
    props: PropsWithChildren<LinkProps<StaticRoute>>
  ): LinkReactElement;
  declare function Link(
    props: PropsWithChildren<LinkProps<Query>>
  ): LinkReactElement;

  export default Link;

But the issue would be fixed if it would be defined this way:

  declare function Link(
    props: PropsWithChildren<LinkProps<Route | StaticRoute | Query>>
  ): LinkReactElement;

  export default Link;

I can't think of any drawbacks, but I am not a TS expert.

tatethurston commented 1 year ago

@medihack This should already be fixed on version 1.0.4. Could you confirm the version you’re using?

medihack commented 1 year ago

I am using 1.0.4. Just installed it yesterday for the first time. I also tried to delete it, but it is re-generated in the same way.

From my pnpm-lock.yaml:

  /nextjs-routes/1.0.4_next@13.1.1:
    resolution: {integrity: sha512-BdlLv1sdiH2+qXWsWrrVfW8N1dR3iV0pjUvpO4JcQyt51fLUwVPX+G5uABA/tLzkWhK3MPUcXUoc3fILfi2cmg==}
    peerDependencies:
      next: '*'
    dependencies:
      chokidar: 3.5.3
      next: 13.1.1_7xlrwlvvs7cv2obrs6a5y6oxxq
    dev: false
evgenyboxer commented 1 year ago

Facing the same issue.

@tatethurston I've looked at the main and yeah it seemed that the problem is solved indeed. Are you able to push another version please? Thanks for the great library.

tatethurston commented 1 year ago

Thanks @medihack and @evgenyboxer, I published v1.0.5 which removes the function overloading definition for Link and should resolve this.