fridays / next-routes

Universal dynamic routes for Next.js
MIT License
2.47k stars 230 forks source link

non-dynamic routes aren't handled #5

Closed sedubois closed 7 years ago

sedubois commented 7 years ago

Here's my project which I just upgraded to use next-routes 🎉

https://github.com/relatenow/relate

Unfortunately I was a bit fast, it looks like non-dynamic routes (like /about) aren't handled properly, they give a not found page when reloading from server (but work client-side). Did I miss something in the configuration or would this be a bug?

fridays commented 7 years ago

Hey, I can't find next-routes in that project, can you post the configuration here?

If a path can't be matched by next-routes, it simply passes the request to the default handler, so it should work fine.

sedubois commented 7 years ago

Sorry forgot to git push 😄 it's there now.

fridays commented 7 years ago

Ah that's because you use a catchall route, it routes everything to the profile page.

If you want some paths not handled by it, add them before:

routes.add('about', '/about');
routes.add('profile', '/:slug');
sedubois commented 7 years ago

Hmm yes that's unavoidable I guess. Thanks 🙏

sedubois commented 7 years ago

Could the API be maybe simplified to accept routes.add('about')? And the second argument pathname would be derived from the first if not provided.

sedubois commented 7 years ago

And even better, accepting an array for the first argument:

routes.add([
  'about',
  'discover'
]);

or routes.addAll

fridays commented 7 years ago

Could the API be maybe simplified to accept routes.add('about')

Good idea! 👍

You can now do that when you npm update

Also you can do chaining:

routes
  .add('about')
  .add('discover')
  .add('blog', '/:slug')
sedubois commented 7 years ago

Thanks, works 👍