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

Add support for parameterized routing. Refs #59, #91 #142

Closed
wants to merge 5 commits into from

Conversation

dstreet
Copy link
Contributor

@dstreet dstreet commented Oct 28, 2016

Pulls routes from next.config.js:

// next.config.js

exports.routes = [
  { path: '/posts/:id', file: 'posts/post' }
]
pages\
    index.js
routes\
    posts\
        post.js

Routes defined this way are resolved to the routes directory rather than pages to avoid conflicts and ensure existing functionality remains intact. Additionally, a parameters map is passed to each of the components via ctx.

@dstreet
Copy link
Contributor Author

dstreet commented Oct 28, 2016

#91, #59

@nodegin
Copy link
Contributor

nodegin commented Oct 29, 2016

This solution is duplicating pages folder! Which isn't minimalistic at all..

@dstreet
Copy link
Contributor Author

dstreet commented Oct 29, 2016

I don't agree. It is meant to be a complement to the pages folder. pages is for static routes, whereas routes is for dynamic routing. If an app has no need for dynamic routing, then this directory can be ignored, maintaining an extremely minimal implementation.

@nkzawa
Copy link
Contributor

nkzawa commented Oct 29, 2016

@dstreet @nodegin I wonder how you think about #25 which can support this kind of routings too, as external plugins. I think it's more flexible and simple.

@nodegin
Copy link
Contributor

nodegin commented Oct 29, 2016

@nkzawa Yes I agree providing an external for detailed customizations. But we can also give some basic support in the framework IMHO.

@dstreet
Copy link
Contributor Author

dstreet commented Oct 29, 2016

@nkzawa I would be ok with that approach as long as plugins could hook into the existing rendering and transpiling.

@hoangzinh
Copy link

hoangzinh commented Nov 20, 2016

I'm looking forward to hoping this PR will be merged to our repo. I need to custom route to do something like:
/photos/:id => pages/photos/photo

@dstreet
Copy link
Contributor Author

dstreet commented Nov 20, 2016

@hoangzinh I think the maintainers are going a different direction.

@hoangzinh
Copy link

@dstreet ah I see

@rauchg
Copy link
Member

rauchg commented Nov 23, 2016

@dstreet

Considering #291 (comment), I suggest you start looking into transforming this into an API like:

const nextRoutes from 'next-routes'
const app = next()
createServer(nextRoutes(app)).listen(process.env.PORT || 3000)

Users of this approach will have to use node server.js instead of next start.

Then you can extend next.config.js with your routes, and you can read the config from app.config (cc @nkzawa).

This will give you the great expressiveness and ease-of-use, without us having to bloat the core :)

@rauchg rauchg closed this Nov 23, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants