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

[1.0] Upgrade React Router to v4 #940

Merged
merged 18 commits into from
May 11, 2017
Merged

[1.0] Upgrade React Router to v4 #940

merged 18 commits into from
May 11, 2017

Conversation

KyleAMathews
Copy link
Contributor

Fixes #937

Current status, rendering things correctly with the development server.

screen shot 2017-05-08 at 5 34 55 pm

@gatsbybot
Copy link
Collaborator

gatsbybot commented May 9, 2017

Deploy preview failed.

Built with commit 48e2735

https://app.netlify.com/sites/using-drupal/deploys/5914ac5c8ebdd97352abb566

@KyleAMathews
Copy link
Contributor Author

KyleAMathews commented May 9, 2017

Deploy preview failed.

Built with commit 48e2735

https://app.netlify.com/sites/gatsbyjs/deploys/5914ac5c8ebdd97352abb562

@KyleAMathews
Copy link
Contributor Author

KyleAMathews commented May 9, 2017

Deploy preview failed.

Built with commit 48e2735

https://app.netlify.com/sites/gatsbygram/deploys/5914ac5c8ebdd97352abb564

@MoOx
Copy link

MoOx commented May 9, 2017

Curious to understand how do you plan to handle recursive routes. Any detail to share?

@KyleAMathews
Copy link
Contributor Author

KyleAMathews commented May 9, 2017

@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 (layouts/index.js is the default layout component if a page doesn't specify one specifically).

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.

@MoOx
Copy link

MoOx commented May 9, 2017

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?

@KyleAMathews
Copy link
Contributor Author

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.

@KyleAMathews
Copy link
Contributor Author

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(
Copy link
Contributor

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}} }
    ), {})

https://codepen.io/fabien0102/pen/MmrqKe?editors=0011#

Copy link
Contributor Author

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!

@KyleAMathews
Copy link
Contributor Author

This PR has two breaking changes.

  • Gatsby now looks for a HTML element with an ID of ___gatsby to render itself instead of the current react-mount. You'll need to change that in your html.js. Gatsby may or may not stay React only in the future after all ;-)
  • The "default" layout component should now be at layouts/index.js not layouts/default.js. A quick move over will fix things.

@KyleAMathews
Copy link
Contributor Author

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 gatsbygram yesterday before realizing this was the only option basically. The problem is that when you load Gatsbygram and click on an image, it loads a modal to display the image but leaves the background the same. So basically the gatsbygram layout needs to say, "keep rendering / despite us having navigated now to /path-to-image/.

If this.props.children is a function, then this is easy. See the code diff for all the gory details https://github.com/gatsbyjs/gatsby/pull/940/files#diff-abbddb44eecd9dea4fc2d307be2d7b9dR157

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.props.children in your layout file to this.props.children().

@KyleAMathews
Copy link
Contributor Author

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 matchPath in addition to its path. The path is what's used for server rendering but in the client, matchPath will be used in addition to path for deciding which page component is in control.

So say you had a marketing website and client-only app in the same Gatsby site.

Marketing pages would be at paths like /, /blog/, /pricing/, /features/, etc. The app might be at /app/. But the app also has a bunch of sub pages like /app/settings/ and so on. But these shouldn't be rendered on the server, just on the client. But without modifying anything, when you try to navigate to /app/settings/, things will break because Gatsby will try to find that page and fail. So what you do is add a matchPath to the app page like /app/:path. Now for any subpath of /app/ Gatsby will still pass control of the site to the app page.

The easiest way to add a matchPath currently is in gatsby-node.js using the API onUpsertPage which is called whenever a page is added.

You'll also need to make sure that whenever a user loads from the server urls like /app/settings/ that you load /app/index.html.

@KyleAMathews
Copy link
Contributor Author

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.

@KyleAMathews
Copy link
Contributor Author

Create an issue if you're interested in working on it and I'll write up some thoughts.

@KyleAMathews
Copy link
Contributor Author

I snuck in some optimizations. gatsby-plugin-google-analytics dropped react-ga as a dependency as it had a ton of code we don't need. gatsby-plugin-catch-links dropped a dependency and inlined a simple version that does exactly what we need.

@KyleAMathews KyleAMathews merged commit 5c89bc3 into 1.0 May 11, 2017
@KyleAMathews KyleAMathews deleted the rr4 branch May 11, 2017 18:46
@KyleAMathews KyleAMathews changed the title [1.0] WIP Upgrade React Router to v4 [1.0] Upgrade React Router to v4 May 11, 2017
@KyleAMathews
Copy link
Contributor Author

This is the URL of the example site I built for client-only routes https://client-only-paths.gatsbyjs.org/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants