Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding an example for universal custom routing #913

Closed

Conversation

gcpantazis
Copy link
Contributor

Howdy! I was looking through the examples for a custom server.js, and they all worked well enough on the server-side. I did notice that they stop working on the client-side unless you use the href and as attributes in conjunction on your Link components, effectively duplicating your routing logic wherever you have a Link.

I can't find it at the moment, but in another issue @rauchg (or perhaps another of the maintainers) +1'd the concept of making a custom component to handle the route matching, referencing some shared configuration, which works like a charm for us. I didn't see a complete example of universal route matching in ./examples, so I thought I'd add one.

Hopefully this isn't duplicative from another example! Thanks so much to the team — this project has been an amazing leg up for us! Hoping to get some of our production code moved over to Next soon!

Copy link
Contributor

@arunoda arunoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty superb. This is why we ship a meta api for routing.

Then we see projects like this.


function matchInternal (route) {
const match = customRoutes.find((element) => {
return route.match(element.test)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about if we written this like this:

element.test(route)

@@ -0,0 +1,9 @@
module.exports = {
customRoutes: [{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like using next.config.js as the place for this. But in this file, we might import a lot of server only modules.
So, when we import this on both environments, that'll be an issue.

So, let's use a different file called routes.json or routes.js

test: /^\/$/,
routeTo: '/foo'
}, {
test: /^\/about(\/?)$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use path-to-regexp for express like route URLs.

import {customRoutes} from '../next.config'

function matchInternal (route) {
const match = customRoutes.find((element) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this match boolean be memorized for performance?

@sedubois
Copy link
Contributor

sedubois commented Jan 29, 2017

This is quite inspiring 🙂 It would also be great to see

  • how to handle route params (e.g /foo/:id)?
  • how would an HTTP server like Koa or Express handle such a routes.js config file without DRY?

@frol
Copy link
Contributor

frol commented Jan 29, 2017

@gcpantazis Great example! Thanks for sharing it!

JFYI: I have recently bumped into another approach with routing: https://github.com/matthewmueller/next-route

@arunoda
Copy link
Contributor

arunoda commented Jan 30, 2017

@frol That's a good one too.
And I'm waiting for, someone is building a React Router like API on top of our routing API.
That's totally doable.

@timneutkens
Copy link
Member

@gcpantazis could you update this one also?

@gcpantazis
Copy link
Contributor Author

@timneutkens aye, will do on Monday. I implemented much of what @arunoda suggested in our downstream project, so I'll port that up here as well. 👍

@timneutkens
Copy link
Member

Awesome, thank you very much ❤️

@timneutkens
Copy link
Member

I'm going to close this for inactivity. Cleaning up old PRs. i'd love to take this in one day though ❤️

@lock
Copy link

lock bot commented May 10, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue/PR related to examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants