nareshbhatia / mobx-state-router

MobX-powered router for React apps
https://nareshbhatia.github.io/mobx-state-router/
MIT License
227 stars 30 forks source link

How to handle nested route or and default routes #6

Closed thupi closed 6 years ago

thupi commented 6 years ago

First of all I want to say this is a awesome solution to routing using mobx. I am currently using router5 to achieve something like that. Mobx-state-router is just way cleaner.

I am curious about two things. How to handle nested routes and something like default routes :-) An example would be that one would like /administration t become /administration/settings. In that way settings page will be the default child.

The other thing is nested routes. An example would be handling auth on a route and the following nested routes.

These are my only concerns a the moment. I will definitely give it at try in the following month. This could potentially solve a lot of my headache with custom routing modules :-) !

nareshbhatia commented 6 years ago

Haven't tried it myself, but I think something like this will work (in your routes.js):

{
    name: 'administrationTemp',
    pattern: '/administration',
    beforeEnter: (fromState, toState, routerStore) => {
        return Promise.reject({
            fromState: fromState,
            toState: new RouterState('administration', { id: 'settings' })
        });
    }
},
{
    name: 'administration',
    pattern: '/administration/:id'
},

Don't understand your second question. Have you seen the docs for Redirecting to the Sign In page?

thupi commented 6 years ago

@nareshbhatia I will definitely give this a try :-)

My second question is about handling auth on a parent route in order to make the children require auth.

Another example would be the possibility of having nested RoouterView components:

Root -> RouterView -> Administration -> RouterView -> SettingsTab :-)

For reference in router5 they have a property called children in their route definition. If App require auth all of the children will require auth too.

This alle makes me able to have Tabs that can use the URL. The tabs will just be a RouterView which will be able to show the child routes of the parent :-)

nareshbhatia commented 6 years ago

@thupi, if I understand you correctly, you are trying to protect all your administration tabs in one place. That should be very simple. Just put a beforeEnter hook on the /administration/:id pattern like this:

{
    name: 'administration',
    pattern: '/administration/:id',
    beforeEnter: (fromState, toState, routerStore) => {
        // check if user is authenticated, if not redirect to sign in page
},

The mobx-shop example goes even further. It creates a reusable hook called checkForUserSignedIn. This hook is called from any route that needs to be protected. For example:

{
    name: 'administration',
    pattern: '/administration/:id',
    beforeEnter: checkForUserSignedIn
},

The checkForUserSignedIn hook remembers the route that was requested by the user and redirects to that route after a successful login.

Hope this clarifies.

thupi commented 6 years ago

@nareshbhatia Alright that seams to solve the problem. But the point i am tring to make is a little different.

In order to show my issue i will just go ahead an provide my current route config :-)

const routes: Array<Route> = [
  {
    name: ROUTE_NAMES.LOGIN,
    path: '/login'
  },
  {
    name: ROUTE_NAMES.SIGNUP,
    path: '/signup'
  },
  {
    name: ROUTE_NAMES.FORGOT,
    path: '/forgot'
  },
  {
    name: ROUTE_NAMES.RESET,
    path: '/reset?secret&id'
  },
  {
    name: ROUTE_NAMES.APP,
    path: '/',
    canActivate: (router, dependencies) => (toState, fromState, done) => {
      if (dependencies) {
        if ((dependencies.store as IStore).meStore.hasToken) {
          done();
          return true;
        } else {
          done({ redirect: { name: ROUTE_PATHS.LOGIN } });
          return false;
        }
      } else {
        done({ redirect: { name: ROUTE_PATHS.LOGIN } });
        return false;
      }
    },
    children: [
      {
        name: ROUTE_NAMES.APP_INTRO,
        path: 'intro'
      },
      {
        name: ROUTE_NAMES.APP_NETWORK,
        path: 'networks/:networkId',
        children: [
          {
            name: ROUTE_NAMES.APP_NETWORK_NOTIFICATIONS,
            path: '/notifications'
          },
          {
            name: ROUTE_NAMES.APP_NETWORK_PEOPLE,
            path: '/people',
            children: [
              {
                name: ROUTE_NAMES.APP_NETWORK_PEOPLE_SEARCH,
                path: '/'
              },
              {
                name: ROUTE_NAMES.APP_NETWORK_PEOPLE_STATISTICS,
                path: '/statistics'
              },
              {
                name: ROUTE_NAMES.APP_NETWORK_PEOPLE_MYTAGS,
                path: '/my-tags'
              },
            ]
          },
          {
            name: ROUTE_NAMES.APP_NETWORK_QUESTIONS,
            path: '/questions'
          },
          {
            name: ROUTE_NAMES.APP_NETWORK_ADMINISTRATION,
            path: '/administration',
            children: [
              {
                name: ROUTE_NAMES.APP_NETWORK_ADMINISTRATION_DETAILS,
                path: '/'
              },
              {
                name: ROUTE_NAMES.APP_NETWORK_ADMINISTRATION_TAG_MANAGER,
                path: '/tag-manager'
              },
              {
                name: ROUTE_NAMES.APP_NETWORK_ADMINISTRATION_MEMBERS,
                path: '/members'
              }
            ]
          }
        ]
      }
    ]
  }
];

As you see i have ROUTE_NAMES.APP which maps to <App />. Inside i have ROUTE_NAMES.APP_INTRO which maps to <AppIntro /> and ROUTE_NAMES.APP_NETWORK which maps to <AppNetwork /> and so on.

In order to achive this i have to have some nested RouterViews. So there is a Router at the root which maps to the ROUTE_NAMES.LOGIN, ROUTE_NAMES.SIGN_UP, ROUTE_NAMES.APP

But inside the ROUTE_NAMES.APP i have one more RouterView. This will only map the children of ROUTE_NAMES.APP. In this case ROUTE_NAMES.APP_INTRO and ROUTE_NAMES.APP_NETWORK.

I hope this explains my need of nested routes. Maybe this kind of routing is not what mobx-state-router is meant for.

nareshbhatia commented 6 years ago

Ah, I now see what you mean. mobx-state-router does not have the concept of nested routes or nested RouterViews. That's the trade-off I made to keep the design simple. You will have to flatten your routes to make them work with mobx-state-router. For most common use-cases this approach works. You will have standalone routes for /, /intro, /networks/:networkId etc. I assume these are separate pages in your app. Your viewMap will simply map these pages to routes. Note that routes are matched from top to bottom, so the recommendation is to a) alphabetize them by pattern and b) keep the more specific routes above the more generic routes (e.g. routes with parameters). Try it - I think you will be fine.

thupi commented 6 years ago

@nareshbhatia Alright :-)

It is actually not :-) In some of the deeply nested routes the View uses information delivered by the parrent. That way i can share a lot of behavior and ui across multiple pages while keeping most of ther different pages accessiable by URL's :-)

The thee of routes maps directly to my tree of routers that are nested. So to flatten them will take a while and i do acutally like the way the routing works in the application right now. All in all i think mobx-state-router have a very nice approch to routing and i like the mix of router5 and mobx. However, this router might just not solve my need for nested routes :-)

Thanks for helping me out with understanding the router :-) !

nareshbhatia commented 6 years ago

Glad to be of help, @thupi.

carlosagsmendes commented 6 years ago

@thupi and @nareshbhatia I'm trying to implement a master-details view pattern. It is a kanban board where I want to show card details.

But after reading this I think there's no way to do something like:

/boards/1 /boards/1/cards/2

Right?

nareshbhatia commented 6 years ago

I think you can implement as two different routes, both pointing to the same view:

  1. /boards/:boardId
  2. /boards/:boardId/cards/:cardId

Or using query parameters: /boards/1?cardId=2

The idea is that you want to get a boardId and an optional cardId to the view. Let me know if this works.

carlosagsmendes commented 6 years ago

Thank you @nareshbhatia

I've tried it, but I still have an issue: when I change the state the whole page is reloaded.

In my case, by clicking on a card I would like to show a modal window with the card details, without redrawing the whole UI.

Here's the online example https://codesandbox.io/s/qzkr4kzrw9

Also, while testing this I've found that I was getting more than one render with a single navigation:

Double Render

The problem seems to be passing params as numbers. Once I add a toString() to the params it only renders once.

routerStore.goTo(
      new RouterState("card", { boardId: 1, cardId: this.props.card.id })
    );

Maybe this behaviour is my fault.

Thanks again

nareshbhatia commented 6 years ago

@carlosagsmendes, thanks for giving this a try.

To give you some insight into the numbers issue, mobx-state-router compares the current router state with the history location to determine if a state change (and hence a re-render) is required. Since the history provides the the params as strings and they are being compared to numbers in your case, it detects a mismatch and does an extra render. The easiest way to work around this issues is not to use numbers as parameters. I did not come across this issue because I use UUIDs as my object keys. Would it be possible for you to not put numbers in your state params? toString() should work. I think that's what you are doing now, but just making sure.

For the second issue, I am not able to see the modal pop up in your example. If that is still giving you an issue, you can try something like this. Have the view always have the modal, however it is shown only when the cardId is present. I think this will optimize the rendering when cardId is passed in params. This is generally how modals work. See this Material UI sample as an example. Also this approach is consistent with UI being a function of the state. Please let me know if this works, otherwise we can think of some other way. I would like to make your use case work.

carlosagsmendes commented 6 years ago

@nareshbhatia many thanks for your suggestions and support.

mobx-state-router compares the current router state with the history location to determine if a state change (and hence a re-render) is required

Since the history provides the params as strings would it be possible to cast the state before doing the comparison? I've fixed it in my code but I think doing so might help avoid unintended re-renders.

I am not able to see the modal pop up in your example

I've changed it to a simpler UI case that demonstrates my concern. So now when you press the button it shows the card details:

cards

When I click on another button to see the respective card details, the whole UI is re-rendered

I also would really like to make this case work. I really like your router and approach 👍

Here's the link for the demo

nareshbhatia commented 6 years ago

Hi @carlosagsmendes, sorry for a delayed response - I was very busy at work.

I looked at your code and the rendering seems to be pretty efficient already. Notice that only two cards are being rendered - the one that is deselected and the one that is selected. All other cards are left untouched. To make this more apparent, I increased the number of cards to 6 and now you can see this very clearly. See my code here. Of course, the BoardPage is redrawn because the RouterView is specifically asking to redraw the page, but the actual elements that are repainted are probably none (thanks to virtual DOM). Going deeper, only the changed elements are being redrawn.

As you already know, the trick is not to dereference your MobX observables until you absolutely need them. So it is best to push the observables as deep as possible and dereference them when we actually need to render something. I have restructured your code to make this a bit more explicit (see comments starting with [Naresh]: .... Also I have taken out unnecessary injections where they are not needed. Especially in your CardListView you had injected rootStore just to handle onClick. I moved this handler to BoardView and now CardListView does not need the injection. It is a dumb standalone component that receives a Card as a prop. Also changed the name of this class to CardView because it is rendering a single Card - the new name seems more appropriate.

Hope this is helpful.

carlosagsmendes commented 6 years ago

Many thanks @nareshbhatia, I really appreciate your support.

I'm just getting started with Mobx (and React) but this looks very promising.

aap82 commented 6 years ago

@carlosagsmendes Take a look at this demo. In the Shell.js file, there is an alternate implementation of the RouterView Component which reacts to a new property in RootStore . With less than 10 lines, you can avoid top-level render when only params change.

carlosagsmendes commented 6 years ago

@aap82 I really like it. This gives me a lot of ideas 👍

I'm unable to get this working with React-Loadable. Would you mind checking it out?

@nareshbhatia do you see any inconvenient in using a custom RouterView component?

Thanks!

aap82 commented 6 years ago

@carlosagsmendes you're missing one word in your BoardPage.js file. As soon as I added it, it worked.

Let me know if you can't figure out.

nareshbhatia commented 6 years ago

@carlosagsmendes, I don't see any issues with using the custom RouterView component suggested by @aap82. It is actually a very nice optimization trick!

carlosagsmendes commented 6 years ago

Thank you both! @aap82 I guess I will have to read "Why we have banned default exports in Javascript and you should do the same" once a day, for a month or so.

davidmz commented 6 years ago

I also faced with the fact that mobx-state-router does not support nested routes. But it turned out that they are very easy to implement :) We just need a helper function that turns a nested routing tree into a non-nested list: https://gist.github.com/davidmz/148101963b980ddcd164770b2e428f62

See examples in the above gist. There are nested (children) routes in 'about' ('/about') branch: 'index' (with '' pattern) and 'team' ('/team'). After flattening they becomes two plain routes: 'about.index' ('/about') and 'about.team' ('/about/team'). Child pattern just appends to the parent one and child name joined with parent name via dot.

This helper also joins transition hooks. Enter hooks of flattened route calls in parent-child order and Exit — in child-parent. So it is possible to write, say, authorization hook just on parent node and it will be executed on each nested nodes.

nareshbhatia commented 6 years ago

Very clever, @davidmz! Thanks for sharing.

ShaneCallananAerlytix commented 3 years ago

One of the problems I'm having related to nesting routes is the ability to render a wrapper component around a set of routes. For instance, imagine we have the following routes:

root -> [/test] . . . . -> [/main] -> [/dashboard]

/test - Some page for testing stuff /main - Entry point of main application (passes some context down to other pages in the hierarchy) /main/dashboard - Uses context provided by <Main> component to do something

I don't seem to be able to do this since if we switch from /main to /main/dashboard, an entirely separate component is rendered. As a result, the wrapper for /main no longer exists.

It seems the only way to get around this is to duplicate the 'wrapper component' in each subroute.

Thoughts on this?