catch-all routing with an _all.js by evantahler · Pull Request #223 · vercel/next.js · GitHub
Skip to content

catch-all routing with an _all.js - #223

Closed
evantahler wants to merge 3 commits into
vercel:masterfrom
evantahler:catch-all-routes
Closed

evantahler wants to merge 3 commits into
vercel:masterfrom
evantahler:catch-all-routes

Conversation

@evantahler

@evantahler evantahler commented Nov 7, 2016 •

Copy link
Copy Markdown
Contributor

Part of the solve for #25
re: #25 (comment)

From the README update:

If you are building an application which requires a catch-all route (for example, /user/4), you can define a _all.js file in the ./pages/user folder. Any un-matched route under the "user" route will be sent to this page.

@evantahler evantahler changed the title catch-all routing with _all.js catch-all routing with an _all.js Nov 7, 2016
@evantahler

evantahler commented Nov 7, 2016 •

Copy link
Copy Markdown
Contributor Author

@rauchg

rauchg commented Nov 7, 2016

Copy link
Copy Markdown
Member

This is an interesting approach. I need to think about it more.

@evantahler

evantahler commented Nov 8, 2016 •

Copy link
Copy Markdown
Contributor Author

As a note, It could also make sense to default the "catch-all" file to index.js, should that exist within a directory. The name _all.js is totally arbitrary.

@rauchg

rauchg commented Nov 8, 2016

Copy link
Copy Markdown
Member

Also, considering we already support pathname in getInitialProps (https://github.com/zeit/next.js/releases/tag/1.1.0) that sounds pretty smart

@rauchg

rauchg commented Nov 8, 2016

Copy link
Copy Markdown
Member

As in, to extract the "fancy parameter", you'd just have to pathname.split('/').pop()

@rauchg

rauchg commented Nov 8, 2016

Copy link
Copy Markdown
Member

My only concern is how we handle this client-side with <Link />. Specially for nested stuff like:

/users/:id/posts

@evantahler

evantahler commented Nov 8, 2016 •

Copy link
Copy Markdown
Contributor Author

@rauchg You just saved 2 lines of code with pathname.split('/').pop(), cool!
(updated)

@evantahler

evantahler commented Nov 8, 2016 •

Copy link
Copy Markdown
Contributor Author

regarding /users/:id/posts... If you are using the file-system only, I guess that isn't possible (save a lot of symlinks). We should clarify that the wildcard can only be the final part of the route.

That's why I prefer to call this a "catch all" instead of any fancier type of regexp-style routing, because it would be misleading

@rauchg

rauchg commented Nov 8, 2016

Copy link
Copy Markdown
Member

Indeed. So I have to think about whether this behavior is worth it. Otherwise, every single folder will stop handling 404s. In that case, explicit _all.js is better

@rauchg

rauchg commented Nov 8, 2016

Copy link
Copy Markdown
Member

But we also need to think about how to handle 404 elegantly from the catch-all. Like, let's say the user id is not found. How do you handle that?

@rauchg

rauchg commented Nov 8, 2016

Copy link
Copy Markdown
Member

But ultimately I guess that's not related to this PR. Since we would still want that for /user?id=xxxx. cc @nkzawa

@evantahler

evantahler commented Nov 8, 2016 •

Copy link
Copy Markdown
Contributor Author

Yeah, I agree that fancier error handling is a good idea, but should be a more global update (and not located to just this PR)

You've already got support for creating a 500, and it is pretty easy :D

render () {
    if(this.state.user_id > 5){
      throw 500
    }

    return <div>
      <p>user_id: {this.state.user_id}</p>
      <Link href={'/'}>Back</Link>
    </div>
  }

Perhaps we can throw numbers as HTML error-codes?

@jaredpalmer

Copy link
Copy Markdown
Contributor

regarding /users/:id/posts... If you are using the file-system only, I guess that isn't possible (save > a lot of symlinks). We should clarify that the wildcard can only be the final part of the route.

@rauchg @evantahler

instead of _all.js, could you designate wildcards with folder names instead? 🤔

// equivalent to /users/:id/posts/:title
.
└── pages
    ├── index.js
    └── users
        ├── _id
        │   ├── edit.js
        │   ├── index.js
        │   └── posts
        │       ├── _title
        │       │   ├── edit.js
        │       │   └── index.js
        │       ├── index.js
        │       └── new.js
        ├── index.js
        └── new.js

Here's how you could mimic a wordpress blog's permalink structure (with paginated archives on year and month):

// equivalent to 
// example.com
// example.com/:year
// example.com/:year/page/:number
// example.com/:year/:month
// example.com/:year/:month/page/:number  
// example.com/:year/:month/:title
.
└── pages
    ├── _year
    │   ├── _month
    │   │   ├── _title
    │   │   │   └── index.js
    │   │   ├── index.js
    │   │   └── page
    │   │       └── _number
    │   │           └── index.js
    │   ├── index.js
    │   └── page
    │       └── _number
    │           └── index.js
    └── index.js

This would cover all use cases i can think of, the only thing this doesn't allow for (out-of-the-box at least) is a medium-like permalink structure: medium.com/@:user_id. However, depending on how you walk the folders / parse this could also be done, but would probably require a different special character as underscore is pretty popular in URLs. Double underscore or dashes would nicely parallel BEM CSS modifiers. Could also pay some homage to php with $.

@rangerscience

rangerscience commented Nov 21, 2016 •

Copy link
Copy Markdown

@jaredpalmer Doesn't the solution run into issues if there's more than one folder? If I have both pages/foo/month and pages/bar/month, which does a request to 6/month get routed to? @evantahler , same goes for the :id.js syntax. Seems to me that the catch-all has to be a specific name (like _all.js) so that it's impossible to accidentally define more than one.

Seems like you'd want something like pages/posts/_all.js catching posts/6 and pages/posts/_all/monthcatchingposts/6/month`.

@rauchg

rauchg commented Nov 21, 2016

Copy link
Copy Markdown
Member

I remain unconvinced of these smart handlers. We are just going to make the
server API really easy to use and provide great examples of custom routing
:)

On Mon, Nov 21, 2016 at 6:29 AM narfanator notifications@github.com wrote:

@jaredpalmer https://github.com/jaredpalmer Doesn't the solution run
into issues if there's more than one folder? If I have both
pages/foo/month and pages/bar/month, which does a request to 6/month get
routed to? @evantahler https://github.com/evantahler , same goes for
the :id.js syntax. Seems to me that the catch-all has to be a specific
name (like _all.js) so that it's impossible to make multiple catch-alls

Seems like you'd want something like pages/posts/_all.js catching posts/6
and pages/posts/_all/monthcatchingposts/6/month`.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#223 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAy8XC7kXG-Av_z8oG3J7VE2N0BxQvUks5rAWR8gaJpZM4Krw5O
.

@evantahler

Copy link
Copy Markdown
Contributor Author

@rauchg so what is the path forward here? I can add an _all to the middle of routes as well, or should I just leave this as-is for catch-all files?

@rauchg

rauchg commented Nov 23, 2016

Copy link
Copy Markdown
Member

I'm documenting the path forward with this in an issue which I should have ready within a day or two

@rauchg

rauchg commented Nov 23, 2016

Copy link
Copy Markdown
Member

@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.

4 participants