Make 0.13.x compatible with React 0.14

[benvinegar]

I work on a large React project, and upgrading our app to both react 0.14 and react-router 1.0.0 at the same time felt like a long, risky undertaking – especially with the huge, breaking API changes to react-router.

Instead, I decided to modify the 0.13.x branch of react-router to support react 0.14, mostly by just substituting calls to react's now-missing internal methods with the same libraries used by react-router 1.0 (e.g. "invariant", "history", and "warning").

I've tested this branch locally on my react 0.14 branch of Sentry, and it seems to work fine.

[mjackson]

The longer we maintain the 0.13.x branch the longer people are going to use it. If you want to volunteer to maintain it, we can merge this. But I don't want to.

[benvinegar]

I mean, my goal is to immediately transition off of 0.13.x as soon as possible. I don't need this to be merged – I can just point npm to my fork for a short duration until I get on to 1.0.

I'm sharing this PR because I think it could help people upgrade – now they can upgrade in stages, rather than all at once. We could agree that this is the last major change, slap a big deprecation notice on the 0.13.x README and call it a day.

[mjackson]

ok, gotcha. Remind me–did we commit our build dir in the 0.13.x branch?

[taion]

Sentry's pretty great. That said, apart from maintenance issues, I'm a little concerned about correctness here. Standalone warning uses console.error instead of console.warn to match React v0.14, which can be a bit of a breaking change for tests.

What happens if you use React Router v0.13.4 with React v0.14.0? Does it completely fail, or do you just get deprecation warnings all over the place?

[taion]

On a complete tangent, because I don't know the meaning of the word off-topic, I noticed that you guys were pulling in all of React-Bootstrap (and other libraries) for your vendor bundle. Is there a benefit to doing things that way? Seems like you end up with a lot of stuff in your vendor bundle that you couldn't possibly be using.

[benvinegar]

What happens if you use React Router v0.13.4 with React v0.14.0? Does it completely fail, or do you just get deprecation warnings all over the place?

It fails because react/lib/warning, react/lib/ExecutionEnvironment, and react/lib/invariant are gone. It seems they're now pulled to React via the fbjs module, which warns non-Facebook projects not to include so I didn't.

e.g. building after changing devDependencies to point to react 0.14.x

ERROR in ./modules/getWindowScrollPosition.js
Module not found: Error: Cannot resolve module 'react/lib/ExecutionEnvironment' in /full-path/react-router/modules
 @ ./modules/getWindowScrollPosition.js 4:16-57

[benvinegar]

On a complete tangent, because I don't know the meaning of the word off-topic, I noticed that you guys were pulling in all of React-Bootstrap (and other libraries) for your vendor bundle. Is there a benefit to doing things that way? Seems like you end up with a lot of stuff in your vendor bundle that you couldn't possibly be using.

I'm new to the project (and webpack, mostly), so I don't have a good answer for this. But I don't think application filesize is a big concern for us at the moment. Upgrading react/react-router, is though :)

[benvinegar]

ok, gotcha. Remind me–did we commit our build dir in the 0.13.x branch?

@mjackson – I assumed so based off this.

[mjackson]

Ah, good point. That release was a bit of a mess :P I don't think I was supposed to check in the build directory.

If you wouldn't mind just popping off the last commit that updates the build dir and increments the version, I can merge. I'll do that part myself, and hopefully not screw it up this time.

[benvinegar]

@taion – re: Sentry and react-bootstrap, we can continue conversation on getsentry/sentry#2171.

[taion]

The React Router v0.13 release scripts run build as part of the release I think?

I think given the choice between using React v0.13-style console.warn and React v0.14-style console-error for warnings, it'd make more sense to just fork the React v0.13 warning module and use console.warn.

Otherwise, would it be possible to use a mini-library like https://www.npmjs.com/package/can-use-dom for canUseDom rather than pulling in history?

[benvinegar]

Otherwise, would it be possible to use a mini-library like https://www.npmjs.com/package/can-use-dom for canUseDom rather than pulling in history?

Sure. I also considered just straight-up copying that function into the project – it's not going to change.

I think given the choice between using React v0.13-style console.warn and React v0.14-style console-error for warnings, it'd make more sense to just fork the React v0.13 warning module and use console.warn.

Also fine.

[taion]

I'm also in favor of this change BTW. I would have had an easier time of upgrading some of my projects if I could have separately upgraded React and React Router. Neither upgrade is particularly difficult, but nicer to be able to have smaller atomic changes for this sort of thing.

[benvinegar]

I think given the choice between using React v0.13-style console.warn and React v0.14-style console-error for warnings, it'd make more sense to just fork the React v0.13 warning module and use console.warn.

Sorry, I misunderstood this. So, React 0.14 uses console.error. Why shouldn't we mirror that behavior here? If the goal is to ready react-router users to upgrade to 0.14, it seems appropriate.

[taion]

You break people still using React v0.13 though. It has to be one or the other.

IMO the right approach here is to allow users to use v0.13.x with React v0.14 if absolutely needed, but then to encourage upgrading to 1.0.0 as quickly as possible, especially when using React v0.14.

[knowbody]

@taion I agree, it's hard to turn someone down, but then if there is 0.14 support ppl will ask for 0.15 support etc. and we will have to keep maintaining both, same as @mjackson mentioned. Might be worth just to say that if someone wants 0.14 support than they need to get React Router 1.0

[taion]

I think it'd be best to have a release that has some React v0.14 compatibility but maintains first-class support for React v0.13 in preference to React v0.14. The 1.0.0 API is different enough that I could see myself not wanting to make upgrading from v0.13.x to v1.0.0-rc{whatever} as a pre-requisite to upgrading to React v0.14.

[knowbody]

what about adding 0.14 support and saying "this is the last release for 0.13.x, update to 1.0"?

[taion]

Yup, and I think this PR does that - just quibbling over whether we should use console.warn (to match React v0.13) or console.error (to match React v0.14) for warnings.

[benvinegar]

So, I've made all the suggested changes and removed the built files / version bump.

[mjackson]

Awesome, thanks @benvinegar. I'll have some time to review this afternoon.

[knowbody]

looks great @benvinegar!

[taion]

This is looking pretty good.

[mjackson]

nm, I don't have time to review today. If anyone else does, please do and
merge.
On Wed, Oct 14, 2015 at 1:53 PM Jimmy Jia [email protected] wrote:

This is looking pretty good.

—
Reply to this email directly or view it on GitHub
#2262 (comment).

[taion]

I looked it over. Looks pretty good. Tests still pass under React v0.13.3.

[taion]

I guess we should release this as v0.13.5 at some point, then?

[idolize]

Would love to use this to upgrade react to 0.14, fix any problems there, and then upgrade to react-router 1.0 without having to do both at once 😄

[benvinegar]

+1 on publishing a new version.

[knowbody]

@idolize @benvinegar created #2302, we should have the 0.13.5 release soon