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

Proposition: parameters in URL #91

Closed
CompuIves opened this issue Oct 26, 2016 · 36 comments
Closed

Proposition: parameters in URL #91

CompuIves opened this issue Oct 26, 2016 · 36 comments

Comments

@CompuIves
Copy link
Contributor

I think it would be great to have parameter in URL functionality (for example /users/:id) while still keeping the simplicity of next itself.

My proposition is to do this for structure:

pages/
  index.js
  users/
    _id.js

id.js will be seen as a parameter path and will receive id as prop. You could also go further and make it like this to map /users/:id/friends/:friendid:

pages/
  index.js
  users/
    _id/
      index.js
      friends/
        _friendid.js

If the id is not specified (eg. /users/) then we'll first check if there is an index.js and otherwise use _id.js.

I created this issue to see if others are interested in this and to see what everyones thoughts/feedback is on this. I'm willing to build this if this is a wanted feature! 😄

@mmmeff
Copy link

mmmeff commented Oct 26, 2016

This idea is amazing.

@mmmeff
Copy link

mmmeff commented Oct 26, 2016

How would you pass the parameter into the component? getInitialProps?

@CompuIves
Copy link
Contributor Author

Thanks! I think the best way is to add it as an argument in getInitialProps here: https://github.com/zeit/next.js/blob/master/server/render.js#L22.

So then you could have this for example with getInitialProps:

getInitialProps(ctx, { id }) {
  // Do request using id
}

@nickdandakis
Copy link

nickdandakis commented Oct 26, 2016

How would this translate a path with nested params like so?
/users/:id/:photoid

Would it make more sense to use a structure like this?

pages/
  users/
    index.js
  users:id/
    index.js
    friends/
      index.js
    friends:friendsid/
      index.js
  users:id:photoid/
    index.js

The above would support routes for

/users/
/users/:id
/users/:id/friends
/users/:id/friends/:friendsid
/users/:id/:photoid

@CompuIves
Copy link
Contributor Author

Ah, I now see that I created a duplicate PR 😅 .

How would this translate a path with nested params like so?
/users/:id/:photoid

It would have a folder structure like this:

pages/
  users/
    _id/
      _photoid.js

But I do find that your structure feels more natural since it doesn't add a magical _.

@mmmeff
Copy link

mmmeff commented Oct 26, 2016

It's important to note that certain OS's will barf on the colons in directory names. In OSX, you can create a directory like :id via terminal but if you try and view it in finder it shows as /id and you won't be able to rename it with finder without breaking the filename

@beepboopitschloe
Copy link

What about using a dot instead of a colon?

pages/
  users/
    index.js
  users.id/
    index.js
    friends/
      index.js
    friends.friendsid/
      index.js
  users.id.photoid/
    index.js

@nodegin
Copy link
Contributor

nodegin commented Oct 27, 2016

@nickdandakis @nmuth -1 for this solution since it's all index.js

@beepboopitschloe
Copy link

@nodegin It could just as easily follow the existing convention of users.js for pages with no children and users/index.js otherwise.

pages/
  users.id/
    index.js
    friends.js
    friends.friendid.js

I think this fits in pretty well with how next handles routing, but I don't like how non-obvious the dot syntax is.

@dlindenkreuz
Copy link
Contributor

dlindenkreuz commented Oct 27, 2016

The concept of a filesystem based router shows its limits when it comes to advanced routing with RegExps, conditional routes and alike.

@CompuIves, I think your suggestion is really elegant within those boundaries, but I also think the project ultimately needs more than the current filesystem based routing to cater to the requirements of larger web apps.

This is of course something for the Next.js team to decide; I'd appreciate some comments and am thrilled to see this discussion about routing evolve.

@dstreet
Copy link
Contributor

dstreet commented Oct 27, 2016

I approached his problem differently, through configuration. Defining routes this way allows for more complex routing. It also allows the file system to maintain semantic meaning.

For instance, the route '/users/:id/friends/:id' could point to the file 'pages/users/friend', which avoids the use of overly complex hierarchies and naming conventions.

You can see my implementation in #59.

I'm still working out a few kinks, but I think it's a good solution to the problem.

@beepboopitschloe
Copy link

@dstreet Question about your implementation. If I have a route configuration like this:

routes: {
  '/posts/:id': 'arbitrary/directories/post'
}

Then navigating to /posts/1234 should render the component in arbitrary/directories/post.js, correct? What should happen if I navigate to arbitrary/directories/post directly? 404?

@nodegin
Copy link
Contributor

nodegin commented Oct 28, 2016

I think @Compulves' solution is the best one yet:

pages/
  photos/
    index.js
    id.js
    _id.js
    share/
      _action.js

which mapping to:

pages/photos
pages/photos/id
pages/photos/:id
pages/photos/share/:action

@dstreet
Copy link
Contributor

dstreet commented Oct 28, 2016

@nmuth That is one of the kinks that needs to be ironed out. Currently it will fall through to the default route, which maps directly to the filesystem, so arbitrary/directories/post would be rendered

@dstreet
Copy link
Contributor

dstreet commented Oct 28, 2016

@nodegin how would you implement pages/photos/:id/share or some other route that is a descendent of a single photo?

Something like this?

pages/
    photos/
        index.js
        id.js
        _id/
            index.js
            share/
                index.js
                _action.js

Also how would you handle more complex routes. For instance, routes with or regex?

@nodegin
Copy link
Contributor

nodegin commented Oct 28, 2016

@dstreet If we just use filesystem for the automatic routing then it must be limits, in this case we can use the solution which embedding the routes in the next section of package.json.

For pages/photos/:id/share I think it is ok to change it to pages/photos/share/:id thus:

pages/
    photos/
        index.js
        _id.js
        share/
          _id.js

And in this case:
/users/:id/friends/:friendid

We can just change to:
/users/:id
/users/friends/:id

@beepboopitschloe
Copy link

@dstreet It might be worth having two approaches: one based on the filesystem, like @Compulves suggested, and one based on a configuration file that can handle arbitrary routes and regexes.

@nodegin I get what you're saying, but I would rather not sacrifice expressiveness just to avoid using index.js files, especially since that falls in line with how Next already works.

@dstreet
Copy link
Contributor

dstreet commented Oct 28, 2016

@nodegin Assuming /users/friends/:id pull the friends of user with ID :id, I can see how that would work, but what if you only wanted to display a single friend, with an ID of :friendid, of the user? However, if in that example, :id is the ID of the friend, then that would work, but only if :id is a guid, otherwise you could run into conflicts.

On another note, and my solution exhibits the same problem, given the following filesystem, what component would render with the route /users/friends?

pages/
    users/
        _id.js
        friends/
            index.js

That path would match two possible components: pages/users/_id.js and pages/users/friends/index.js

@dstreet
Copy link
Contributor

dstreet commented Oct 28, 2016

@nmuth I too am leaning towards to dual approach. My only concern is that it opens up the framework to too much confusion, and how would conflicts between the two solutions be resolved?

@dstreet
Copy link
Contributor

dstreet commented Oct 28, 2016

@nmuth To solve the issue of the routes falling through to the filesystem paths, the implementation could be adjusted such that

routes: [
    { path: '/posts/:id',  module: 'arbitrary/directories/post', accessDirect: false }
]

would disallow the default routing from matching on a route where accessDirect is false.

@nodegin
Copy link
Contributor

nodegin commented Oct 28, 2016

@dstreet

if you only wanted to display a single friend, with an ID of :friendid

/users/:id would do that since friend should be an user too

@nodegin
Copy link
Contributor

nodegin commented Oct 28, 2016

How about we don't use any folder:

/users
/users/:id
/users/:id/friends
/users/:id/share
/users/:id/share/:action
pages/
  _users.js
  _users_$id.js
  _users_$id_friends.js
  _users_$id_share.js
  _users_$id_share_$action.js

where _ represents /, $ represents :

@dstreet
Copy link
Contributor

dstreet commented Oct 28, 2016

Hah. I like the way you think, @nodegin

@nickdandakis
Copy link

@nodegin, pretty sure $ are frowned upon within filenames. It may be interpreted as an environment variable in Unix.

@nodegin
Copy link
Contributor

nodegin commented Oct 29, 2016

@nickdandakis How about use @ instead?

users.js
users_@id.js
users_@id_@action.js

@dstreet
Copy link
Contributor

dstreet commented Oct 29, 2016

This syntax needed to solve this within the filesystem alone is awkward and extremely limiting.

@sedubois
Copy link
Contributor

sedubois commented Oct 29, 2016

Naive question (from someone unfamiliar with routing and the decision-making behind next), in order to avoid doing all the hard work again, is there any possibility to use React-Router v4 directly, a subset of it, or some of its key decisions? It sounds like they learned a lot from their mistakes and it would be great not to reinvent the wheel. AFAIK, in v4 they made sure to only handle routing and everything else is externalized, so I guess it should be lightweight, and it has a huge community base.

Everything is provided as a component (so can use composability and the React component lifecycle): <Route>, <ServerRouter>, <Match>, <Miss>, etc. See new docs deployed with now 😉

@nodegin
Copy link
Contributor

nodegin commented Oct 29, 2016

Hi guys, please take a look at PR #147

@CompuIves
Copy link
Contributor Author

We should try to keep keep the need of diving into configuration files to a minimum. Therefore I think we should have parameterized routing with just the file system if we can find a simple, not confusing implementation while also keeping the possibility to use your own system (eg with this exploration #25).

@ksmithut
Copy link

ksmithut commented Nov 2, 2016

I'm a little late to the party, but you could use parameters in routes like swaggerize-express does it.

tl;dr

users.js
users/
  |- {id}.js
  |- friends.js
  |- friends/
     |- {friendId}.js

@dstreet
Copy link
Contributor

dstreet commented Nov 2, 2016

@ksmithut #147 has already been updated to use that syntax

@ksmithut
Copy link

ksmithut commented Nov 2, 2016

lol, definitely late to the party :) Thanks @dstreet

@kachkaev
Copy link
Contributor

@nodegin could you please share a couple of thoughts on when this feature may be delivered? Since #147 is now closed without merging, I guess you're working on some other approach.

Awesome framework - I'm very keen to try it out!

@arunoda
Copy link
Contributor

arunoda commented Nov 25, 2016

@kachkaev I think that's related to this: #291

@rauchg
Copy link
Member

rauchg commented Dec 8, 2016

I commented elsewhere that APIs like this will be possible in userland, by using the programmatic API. Quoting:

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 :)

@doodiosie
Copy link

doodiosie commented Apr 10, 2017

I've written some code to have next routing behave similar to how .htaccess works in php. My routes file looks like this :

module.exports = [
    {
        test: /edit\/([0-9]+)/g,
        redirect: "edit?id=$1"
    }
];

and the interpretation code is done as so :

var server = http.createServer(function (req, res) {
    var newUrl = routes.reduce(function (url, route) {
        return url.replace(route.test, route.redirect);
    }, req.url);
    newReq = Object.assign(req, {url: newUrl});
    handle(newReq, res);
});

I might spend some time trying to get this into a package but having moved from php to node and now to next, this is a pretty good solution for me

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

No branches or pull requests