perf(router-store): optimize selectQueryParams, selectQueryParam and …

…selectFragment selectors
ngrx · Jan 4, 2021 · 37e4a91 · 37e4a91
1 parent 7c29320
commit 37e4a91
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 33 deletions.
diff --git a/modules/router-store/spec/router_selectors.spec.ts b/modules/router-store/spec/router_selectors.spec.ts
@@ -12,11 +12,15 @@ const mockData = {
       url: [],
       outlet: 'primary',
       routeConfig: null,
-      queryParams: {},
+      queryParams: {
+        ref: 'ngrx.io',
+      },
       queryParamMap: {
-        params: {},
+        params: {
+          ref: 'ngrx.io',
+        },
       },
-      fragment: null,
+      fragment: 'test-fragment',
       firstChild: {
         params: {},
         paramMap: {
@@ -34,10 +38,12 @@ const mockData = {
           path: 'login',
         },
         queryParams: {
-          id: 3,
+          ref: 'ngrx.io',
         },
         queryParamMap: {
-          params: {},
+          params: {
+            ref: 'ngrx.io',
+          },
         },
         firstChild: {
           params: {
@@ -72,7 +78,7 @@ const mockData = {
           fragment: 'test-fragment',
           children: [],
         },
-        fragment: null,
+        fragment: 'test-fragment',
         children: [],
       },
       children: [
@@ -92,10 +98,15 @@ const mockData = {
           routeConfig: {
             path: 'login',
           },
-          queryParams: {},
+          queryParams: {
+            ref: 'ngrx.io',
+          },
           queryParamMap: {
-            params: {},
+            params: {
+              ref: 'ngrx.io',
+            },
           },
+          fragment: 'test-fragment',
           children: [],
         },
       ],
@@ -104,6 +115,7 @@ const mockData = {
   },
   navigationId: 1,
 };
+
 describe('Router State Selectors', () => {
   describe('Composed Selectors', () => {
     interface State {
@@ -145,25 +157,19 @@ describe('Router State Selectors', () => {
     it('should create a selector for selecting the fragment', () => {
       const result = selectors.selectFragment(state);
 
-      expect(result).toEqual(
-        state.router.state.root.firstChild.firstChild.fragment
-      );
+      expect(result).toEqual(state.router.state.root.fragment);
     });
 
     it('should create a selector for selecting the query params', () => {
       const result = selectors.selectQueryParams(state);
 
-      expect(result).toEqual(
-        state.router.state.root.firstChild.firstChild.queryParams
-      );
+      expect(result).toEqual(state.router.state.root.queryParams);
     });
 
     it('should create a selector for selecting a specific query param', () => {
       const result = selectors.selectQueryParam('ref')(state);
 
-      expect(result).toEqual(
-        state.router.state.root.firstChild.firstChild.queryParams.ref
-      );
+      expect(result).toEqual(state.router.state.root.queryParams.ref);
     });
 
     it('should create a selector for selecting the route params', () => {

diff --git a/modules/router-store/src/router_selectors.ts b/modules/router-store/src/router_selectors.ts
@@ -2,35 +2,33 @@ import { createSelector } from '@ngrx/store';
 import { RouterStateSelectors } from './models';
 import { RouterReducerState } from './reducer';
 
-export function getSelectors<V>(
-  selectState: (state: V) => RouterReducerState<any>
-): RouterStateSelectors<V>;
 export function getSelectors<V>(
   selectState: (state: V) => RouterReducerState<any>
 ): RouterStateSelectors<V> {
   const selectRouterState = createSelector(
     selectState,
     (router) => router && router.state
   );
-  const selectCurrentRoute = createSelector(
+  const selectRootRoute = createSelector(
     selectRouterState,
-    (routerState) => {
-      if (!routerState) {
-        return undefined;
-      }
-      let route = routerState.root;
-      while (route.firstChild) {
-        route = route.firstChild;
-      }
-      return route;
-    }
+    (routerState) => routerState && routerState.root
   );
+  const selectCurrentRoute = createSelector(selectRootRoute, (rootRoute) => {
+    if (!rootRoute) {
+      return undefined;
+    }
+    let route = rootRoute;
+    while (route.firstChild) {
+      route = route.firstChild;
+    }
+    return route;
+  });
   const selectFragment = createSelector(
-    selectCurrentRoute,
+    selectRootRoute,
     (route) => route && route.fragment
   );
   const selectQueryParams = createSelector(
-    selectCurrentRoute,
+    selectRootRoute,
     (route) => route && route.queryParams
   );
   const selectQueryParam = (param: string) =>

diff --git a/projects/ngrx.io/content/guide/migration/v11.md b/projects/ngrx.io/content/guide/migration/v11.md
@@ -58,3 +58,63 @@ The new method name `setAll` describes the intention better.
 ```ts
 adapter.setAll(action.entities, state);
 ```
+
+### @ngrx/router-store
+
+#### Optimized `selectQueryParams`, `selectQueryParam` and `selectFragment` selectors
+
+They select query parameters/fragment from the root router state node instead of the last router state node.
+
+BEFORE:
+
+```ts
+const queryParams$ = this.store.select(selectQueryParams);
+const fragment$ = this.store.select(selectFragment);
+
+/*
+router state:
+{
+  root: {
+    queryParams: {
+      search: 'foo',
+    },
+    fragment: 'bar',
+    firstChild: {
+      queryParams: {
+        search: 'foo', 👈 query parameters are selected from here
+      },
+      fragment: 'bar', 👈 fragment is selected from here
+      firstChild: undefined,
+    },
+  },
+  url: '/books?search=foo#bar',
+}
+*/
+```
+
+AFTER:
+
+```ts
+const queryParams$ = this.store.select(selectQueryParams);
+const fragment$ = this.store.select(selectFragment);
+
+/*
+router state:
+{
+  root: {
+    queryParams: {
+      search: 'foo', 👈 query parameters are selected from here
+    },
+    fragment: 'bar', 👈 fragment is selected from here
+    firstChild: {
+      queryParams: {
+        search: 'foo',
+      },
+      fragment: 'bar',
+      firstChild: undefined,
+    },
+  },
+  url: '/books?search=foo#bar',
+}
+*/
+```