fix(router): Ensure all outlets are used when commands has a prefix

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -1376,9 +1376,6 @@
   {
     "name": "getParentInjectorView"
   },
-  {
-    "name": "getPath"
-  },
   {
     "name": "getPathIndexShift"
   },
@@ -1490,6 +1487,9 @@
   {
     "name": "isArrayLike"
   },
+  {
+    "name": "isCommandWithOutlets"
+  },
   {
     "name": "isComponentDef"
   },

packages/router/src/create_url_tree.ts

@@ -37,6 +37,14 @@ function isMatrixParams(command: any): boolean {
   return typeof command === 'object' && command != null && !command.outlets && !command.segmentPath;
 }
 
+/**
+ * Determines if a given command has an `outlets` map. When we encounter a command
+ * with an outlets k/v map, we need to apply each outlet individually to the existing segment.
+ */
+function isCommandWithOutlets(command: any): command is {outlets: {[key: string]: any}} {
+  return typeof command === 'object' && command != null && command.outlets;
+}
+
 function tree(
     oldSegmentGroup: UrlSegmentGroup, newSegmentGroup: UrlSegmentGroup, urlTree: UrlTree,
     queryParams: Params, fragment: string): UrlTree {
@@ -75,7 +83,7 @@ class Navigation {
       throw new Error('Root segment cannot have matrix parameters');
     }
 
-    const cmdWithOutlet = commands.find(c => typeof c === 'object' && c != null && c.outlets);
+    const cmdWithOutlet = commands.find(isCommandWithOutlets);
     if (cmdWithOutlet && cmdWithOutlet !== last(commands)) {
       throw new Error('{outlets:{}} has to be the last command');
     }
@@ -178,15 +186,8 @@ function createPositionApplyingDoubleDots(
   return new Position(g, false, ci - dd);
 }
 
-function getPath(command: any): any {
-  if (typeof command === 'object' && command != null && command.outlets) {
-    return command.outlets[PRIMARY_OUTLET];
-  }
-  return `${command}`;
-}
-
 function getOutlets(commands: any[]): {[k: string]: any[]} {
-  if (typeof commands[0] === 'object' && commands[0] !== null && commands[0].outlets) {
+  if (isCommandWithOutlets(commands[0])) {
     return commands[0].outlets;
   }
 
@@ -251,7 +252,14 @@ function prefixedWith(segmentGroup: UrlSegmentGroup, startIndex: number, command
   while (currentPathIndex < segmentGroup.segments.length) {
     if (currentCommandIndex >= commands.length) return noMatch;
     const path = segmentGroup.segments[currentPathIndex];
-    const curr = getPath(commands[currentCommandIndex]);
+    const command = commands[currentCommandIndex];
+    // Do not try to consume command as part of the prefixing if it has outlets because it can
+    // contain outlets other than the one being processed. Consuming the outlets command would
+    // result in other outlets being ignored.
+    if (isCommandWithOutlets(command)) {
+      break;
+    }
+    const curr = `${command}`;
     const next =
         currentCommandIndex < commands.length - 1 ? commands[currentCommandIndex + 1] : null;
 
@@ -276,9 +284,9 @@ function createNewSegmentGroup(
 
   let i = 0;
   while (i < commands.length) {
-    if (typeof commands[i] === 'object' && commands[i] !== null &&
-        commands[i].outlets !== undefined) {
-      const children = createNewSegmentChildren(commands[i].outlets);
+    const command = commands[i];
+    if (isCommandWithOutlets(command)) {
+      const children = createNewSegmentChildren(command.outlets);
       return new UrlSegmentGroup(paths, children);
     }
 
@@ -290,7 +298,7 @@ function createNewSegmentGroup(
       continue;
     }
 
-    const curr = getPath(commands[i]);
+    const curr = isCommandWithOutlets(command) ? command.outlets[PRIMARY_OUTLET] : `${command}`;
     const next = (i < commands.length - 1) ? commands[i + 1] : null;
     if (curr && next && isMatrixParams(next)) {
       paths.push(new UrlSegment(curr, stringify(next)));

packages/router/test/create_url_tree.spec.ts

@@ -112,6 +112,100 @@ describe('createUrlTree', () => {
     expect(serializer.serialize(t)).toEqual('/a/(b//right:d/11/e)');
   });
 
+  describe('', () => {
+    /**
+     * In this group of scenarios, imagine a config like:
+     * {
+     *   path: 'parent',
+     *   children: [
+     *     {
+     *       path: 'child',
+     *       component: AnyCmp
+     *     },
+     *     {
+     *       path: 'popup',
+     *       outlet: 'secondary',
+     *       component: AnyCmp
+     *     }
+     *   ]
+     * },
+     * {
+     *   path: 'other',
+     *   component: AnyCmp
+     * },
+     * {
+     *   path: 'rootPopup',
+     *   outlet: 'rootSecondary',
+     * }
+     */
+
+    it('should support removing secondary outlet with prefix', () => {
+      const p = serializer.parse('/parent/(child//secondary:popup)');
+      const t = createRoot(p, ['parent', {outlets: {secondary: null}}]);
+      // - Segment index 0:
+      //   * match and keep existing 'parent'
+      // - Segment index 1:
+      //   * 'secondary' outlet cleared with `null`
+      //   * 'primary' outlet not provided in the commands list, so the existing value is kept
+      expect(serializer.serialize(t)).toEqual('/parent/child');
+    });
+
+    it('should support updating secondary and primary outlets with prefix', () => {
+      const p = serializer.parse('/parent/child');
+      const t = createRoot(p, ['parent', {outlets: {primary: 'child', secondary: 'popup'}}]);
+      expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)');
+    });
+
+    it('should support updating two outlets at the same time relative to non-root segment', () => {
+      const p = serializer.parse('/parent/child');
+      const t = create(
+          p.root.children[PRIMARY_OUTLET], 0 /* relativeTo: 'parent' */, p,
+          [{outlets: {primary: 'child', secondary: 'popup'}}]);
+      expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)');
+    });
+
+    it('should support adding multiple outlets with prefix', () => {
+      const p = serializer.parse('');
+      const t = createRoot(p, ['parent', {outlets: {primary: 'child', secondary: 'popup'}}]);
+      expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)');
+    });
+
+    it('should support updating clearing primary and secondary with prefix', () => {
+      const p = serializer.parse('/parent/(child//secondary:popup)');
+      const t = createRoot(p, ['other']);
+      // Because we navigate away from the 'parent' route, the children of that route are cleared
+      // because they are note valid for the 'other' path.
+      expect(serializer.serialize(t)).toEqual('/other');
+    });
+
+    it('should not clear secondary outlet when at root and prefix is used', () => {
+      const p = serializer.parse('/other(rootSecondary:rootPopup)');
+      const t = createRoot(p, ['parent', {outlets: {primary: 'child', rootSecondary: null}}]);
+      // We prefixed the navigation with 'parent' so we cannot clear the "rootSecondary" outlet
+      // because once the outlets object is consumed, traversal is beyond the root segment.
+      expect(serializer.serialize(t)).toEqual('/parent/child(rootSecondary:rootPopup)');
+    });
+
+    it('should not clear non-root secondary outlet when command is targeting root', () => {
+      const p = serializer.parse('/parent/(child//secondary:popup)');
+      const t = createRoot(p, [{outlets: {secondary: null}}]);
+      // The start segment index for the command is at 0, but the outlet lives at index 1
+      // so we cannot clear the outlet from processing segment index 0.
+      expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)');
+    });
+
+    it('can clear an auxiliary outlet at the correct segment level', () => {
+      const p = serializer.parse('/parent/(child//secondary:popup)(rootSecondary:rootPopup)');
+      //                                       ^^^^^^^^^^^^^^^^^^^^^^
+      // The parens here show that 'child' and 'secondary:popup' appear at the same 'level' in the
+      // config, i.e. are part of the same children list. You can also imagine an implicit paren
+      // group around the whole URL to visualize how 'parent' and 'rootSecondary:rootPopup' are also
+      // defined at the same level.
+      const t = createRoot(p, ['parent', {outlets: {primary: 'child', secondary: null}}]);
+      expect(serializer.serialize(t)).toEqual('/parent/child(rootSecondary:rootPopup)');
+    });
+  });
+
   it('should throw when outlets is not the last command', () => {
     const p = serializer.parse('/a');
     expect(() => createRoot(p, ['a', {outlets: {right: ['c']}}, 'c']))