Platform: Router selectors are only selecting route params of last child

Created on 13 Aug 2019  路  12Comments  路  Source: ngrx/platform

Minimal reproduction of the bug/regression with instructions:

https://stackblitz.com/edit/ngrx-router-aggregated-params

Expected behavior:

The router selector should aggregate the route params when there are embedded routes with multiple router outlets.

Working:

  {
    path: 'embedded',
    children: [
      {
        path: 'first/:foo',
        children: [
          {
            path: 'second/:bar',
            component: DummyComponent,
          },
        ]
      },
    ],
  },

Not Working:

  {
    path: 'container',
    children: [
      {
        path: 'first/:foo',
        component: ContainerComponent, // <-- swallows upper route params (foo)
        children: [
          {
            path: 'second/:bar',
            component: DummyComponent,
          },
        ]
      },
    ],
  },

Other Informations

Current Code: https://github.com/ngrx/platform/blob/master/modules/router-store/src/router_selectors.ts

I would propose something like this:

        let route = routerState.root;
        let params: Params = {};
        while (route.firstChild) {
            route = route.firstChild;
            params = {
                ...params,
                ...route.params,
            };
        }

I would be willing to submit a PR to fix this issue

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

Accepting PRs Docs Router Store

Most helpful comment

If somebody stumbles on this, here is the recipe I came up with. It accumulates all route params:

export const selectRouteNestedParams = createSelector(selectRouter, (router) => {
  let currentRoute = router?.state?.root;
  let params: Params = {};
  while (currentRoute?.firstChild) {
    currentRoute = currentRoute.firstChild;
    params = {
      ...params,
      ...currentRoute.params,
    };
  }
  console.log(params);
  return params;
});

export const selectRouteNestedParam = (param: string) =>
  createSelector(selectRouteNestedParams, (params) => params && params[param]);

All 12 comments

@timdeschryver Would this change be accepted? Then I would submit a PR, other ways I can save the effort.

I see a problem with this and I'm not sure if we should move on with this or not.
If a user would use the same key in a route, one would override the other, e.g. /feature/{id}/child/{id}.

@brandonroberts what are your thoughts here?

Came across this issue because of a similar use case.

Would it be possible to return params nested by the outlet they're in (if there's nested outlets)? So in this case, something like

{
  primary: {
    foo: 'containerFirst',
    primary: {
       bar: 'containerSecond'
    }
  }
}

which is a little gross, but at least the data would all be there and the process of digging this out would be abstracted from the user of the selector, and the user of the selector could at least write additional selectors on top of this to clean it up into the form that they need.

Another possibility would be to provide different selectors so the developer can decide which one is needed.
e.g. selectRouteParams and selectComposedRouteParams

The problem I'm experiencing is I have two router outlets at the same level, named 'primary' (default) and 'secondary'. My solution is to provide a custom serializer. In my case, to include params from 'secondary' routes, I have to recursively check all of the children starting from the top, instead of just firstChild, which is also susceptible to the same problem @timdeschryver called out, but it works if I make sure not to repeat different param names in different routes.

So my issue is just a different form of the original, just slightly different solution due to route config. But same thing problem: I'm only getting params from 'primary', not 'secondary' because of the selector only pulling from firstChild.

What we currently have fits most scenarios without issue, so I don't think we should modify the existing behavior. We could expose the selectRouterState selector that gives you the top-level router state so you can build your own selectors to get at the data you need.

I agree with @brandonroberts , it would be hard to cover all cases.
What about creating a router recipe on how to write your own router selectors if that would be helpful.

@timdeschryver How can I propose a recipe? Simply adding docs/router-store/selectors.md or is there a special place/convention for recipes?

The docs folder is for the v6 documentation, so not there.

You can add it the ngrx.io project with the documentation of @ngrx/router-store.
Currently router-store does not have recipes, but you can take a look at the @ngrx/store documentation. You basically have to create a new recipes folder in the docs folder and create the selectors.md file in there.

The new recipes folder has to be added to the site navigation.

If something is unclear you can always ping us for more information 馃檪

If somebody stumbles on this, here is the recipe I came up with. It accumulates all route params:

export const selectRouteNestedParams = createSelector(selectRouter, (router) => {
  let currentRoute = router?.state?.root;
  let params: Params = {};
  while (currentRoute?.firstChild) {
    currentRoute = currentRoute.firstChild;
    params = {
      ...params,
      ...currentRoute.params,
    };
  }
  console.log(params);
  return params;
});

export const selectRouteNestedParam = (param: string) =>
  createSelector(selectRouteNestedParams, (params) => params && params[param]);

Hi @qortex , do you want to add this to the docs?

Hi @qortex , do you want to add this to the docs?

Sure, here is the PR #2748

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hccampos picture hccampos  路  3Comments

smorandi picture smorandi  路  3Comments

NathanWalker picture NathanWalker  路  3Comments

shyamal890 picture shyamal890  路  3Comments

sandangel picture sandangel  路  3Comments