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

[META] v3.0.0 #3611

Closed
2 of 8 tasks
taion opened this issue Jul 5, 2016 · 34 comments
Closed
2 of 8 tasks

[META] v3.0.0 #3611

taion opened this issue Jul 5, 2016 · 34 comments
Labels
Milestone

Comments

@taion
Copy link
Contributor

taion commented Jul 5, 2016

I'm setting this up as a meta-issue instead of a milestone to allow for more commentary and discussion. I'm going to close the originating issues and fold them into here for now, so we have a single issue on which to track things.

We're looking to cut a v3.0.0 soon, starting with PRs, to update dependencies, clean up deprecations from v2.x, and give users a path forward. I'm going to use this meta issue to list out the changes we're looking at making as we lead into release.

Requirements

These are the things that we must resolve before release:

Wishlist

These are the things that I'd personally like to get to before release, but am willing to discard if they cause too much schedule slip:

  • Pull out <LocationProvider> component to inject down location as a prop into a separate component that just handles routing state management
    • Would allow better Redux integration, since the matcher could then consume location from the store
    • Might not be possible? How do we interact with listenBefore?
  • Move matching logic into route objects (Customizable URL matching rules #2286)
    • Instead of having a top-level matchRoutes handler, instead have the router ask each route for whether it matches, and the matched routes
    • This would allow creating e.g. <HapiRoute>, to allow users to use route handlers with their choice of route matching
    • We could get a proper routeParams too, as each route would give its matched params (Empty routeParams cause  #3549)
  • Support returning promises from dynamic methods on routes (getComponent, getChildRoutes, &c.) (Feature request: Promise support in getComponent #3606)
    • This will allow better support for System.import
  • Remove some edge case handling in default matching logic (with deprecations on v2.x)
    • Remove default support for nested absolute routes whose parents don't match (this can be handled in a custom-matching route) (Remove support for nested absolute routes #3172)
    • Change default pattern handling to path-to-regexp? This would make things easier to learn. We could warn on individual routes where the two matching algorithms give different results
  • Split up client-side and server-side match helpers, since they don't do quite the same thing (Revisit server-side API and isolate into subdir #3290)
  • Ensure that context subscriber handling works with context interceptors, as this will be needed for relative link support (but such support proper probably is too ambitious to squeeze into v3.0.0)

Well, please discuss.

The idea is to avoid any major breaking API changes – a working v2.x config should work with minor changes in v3.0.0. I'd like to take the opportunity to clean up cruft, but it's intentional that the wishlist is much longer than the list of requirements – I'm not going to be heartbroken if we can't get to some of these.

@taion taion added this to the v3.0.0 milestone Jul 5, 2016
@taion
Copy link
Contributor Author

taion commented Jul 5, 2016

Oops! Hit "submit" too quickly.

cc @reactjs/routing, @gaearon

The wishlist really is the wishlist. I'd like to get an rc.1 with the upgraded history dep this week if possible, and the rest is just "as we have time before we hit a reasonable release deadline".

@taion
Copy link
Contributor Author

taion commented Jul 5, 2016

BTW, "support async route leave hooks" is I think an explicit non-feature here. Users should leverage the custom confirmation prompt support in history to accomplish showing custom dialogs there.

This was referenced Jul 5, 2016
@timdorr
Copy link
Member

timdorr commented Jul 5, 2016

Regarding the history v3 upgrade, how do we want to handle that? Should we support both v2 and v3 histories? The API change around location is rather significant, so I know it would break a number of tools (react-router-redux most notably, but the fix is really simple).

@taion
Copy link
Contributor Author

taion commented Jul 6, 2016

I'm condensing and clearing out some earlier comments:

@dmitriid notes that the withRouter API doesn't match the API for route components in 2.x. This is resolved in 3.0.0-alpha.1, where withRouter also injects routes, params, and location. Thus, the injected props to a withRouter component mostly matches what's available to a route component, net of route, which we would need extra work to be able to inject. We will keep context.router as a supported API, subject to React not changing how context works under us, &c.

@umidbekkarimov mentions push handlers with special query support. We're currently not planning on implementing something along those lines, because the current push API is already quite good for merging and overriding queries:

router.push({ ...location, query: newQuery })
router.push({ ...location, query: { ...query, ...newQuery } })

@taion
Copy link
Contributor Author

taion commented Jul 6, 2016

For history v3, I think the right way to go forward is to do something like this check: https://github.com/reactjs/react-router/blob/v2.5.2/modules/Router.js#L18-L21

The user-facing APIs are largely identical, so we'll just force the user to supply a v3 history, and throw if the history does not support getCurrentLocation.

@timdorr
Copy link
Member

timdorr commented Jul 13, 2016

I was going to PR something around that soon. It shouldn't be hard, just change things like this call to getComponents to check for a Promise.

@donaldpipowitch
Copy link

I have the feeling that async onEnter isn't working as expected in general (imho).

Say I have a route-a rendering ComponentA and a route-b rendering ComponentB and switching between both routes takes 2 seconds.

  1. When I switch from route-a to route-b I wouldn't expect that the URL changes immediately, but only if I call the callback inside onEnter. That way I can abort the transition if an error happens or redirect to a different route without changing the URL to a route which is never rendered.
  2. When I switch from route-a to route-b and I go back in my browser history after 1 second, I see the URL for route-a, but ComponentB will be rendered. (This is basically a different scenario for issue Async onEnter hook can lead to inconsistent state #3476.)
  3. Animations don't behave as expected (explained here).

It would be nice if this can be addressed in v3 and if this works as intended, it would be nice if the docs could explain this behaviour.

@yormi
Copy link

yormi commented Aug 11, 2016

I don't know if it can be useful for you guys but since 3.0.0-alpha.2, the first click on a Link when the app is loaded change the URL but does not render the component associated to the new URL.

After that first click, everything works fine though.

I use react-router-redux, I don't know if it changes something...

@taion
Copy link
Contributor Author

taion commented Aug 15, 2016

Quick update for anyone that cares – main blocker for moving to an actual RC is figuring out how to display loading status when doing async routing, as that's become a major issue for us at work.

Need to figure out the appropriate API here, and I don't think we should commit to an RC until we've figured that out (would ideally like to avoid that sort of API change after we hit RC).

@jedwards1211
Copy link

@taion some thoughts about loading status:

These days I prefer doing async loading in a userland component (that displays loading status) instead of using an async getComponent. IMO getComponent isn't really the best place to do async loading, and it would be better to deprecate it and encourage users to pass in a component that does the async loading (and status display if they want).

On the other hand, getChildRoutes is quite different. I have a userland solution for displaying loading status while async loading child routes too, but it involves several messy hacks. So I think a good API for returning a temporary loading route while the real child routes are being loaded is worth the effort (and I'm guessing the same would be the case for getIndexRoute, though I haven't ever needed to use it).

@taion
Copy link
Contributor Author

taion commented Aug 16, 2016

Doing async loading in the component is tempting but suboptimal in the case of nested routes – it forces waterfalled loads, as we've discussed on other issues.

I consider that compromise to be unacceptable. Users should not do async loading as part of the render cycle.

@jedwards1211
Copy link

Oh man, I keep forgetting about that. How awkward...

@donaldpipowitch
Copy link

donaldpipowitch commented Aug 19, 2016

Maybe this is an interesting point for an upgrade guide:

  • Since react-router v3 uses history v3 hashHistory has no _k by default, if no location.state is used (see State management in hash history history#163).
  • Some libraries like taion/react-router-scroll need that key to work correctly, even if location.state isn't used. (Maybe this is a code smell in itself...?)

I'm not really sure currently what is the best way to bring back _k. I guess I need to create a custom hashHistory...?

EDIT:

Maybe the fastest way to get _k back.

hashHistory.listenBefore((location) => {
  if (location.state === undefined) {
    location.state = true;
  }
});

@buzinas
Copy link

buzinas commented Aug 20, 2016

I would like to have two other hooks: onBeforeEnter and onBeforeLeave. I needed those hooks in two projects already, and I'm always hacking something to make this behavior possible.

When entering a route, the general idea is:

  • If there is no onBeforeEnter hook set, we keep the current behavior
  • If it's set, it is expected to return a Boolean.
    • If its return value is truthy, we keep the current behavior
    • If its return value is falsy, then we don't call the onEnter hook and we cancel the current behavior.

The onBeforeLeave hook would work in a similar way when leaving a route.

@taion
Copy link
Contributor Author

taion commented Aug 20, 2016

We're not planning on adding additional route hooks at the moment. The API surface area is too large there, and many of the existing use cases are better handled at the component level.

While we don't plan on deprecating any existing route hooks in v3.x, we are unlikely to add any new ones. The set of use cases there are not broad enough to warrant the increased API surface area.

@zdila
Copy link

zdila commented Sep 9, 2016

Meanwhile history 4.0.0 has been released ;-)

@killroy42
Copy link

But how does it work? :(

@timdorr
Copy link
Member

timdorr commented Sep 13, 2016

history 4.0 will correspond with react-router 4.0, which is being worked on separately from 3.0. This is more of an evolutionary change, whereas 4.0 will be more revolutionary (and hence history 4.0 is a very big shift).

@killroy42
Copy link

I'm downgrading to 2.8.0. It seems that the 4.x release isn't official. This site https://react-router-website-uxmsaeusnn.now.sh/quick-start looks kinda dodge and the API is a bit of a mess now, aparently... I supose I upgraded pre-maturely. but a -beta or -rc tag would be great.

@timdorr
Copy link
Member

timdorr commented Sep 13, 2016

It's a pre-release, hence the - after the version. It's "official" in the sense that @ryanflorence released it. It has an entirely new design, so there isn't really a defined upgrade path just yet. You should give it a try on a new application, rather than trying to shoehorn it into an existing app with routing in it.

@killroy42
Copy link

Understood. I downgraded. Been optimistically updating a lot of things today, Not sure if I spend more time chasing bugs already fixed in more recent releases or new bugs introduced by more recent releases ;)

@CrocoDillon
Copy link

I know that this is about v3 but since we're talking about v4 already anyway, is there any place where I can find the reasoning behind the changes in v4? It seems like everything has changed and it would help me (and I'm sure others) to know why.

@ryanflorence
Copy link
Member

Yeah we'll have a write up soon. Short story is, React Router had too much API and got way too far from being a "React Router" but was rather a "Router for React". It wasn't composable, and it wasn't declarative. So we made it composable and declarative.

@jochenberger
Copy link
Contributor

On a side note, https://react-router-website-uxmsaeusnn.now.sh/ contains a dead link ("hosted on GitHub" points to https://github.com/reactjstraining/react-router).

@timdorr
Copy link
Member

timdorr commented Sep 16, 2016

Just merged in master to next and resolved some conflicts there. I think we're good to go up to beta now. Most other things in here can be addressed in a minor.

@timdorr
Copy link
Member

timdorr commented Sep 16, 2016

3.0.0-beta.1 is out! https://github.com/ReactTraining/react-router/releases/tag/v3.0.0-beta.1

npm install react-router@beta to get it!

@jedwards1211
Copy link

I'm not on twitter, so I'm posting here, v4 looks awesome! I read some of the examples and rationale today, and the new API makes so much sense. Reminds me how some of my own component designs have evolved as I've gotten more experienced using React. Can't wait to use v4!

@jflayhart
Copy link

jflayhart commented Sep 19, 2016

Really would love to see efforts for #2286. I am of the opinion, however, that this is a bug with react-router and we should be able to specify if an :id param is alphanumeric or of type number. It took me some time to debug these routing issues until I found this ticket; only to realize I just have to create a workaround for now.

@timdorr
Copy link
Member

timdorr commented Oct 14, 2016

@jflayhart 4.0 uses path-to-regexp, which lets you do that exact kind of thing.

Are we good to go on 3.0? #3852 is the only thing outstanding, and that can be done in a minor later.

@johnnyreilly
Copy link
Contributor

Is 3.0 still going to be released then? I kind of assumed you were going to jump straight to 4. That's not the case?

@timdorr
Copy link
Member

timdorr commented Oct 14, 2016

2.x/3.x will be maintained indefinitely, for those that don't want or aren't able to upgrade to 4.0. 3.0 rolls up some deprecation removals and a few improvements/fixes, but is otherwise a fairly boring release 😄

@johnnyreilly
Copy link
Contributor

That's actually great news. I know the project I'm working on is fairly large / complex and the likelihood of getting a thumbs up to go to 4.0 may well be a long time coming... So thanks 👍 😄

@taion
Copy link
Contributor Author

taion commented Oct 14, 2016

I'm not aware of any blockers.

@timdorr
Copy link
Member

timdorr commented Oct 25, 2016

We're live! https://github.com/ReactTraining/react-router/releases/tag/v3.0.0

@timdorr timdorr closed this as completed Oct 25, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests