V2

[mjackson]

I believe this branch contains all the modifications we need to release v2. @rackt/routing please review in case I missed something.

[mjackson]

The only thing I'm a bit hesitant about is that we didn't warn people we were going to remove createLocation as public API, but I think that's a relatively minor change because it was only ever really used for server-side rendering.

Also, should probably update docs before merging.

[taion]

What do we want to do with the match API in React Router? Currently it calls into history.createLocation: https://github.com/rackt/react-router/blob/v1.0.1/modules/match.js#L37-L39.

I'm fine with deprecating createLocation - we just need to figure out how to write that function before we proceed, though.

[timdorr]

I think we should keep it. You can go from LD to string with createHref or createPath, but not back? Since we seem to prefer LDs, that seems a bit backwards.

[taion]

The point of createLocation is to go from location descriptor to location, so the router can match. Maybe we should update createLocation to also take a location descriptor? I don't know how to make match work otherwise.

Maybe we should be unconditionally calling history.createLocation on the input to match.

[taion]

Okay, sorry, I think we're actually not ready for 2.x yet... I totally forgot about createLocation.

I think we should update createLocation to work with match and push that out as 1.16.x. Then we'll finally have fully forward-compatible APIs, and React Router 1.1.x can depend on "history": "^1.16.0 || 2.x".

This way users will actually not be broken, and they'll have the full set of deprecations.

[mjackson]

Users shouldn't ever actually need to use createLocation. It's private API.

Server-side match can just work like:

match(req.url, routes, ...)

That's the beautiful thing about us being able to accept either a path or an object as a "location descriptor". You can give us a plain URL if that's all you have. In a lot of ways it's actually simpler.

[timdorr]

But how does match convert that string to a location descriptor? Via private API usage?

[taion]

Nope, I figured out how to do it without using any private APIs, and without some of the hacks that I attempted on the way:

remix-run/react-router#2680

[taion]

Per this discussion on remix-run/react-router#2680, we're still going to have to update history.createLocation to accept a location descriptor, and warn when called with multiple arguments.

We can and should drop the non-instance-bound createLocation as a public API, though - there's no reason for anyone to use that when history.createLocation is available.

[mjackson]

@taion @timdorr I've added back the history.createLocation instance method, but no warnings about multiple arguments. At this point I'd rather just ship v2.

[taion]

I can try to put together a PR for createLocation later today or maybe tomorrow. At this point, until we update React Router, nobody will be able to use history v2 anyway, so there's not much immediate benefit from releasing right now.

[mjackson]

The only real immediate benefit to me is that I get to cross something off my TODO list.

[taion]

That's a pretty good benefit (:

This sounds a little byzantine, but what do you think of releasing a 1.16.0 with the deprecation warnings removed (maybe after we fix createLocation)?

I'm a little uncomfortable that all the code we've pushed since 1.13.1 isn't getting used (even through the backward compat shims). It means that we aren't getting e.g. bug reports, so if there are problems, we won't know until (much) later.

I'm thinking something like:

1. Fix createLocation API
2. Release a 1.16.0 without deprecation warnings that users can upgrade to for the moment until we're done with React Router 1.1.x
3. Release 2.0.0
4. Release a 1.17.0 that adds back the deprecations

It sounds weird written out like this, but it means that we can start exercising the new/modified code paths, and get more confidence around the new code working, which means we have more time to fix bugs as they come up.

[mjackson]

ok, let's do it.

[taion]

👍

[taion]

💯 on ripping out my confusing mess here.

[timdorr]

I noticed this diverged quite a bit from master. Doing a rebase now to get everything back in line.

Edit: Just because I'm paranoid, I left the old version on my fork.

[timdorr]

BTW, this looks all good to me too.

[mjackson]

Instead of removing deprecated code in 2.0, we'll remove it in 3.0.