fix(router): don't mutate route configs (#22358)

Fixes #22203

PR Close #22358

packages/router/src/config.ts

@@ -457,3 +457,9 @@ function getFullPath(parentPath: string, currentRoute: Route): string {
     return `${parentPath}/${currentRoute.path}`;
   }
 }
+
+
+export function copyConfig(r: Route): Route {
+  const children = r.children && r.children.map(copyConfig);
+  return children ? {...r, children} : {...r};
+}

packages/router/src/router.ts

@@ -18,7 +18,7 @@ import {map} from 'rxjs/operator/map';
 import {mergeMap} from 'rxjs/operator/mergeMap';
 
 import {applyRedirects} from './apply_redirects';
-import {LoadedRouterConfig, QueryParamsHandling, Route, Routes, validateConfig} from './config';
+import {LoadedRouterConfig, QueryParamsHandling, Route, Routes, copyConfig, validateConfig} from './config';
 import {createRouterState} from './create_router_state';
 import {createUrlTree} from './create_url_tree';
 import {ActivationEnd, ChildActivationEnd, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, NavigationTrigger, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RoutesRecognized} from './events';
@@ -34,6 +34,7 @@ import {UrlSerializer, UrlTree, containsTree, createEmptyUrlTree} from './url_tr
 import {forEach} from './utils/collection';
 import {TreeNode, nodeChildrenAsMap} from './utils/tree';
 
+
 /**
  * @whatItDoes Represents the extra options used during navigation.
  *
@@ -342,7 +343,7 @@ export class Router {
    */
   resetConfig(config: Routes): void {
     validateConfig(config);
-    this.config = config;
+    this.config = config.map(copyConfig);
     this.navigated = false;
     this.lastSuccessfulId = -1;
   }

packages/router/src/router_config_loader.ts

@@ -12,7 +12,7 @@ import {fromPromise} from 'rxjs/observable/fromPromise';
 import {of } from 'rxjs/observable/of';
 import {map} from 'rxjs/operator/map';
 import {mergeMap} from 'rxjs/operator/mergeMap';
-import {LoadChildren, LoadedRouterConfig, Route} from './config';
+import {LoadChildren, LoadedRouterConfig, Route, copyConfig} from './config';
 import {flatten, wrapIntoObservable} from './utils/collection';
 
 /**
@@ -41,7 +41,7 @@ export class RouterConfigLoader {
 
       const module = factory.create(parentInjector);
 
-      return new LoadedRouterConfig(flatten(module.injector.get(ROUTES)), module);
+      return new LoadedRouterConfig(flatten(module.injector.get(ROUTES)).map(copyConfig), module);
     });
   }
 

packages/router/test/router.spec.ts

@@ -22,6 +22,38 @@ import {RouterTestingModule} from '../testing/src/router_testing_module';
 import {Logger, createActivatedRouteSnapshot, provideTokenLogger} from './helpers';
 
 describe('Router', () => {
+
+  describe('resetConfig', () => {
+    class TestComponent {}
+
+    beforeEach(() => { TestBed.configureTestingModule({imports: [RouterTestingModule]}); });
+
+    it('should copy config to avoid mutations of user-provided objects', () => {
+      const r: Router = TestBed.get(Router);
+      const configs = [{
+        path: 'a',
+        component: TestComponent,
+        children: [{path: 'b', component: TestComponent}, {path: 'c', component: TestComponent}]
+      }];
+      r.resetConfig(configs);
+
+      let rConfigs = r.config;
+
+      // routes array and shallow copy
+      expect(configs).not.toBe(rConfigs);
+      expect(configs[0]).not.toBe(rConfigs[0]);
+      expect(configs[0].path).toBe(rConfigs[0].path);
+      expect(configs[0].component).toBe(rConfigs[0].component);
+
+      // children should be new array and routes shallow copied
+      expect(configs[0].children).not.toBe(rConfigs[0].children);
+      expect(configs[0].children[0]).not.toBe(rConfigs[0].children ![0]);
+      expect(configs[0].children[0].path).toBe(rConfigs[0].children ![0].path);
+      expect(configs[0].children[1]).not.toBe(rConfigs[0].children ![1]);
+      expect(configs[0].children[1].path).toBe(rConfigs[0].children ![1].path);
+    });
+  });
+
   describe('resetRootComponentType', () => {
     class NewRootComponent {}
 

packages/router/test/router_preloader.spec.ts

@@ -210,4 +210,35 @@ describe('RouterPreloader', () => {
              expect((c[1] as any)._loadedConfig).toBeDefined();
            })));
   });
+
+  describe('should copy loaded configs', () => {
+    const configs = [{path: 'LoadedModule1', component: LazyLoadedCmp}];
+    @NgModule({declarations: [LazyLoadedCmp], imports: [RouterModule.forChild(configs)]})
+    class LoadedModule {
+    }
+
+    beforeEach(() => {
+      TestBed.configureTestingModule({
+        imports: [RouterTestingModule.withRoutes([{path: 'lazy1', loadChildren: 'expected'}])],
+        providers: [{provide: PreloadingStrategy, useExisting: PreloadAllModules}]
+      });
+    });
+
+    it('should work',
+       fakeAsync(inject(
+           [NgModuleFactoryLoader, RouterPreloader, Router],
+           (loader: SpyNgModuleFactoryLoader, preloader: RouterPreloader, router: Router) => {
+             loader.stubbedModules = {expected: LoadedModule};
+
+             preloader.preload().subscribe(() => {});
+
+             tick();
+
+             const c = router.config as{_loadedConfig: LoadedRouterConfig}[];
+             expect(c[0]._loadedConfig).toBeDefined();
+             expect(c[0]._loadedConfig !.routes).not.toBe(configs);
+             expect(c[0]._loadedConfig !.routes[0]).not.toBe(configs[0]);
+             expect(c[0]._loadedConfig !.routes[0].component).toBe(configs[0].component);
+           })));
+  });
 });