Add hook for router on update. Fixes #302.

[AndersDJohnson]

No description provided.

[KyleAMathews]

So... how is this different from the existing onRouteChange? https://github.com/adjohnson916/gatsby/blob/a5303533f3f11ea45b7a7e2d38f311a6d0dc0895/lib/utils/web-entry.js#L18-L22

Not sure I see a difference? If you can't do what you want with onRouteChange then that's something that should be fixed but the two hooks seem so similar that that we should only have one.

[AndersDJohnson]

@KyleAMathews I just tested the anchor links solution I referenced in #302 with the existing onRouteChange hook, and you're right it seems to work fine either way.

I guess the difference is that onRouteChange listens to browserHistory directly whereas this new onRouteUpdate uses React Router's listener wrapping browserHistory's listener with some additional logic, which seems to include (1) waiting until React Router has set state & re-rendered before calling - this may be important to prevent an attempt to scroll to yet-unrendered elements, and (2) some state matching logic that I don't yet fully understand. I'm not sure which is preferable at this time, but I'd lean toward my implementation with onUpdate on the chance that it might respect the React Router lifecycle better. Thoughts?

1. https://github.com/reactjs/react-router/blob/b9a334e0285c9a080a93e3818c593fb28be33b50/modules/Router.js#L73
2. https://github.com/reactjs/react-router/blob/b9a334e0285c9a080a93e3818c593fb28be33b50/modules/createTransitionManager.js#L260

[KyleAMathews]

@adjohnson916 thanks for the research! Yeah they do sound almost identical. Let's just use React Router's actual API vs. our History implementation.

How about we mark the old onRouteChange as deprecated and use your new onRouteUpdate instead. Could you make that change in to the README plus add a console.log to the current onRouteChange saying something like "onRouteChange is now deprecated and will be removed in the next major Gatsby release (0.12). Please use onRouteUpdate instead. See the PR for more info INSERT_LINK_TO_THIS_PR_HERE"

[AndersDJohnson]

@KyleAMathews Thanks, I've made the changes in another commit to this PR's branch. Note that the new method onRouteUpdate does not receive a location parameter, as it's not passed in by React Router. Also I used console.warn instead of console.log, and enhanced that deprecation message a little bit.

[KyleAMathews]

How about we keep passing the location object to the new hook? Any reason not too? That'd smooth the transition plus it's useful.

Also there's a small eslint error that needs fixed.

[AndersDJohnson]

@KyleAMathews We could pass in the location object, it's just that React Router's onUpdate callback doesn't, so it'd be merely a convenience for migrating from Gatsby's onRouteChange - you want that?

I see the eslint error now - sorry I didn't check earlier. Mind if I change eslint rules by adding quotes with "allowTemplateLiterals": true (see http://eslint.org/docs/rules/quotes), or would you prefer concatenating strings across multiple lines, or just a single long line with the string?

[KyleAMathews]

Yes, please pass the location object through onRouteUpdate.

So on the lint error, I don't see why you'd have to change the lint rules. Perhaps all you need to do is remove the back slashes? It's not necessary to escape new lines with back ticks as they work for multi-line quotes.

[AndersDJohnson]

@KyleAMathews OK, now passing location.

I didn't want the newlines in the output. So I've just collapsed it to a single long line, and that passes eslint. Hope that's OK.

[KyleAMathews]

Looks great, thanks for taking this on! Will make a quick new release in a sec.

[AndersDJohnson]

@KyleAMathews We have an issue here. Apparently getCurrentLocation is a private API (remix-run/history#48) and can be undefined, as it is for me lately locally. I'll try to work on a replacement.

[KyleAMathews]

👍 thanks for noticing + taking care of this!

[AndersDJohnson]

@KyleAMathews See #343. Thanks.