search

ckastbjerg / next-type-safe-routes

Never should your users experience broken links again!
MIT License
70 stars 5 forks source link

API suggestion: catch-all routes use the chosen parameter name #26

Open osdiab opened 3 years ago

osdiab commented 3 years ago

This is irrelevant if we go with #21 , but right now the getRoute function refers to the catch-all part of a route to be path, ignoring the name in the actual route. For instance, /foo/[...bar].ts would be populated with getRoute({route: '/foo/[...bar].ts', path: 'baz'}) instead of getRoute({route: '/foo/[...bar].ts', params: {bar: 'baz'}}).

I found it counterintuitive that while NextJS refers to those catch-all params as params, this library doesn't.

ckastbjerg commented 2 years ago

It's been awhile since I worked on the library, so forgive me if I'm not remembering correctly here...

But I believe I chose the path as you can have both params and path. E.g.:

getRoute({
  route: "/nested-catch-all/[dynamic]/slugs",
  params: { dynamic: 1 },
  path: "/a/b/c",
})

More details here: https://github.com/ckastbjerg/next-type-safe-routes/pull/13

Or are you arguing that they should be treated the same?

But yes, if the API surfaces was limited/the implementation changed, it might not matter I guess

osdiab commented 2 years ago

Yes, just saying it would be more consistent if they were treated the same.