-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[1.0] Upgrade React Router to v4 #940
Conversation
Deploy preview failed. Built with commit 48e2735 https://app.netlify.com/sites/using-drupal/deploys/5914ac5c8ebdd97352abb566 |
Deploy preview failed. Built with commit 48e2735 https://app.netlify.com/sites/gatsbyjs/deploys/5914ac5c8ebdd97352abb562 |
Deploy preview failed. Built with commit 48e2735 https://app.netlify.com/sites/gatsbygram/deploys/5914ac5c8ebdd97352abb564 |
Curious to understand how do you plan to handle recursive routes. Any detail to share? |
@MoOx every page component is at the heart of things and it can have 1 or 0 layout components. Generally sites have 1 to N page components and often just 1 layout component ( Up to now v1 of Gatsby hasn't supported multiple levels of layouts (which v0 does) but in this PR I'm actually adding back support for that. Basically a layout will be able to indicate that it has a parent layout component. So when Gatsby creates its routes it'll just recurse up the layouts to the root component and then render that hierarchy. |
Oh so Gatsby generate the routes for react-router... I was thinking user were able define their own routes :) So what the point of using react-router? |
Something's gotta handle routes and components. Also RR v4 makes it far easier to embed client-only routes within a larger static Gatsby site. meetfabric.io, the company sponsoring the upgrade, is doing exactly that as they'll be using Gatsby for both their marketing website as well as the client app portion of things. |
It's possible at some point we could make RR an optional dependency to slim things further but not today. |
|
||
// console.log(pages) | ||
// Write out routes.json | ||
const routesData = _.merge( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor proposition:
const routesData = pages.reduce((mem, {path, componentChunkName, layout, jsonName}) => (
{ ...mem, ...{[path]: {componentChunkName, layout, jsonName}} }
), {})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! I'll make that change. Object/array spreading makes reducing quite nice!
This PR has two breaking changes.
|
Ok, this looks ready to go in. One more breaking change — this.props.children in layout components is now a "function". I spent hours trying to upgrade If See this article for a good intro to children as a function https://medium.com/merrickchristensen/function-as-child-components-5f3920a9ace9 This is an easy change though. Just change |
This PR also adds support for client-only routes (and an example site showing this off). Basically how it works is that a page can have a So say you had a marketing website and client-only app in the same Gatsby site. Marketing pages would be at paths like The easiest way to add a You'll also need to make sure that whenever a user loads from the server urls like |
I had some tasks for this upgrade that I'm not going to get to right now. Multiple layouts + layout hierarchies, and layouts w/ graphql queries. This is very doable just ran out of time + the API could be tricky so it deserves a deeper dive + discussion. |
Create an issue if you're interested in working on it and I'll write up some thoughts. |
I snuck in some optimizations. |
This is the URL of the example site I built for client-only routes https://client-only-paths.gatsbyjs.org/ |
Fixes #937
Current status, rendering things correctly with the development server.