handle nested IndexRoute in path-less route

[danieljuhl]

This test is related to #1939 which was closed, but not fixed.

[knowbody]

@danieljuhl I was taking part in this conversation, and I thought the solution was there. Could you explain once again what is the use case of nesting "path-less" routes? What's the point of that? Seems not right to me...

[danieljuhl]

@knowbody Unfortunately the issue was just closed, with open questions. Consider the following reduced example:

    <Router history={ history }>
      <Route path='/' component={ App }>
        <Route path='signup' component={ Signup } />
        <Route path='login' component={ Login } />
        <Route component={ ListPage }>
          <Route path='search' component={ Search } />
          <IndexRoute component={ Home } />
        </Route>
        <Route path='*' name='not-found' component={ NotFound } />
      </Route>
    </Router>

Search (/search) and Home (/) is both a ListPage, with some surrounding logic and a sidebar, but that logic and sidebar should not apply to Login (/login) and Signup (/signup)

[knowbody]

cool, now I remember the discussion, thanks. it looks good to me, can you rebase and I'll merge that

[danieljuhl]

Like this?

[knowbody]

thanks

[taion]

Oh very nice - I've hit this a few times and didn't realize it was a bug.

[hyzhak]

Great! thanks for patch. When will you release it in npm?

[knowbody]

when everything in 1.0 release milestone will be done

[hyzhak]

@knowbody ok, it seems shouldn't get a lot of time https://github.com/rackt/react-router/milestones/v1.0.0-final only thing that there left is #2302 Release the last version for 0.13.x

[tobiash]

Is it intentional that this only descends into childRoutes, but does not look at getChildRoutes?

[danieljuhl]

@tobiash what do you mean? Can you provide a use case/code sample, that does not get covered by the PR.

[taion]

I'm not sure we should support getChildRoutes there - we'd potentially have to keep speculatively loading multiple sets of child routes in various edge conditions and arbitrarily block the transition.

[tobiash]

@danieljuhl the following should demonstrate the issue:

const routes = [{
  path: '/',
  component: App,
  getChildRoutes(location, cb) {
   return cb(null, [
      { path: 'signup', component: SignUp },
      { path: 'login', component: Login },
      { 
        component: ListPage,
        indexRoute: { component: Home },
        childRoutes: [
          { path: 'search', component: Search }
        ]
      }
    ]);
  }
}]

If you go to /, the matching algorithm starts looking for an index route at the root level, while it would look into childRoutes, it does not call getChildRoutes.

my specific usecase was code-splitting with webpack. If this would work I could do:

{
  path: 'submodule',
  getChildRoutes(location, cb) {
    require.ensure(['./submodule/Index', './submodule/App', './submodule/Page'], cb(null, [{
      component: require('./submodule/App'),
      indexRoute: { component: require('./submodule/Index') },
      childRoutes: [
        ...
      ]
    }]);
  }
}

Without it, I have to implement three methods with require.ensure, like

{
  path: 'submodule',
  getChildRoutes(location, cb) { require.ensure(...) },
  getIndexRoute(location, cb) { require.ensure(...) },
  getComponent(location, cb) { require.ensure(...) }
}

Yes, it might lead to childRoutes being loaded that don't actually contain an indexRoute, but I wonder if there are many valid usecases where multiple childRoutes would get loaded? And if it's a problem, you could always short-circuit that by providing a (dummy) indexRoute at the root-level.

[taion]

@tobiash Can you open a new issue to discuss this further? It'll make it easier to keep track of the discussion.

[dnutels]

So, if I understand correctly, this issue:

<Router history={createHistory()}>
    <Route path="/" component={App}>
        <Route component={Index}>
            <IndexRoute component={SomeComponent}/>
            <Route path="another" component={AnotherComponent}/>
        </Route>
        <Route path="*" component={NoMatch}/>
    </Route>
</Router>

is fixed and for (for example) localhost:6001/ would load App -> Index -> SomeComponent.

Why is <Route component={Index}> not <IndexRoute component={Index}> then?

It essentially is, apparently. Should we allow nested IndexRoutes?

[taion]

Because you want both / and /another to be nested under Index.

[dnutels]

I understand. I was asking whether you could, theoretically, merge the decision logic between no-path Route (which matches its children) and IndexRoute (when the route didn't directly match)?

Basically have childless IndexRoute to behave as it is now, and IndexRoute with children to behave like no-path Route?

As it is now - is there a use case for IndexRoute that has children, but doesn't behave like no-path Route?

The reason for the question was that I felt a bit of a-symmetry...

[taion]

There are a couple of good reasons why we don't want to do that. The issue tracker isn't the right place to discuss them, though, sorry.

[cpsubrian]

Just chiming in here to see if there's been movement. I'm also starting down the path of code-splitting and have the following JSX routes:

<Route path='/' component={AppLayout}>
  {/* Admin Pages */}
  <Route path='admin' component={AdminLayout}>
    <IndexRoute component={AdminDashboard}/>
    <Route path='users' component={AdminUsers}/>
    <Route path='features' component={AdminFeatures}/>
    <Route path='campaigns' component={AdminCampaigns}/>
    <Route path='*' component={NotFound}/>
  </Route>

  {/* Front-end Pages */}
  <Route component={FrontendLayout}>
    <IndexRoute component={Home}/>
    <Route path='campaigns' component={Campaigns}/>
    <Route path='campaigns/:id' component={Campaign}/>
    <Route path='about' component={About}/>
    <Route path='privacy' component={Privacy}/>
    <Route path='login' component={Login}/>
    <Route path='*' component={NotFound}/>
  </Route>
</Route>

The issue is how to convert this to dynamic routes without loading the
front-end if you navigate directly to the backend.

As stated above I'm pretty sure I can solve this by using getChildRoutes, getIndexRoute, getComponent and copious amounts of require.ensure calls.
Would prefer not too 😄

[cpsubrian]

FWIW the most terse version that I could get working of the above is:

// Polyfill webpack require.ensure for node.
if (typeof require.ensure !== 'function') require.ensure = (d, c) => c(require)

// Export the routes.
export default {
  path: '/',
  component: require('../components/routes/layouts/AppLayout'),
  childRoutes: [{
    // Admin
    path: 'admin',
    getComponent: (location, cb) => {
      require.ensure([], (require) => {
        cb(null, require('../components/routes/layouts/AdminLayout'))
      })
    },
    getIndexRoute: (location, cb) => {
      require.ensure([], (require) => {
        cb(null, {
          component: require('../components/routes/admin/Dashboard')
        })
      })
    },
    getChildRoutes: (location, cb) => {
      require.ensure([], (require) => {
        cb(null, [
          require('./admin')
        ])
      })
    }
  }, {
    // Frontend
    getComponent: (location, cb) => {
      require.ensure([], (require) => {
        cb(null, require('../components/routes/layouts/FrontendLayout'))
      })
    },
    getIndexRoute: (location, cb) => {
      require.ensure([], (require) => {
        cb(null, {
          component: require('../components/routes/frontend/Home')
        })
      })
    },
    getChildRoutes: (location, cb) => {
      require.ensure([], (require) => {
        cb(null, [
          require('./frontend')
        ])
      })
    }
  }]
}

The meat of the routes are still JSX and are in ./admin and ./frontend. I'm not sure there's really any action to take here, but it would be much more convenient syntax-wise if nested index routes in getChildRoutes were found.

[knowbody]

Please use Stack Overflow or Reactiflux for questions – not the issue tracker.

[cpsubrian]

Totally get it, but there is a bug here that I thought was being discussed still, and I'm hitting it. Just wanted to add my use case to the pile. Going away now 😄

[taion]

There is not a bug here. You're just doing something wrong.

[cpsubrian]

Hrmm, maybe I'm misreading the issue. Will see if I can work up a simpler way to reproduce and open a new issue if so. Sorry for polluting this PR.

[taion]

We're considering deprecating and removing this feature to remove the inconsistency between synchronous and asynchronous child routes. See #3326.