Skip to content
This repository has been archived by the owner on May 10, 2021. It is now read-only.

[MERGED] support optional catch-all routes #15

Closed
wants to merge 1 commit into from

Conversation

rajington
Copy link
Contributor

Next.js added a new, experimental, optional catch-all routes, this adds support for it by looking for one more wrapping square bracket

Next.js added a new, experimental, [optional catch-all routes](https://nextjs.org/docs/api-routes/dynamic-api-routes#optional-catch-all-api-routes), this adds support for it by looking for one more wrapping square bracket
@FinnWoelm
Copy link
Collaborator

Hey @rajington,

This is excellent! Thank you so much for the PR and the contribution. 😊

This looks really good — let me take a closer look tonight and add a few tests.

Hoping to merge and publish later today! 🙂
- Finn

FinnWoelm added a commit that referenced this pull request Jun 13, 2020
The default behavior of NextJS catch-all routes is to match only if at
least one URL parameter is present.

So far, next-on-netlify matched catch-all routes even when no URL
parameter was present. For example, /pages/shows/[...slug] would
match /shows/2, shows/3/my/path, but also /shows. This does not match
the NextJS behavior described here:
https://nextjs.org/docs/routing/dynamic-routes#catch-all-routes

This commit matches catch-all routes only when at least one URL
parameter is present. It is in preparation for #15, which will introduce
support for optional catch-all routes. Optional catch-all routes also
match the page's base path without URL parameters (e.g. /shows).
FinnWoelm added a commit that referenced this pull request Jun 13, 2020
Next.js added a new, experimental, [optional catch-all routes](
https://nextjs.org/docs/api-routes/dynamic-api-routes#optional-catch-all-api-routes
), this adds support for it.

See: #15
@FinnWoelm
Copy link
Collaborator

Hi @rajington,

Thanks again for this PR! I just added some tests and merged it (I rebased, so it's not showing up as merged 😐). It will go live with the next release of next-on-netlify today or tomorrow :)

Two caveats:

  1. The experimental optional catch-all routes feature does not yet seem to work with pages using getStaticProps and getStaticPaths. The paths end up a bit garbled. Looks like this will be fixed in NextJS 9.4.5: make getStaticPaths work with optional catch-all routes vercel/next.js#13559

  2. Ideally, I should be creating two redirects for the data routes of optional catch-all routes: one for the base path with no params and one with a * for one or more params. For the sake of simplicity, I'm just creating a single redirect for now.

    For example, a page like /shows/[[...slug]].js should have two redirects for the data routes:

    1. /_next/data/shows.json
    2. /_next/data/shows/*

    For now, I'm just using a single redirect /_next/data/shows*. This is not ideal, because this will also match /_next/data/showsButDifferent.json — which it should not match. I've made a note and hope to fix that soon.

Thank you again, @rajington! Let me know if you end up building something awesome with next-on-netlify. Would love to feature it in the README 😁

- Finn

@FinnWoelm FinnWoelm closed this Jun 13, 2020
@FinnWoelm FinnWoelm changed the title support optional catch-all routes [MERGED] support optional catch-all routes Jun 13, 2020
@rajington
Copy link
Contributor Author

rajington commented Jun 13, 2020

will do, thanks! you're doing a great job with this project! this might be the missing piece for SSR on Netlify, wouldn't be surprised if they featured it

FinnWoelm added a commit that referenced this pull request Jun 14, 2020
- Add support for [NextJS optional catch-all routes](
  https://nextjs.org/docs/api-routes/dynamic-api-routes#optional-catch-all-api-routes
  ) ([#15](#15))
- Fix: An `index.js` page with `getStaticProps` no longer causes
  `next-on-netlify` to fail
  ([#18](#18))
- Fix: Catch-all routes now correctly require that at least one URL
  parameter is present (unlike optional catch-all routes)
  ([479b7e7](479b7e7))
- Fix: Data routes now correctly work for pages with catch-all routing
  ([0412b45](0412b45))
@FinnWoelm
Copy link
Collaborator

Thanks for the kind words, @rajington! It's a privilege to contribute back to the community and to meet nice people like yourself :)

I just published next-on-netlify v2.1.0 which includes your work on optional catch-all routes! 🙌

@rajington
Copy link
Contributor Author

they did already feature it! https://www.netlify.com/blog/2020/06/10/2-ways-to-create-server-rendered-routes-using-next.js-and-netlify/

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.

2 participants