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

When route path matches getChildRoutes should be called right away #3852

Closed
kirill-konshin opened this issue Sep 14, 2016 · 14 comments
Closed
Labels

Comments

@kirill-konshin
Copy link
Contributor

kirill-konshin commented Sep 14, 2016

There is a difference in getChildRoutes vs childRoutes behavior. IndexRoutes along the way are expected to be used even if they belong to "empty" path sub-routes, which is true for childRoutes and does not work when routes are asynchronously loaded via getChildRoutes.

Version

2.8.1

Test Case

http://jsbin.com/xajuyoj/5/edit?js,console,output

Steps to reproduce

Click "Feed", then click "Feed 1"

Expected Behavior

When "Feed" is clicked getChildRoutes is expected to be loaded (because route matches /feed), index page supposed to be displayed.

Actual Behavior

getChildRoutes is not loaded when "Feed" is clicked.
getChildRoutes is called only when route becomes /feed/1, e.g. when "Feed 1" is clicked.
If sync childRoutes is used the behavior is correct.

PS

The rationale is to offload the entire sub-tree (incl. index route and route with params) into a code split point. Seems that right now the only option is to separately offload index route and child routes, there's no way to put them both in one chunk.

@timdorr timdorr added this to the v3.0.0 milestone Sep 14, 2016
@timdorr
Copy link
Member

timdorr commented Sep 16, 2016

You should be using indexRoute's async counterpart getIndexRoute: https://github.com/ReactTraining/react-router/blob/master/docs/API.md#getindexroutepartialnextstate-callback

@timdorr timdorr closed this as completed Sep 16, 2016
@kirill-konshin
Copy link
Contributor Author

No, please look again.

I have specifically pointed out the difference in behavior of childRoutes vs. getChildRoutes. I am aware of getIndexRoute method, but that's not the case since IndexRoute belongs to a nested Route w/o path and this is done specifically to avoid getIndexRoute:

Root path="/" component={App}
  Root component={Layout} path="feed" <------ this one has getChildRoutes
    Root component={FeedLayout} <------- this one is loaded asynchronously
      IndexRoute
      Route path=":feedId"

If setup is synchronous everything works, if setup is async getChildRoutes is not called until user navigates anywhere inside /feed/ route, for instance /feed/1, but in fact, async load should happen when at least /feed is matched.

@timdorr timdorr reopened this Sep 16, 2016
@timdorr timdorr added the bug label Sep 16, 2016
@timdorr
Copy link
Member

timdorr commented Sep 16, 2016

Yeah, looks like you're right. Sorry about the quick closing.

@khakulov
Copy link

khakulov commented Sep 22, 2016

is there any idea how to fix this?

@kirill-konshin
Copy link
Contributor Author

kirill-konshin commented Oct 11, 2016

Any updates? It's 4 weeks since it was created...

@ryanflorence
Copy link
Member

Our limited time to work on this is focused on v4, but we will happily merge a pull request to correct this behavior.

@ryanflorence
Copy link
Member

It's 4 weeks since it was created...

This is open source, so you are just as responsible as us to have done something in the last four weeks 😉

@kirill-konshin
Copy link
Contributor Author

If only I knew where to look at least :) any suggestions? I am totally fine to make a PR.

@ryanflorence
Copy link
Member

I haven't been in that code in a long time, I'd drop a debugger in your getChildRoutes call and then follow the stack trace up as a starting point.

@ryanflorence
Copy link
Member

if I remember right, it's probably somewhere in "transition manager"

@kirill-konshin
Copy link
Contributor Author

OK, I'll take a look. Basically the issue boils down to a place when async getChildRoutes should be loaded if previous URL portion matches, e.g. when route is /feed/1 it's called and when /feed it's not...

@timdorr
Copy link
Member

timdorr commented Oct 12, 2016

Actually, you'll want to look here: https://github.com/ReactTraining/react-router/blob/v3/modules/matchRoutes.js

If you want to do a PR (which would be great!), you should do so against the v3 branch since it's technically a behavior change and should go into the next major anyways.

@timdorr timdorr mentioned this issue Oct 14, 2016
8 tasks
@timdorr timdorr removed this from the v3.0.0 milestone Oct 25, 2016
@timdorr timdorr added the v3 label Oct 25, 2016
@kirill-konshin
Copy link
Contributor Author

I've updated my setup to 3.0.0 but the behavior is still the same.

http://jsbin.com/lihagoxupu/2/edit?js,console,output

@kirill-konshin
Copy link
Contributor Author

I confirm that bug is not reproduced on 3.0.2. Updated setup: http://jsbin.com/cidojibijo/5/edit?js,console,output

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 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

4 participants