fixup! Create url tree without currentUrlTree

atscott · Oct 29, 2021 · Oct 29, 2021 · Oct 30, 2021 · Mar 8, 2022 · Mar 9, 2022
commit c629a06701913def3b26ef7e14894bc2dc867eb0
diff --git a/packages/router/src/create_url_tree.ts b/packages/router/src/create_url_tree.ts
@@ -20,41 +20,20 @@ export function createUrlTree(
   }
   const targetGroup = relativeTo;
   if (commands.length === 0) {
-    return tree(root, queryParams, fragment);
+    return tree(root, root, root, queryParams, fragment);
   }
 
   const nav = computeNavigation(commands);
 
   if (nav.toRoot()) {
-    return tree(new UrlSegmentGroup([], {}), queryParams, fragment);
+    return tree(root, root, new UrlSegmentGroup([], {}), queryParams, fragment);
   }
 
-  const position = findNewStartingPosition(nav, root, targetGroup)
+  const position = findStartingPosition(nav, root, targetGroup)
   const newSegmentGroup = position.processChildren ?
       updateSegmentGroupChildren(position.segmentGroup, position.index, nav.commands) :
       updateSegmentGroup(position.segmentGroup, position.index, nav.commands);
-  position.segmentGroup.children = newSegmentGroup.children;
-  position.segmentGroup.segments = newSegmentGroup.segments
-
-  return tree(root, queryParams, fragment);
-}
-
-function findNewStartingPosition(
-    nav: Navigation, root: UrlSegmentGroup, target: UrlSegmentGroup): Position {
-  if (nav.isAbsolute) {
-    return new Position(root, true, 0);
-  }
-
-  if (!target) {
-    return new Position(root, false, NaN);
-  }
-  if (target.parent === null) {
-    return new Position(target, true, 0);
-  }
-
-  const modifier = isMatrixParams(nav.commands[0]) ? 0 : 1;
-  const index = target.segments.length - 1 + modifier;
-  return createPositionApplyingDoubleDots(target, index, nav.numberOfDoubleDots);
+  return tree(root, position.segmentGroup, newSegmentGroup, queryParams, fragment);
 }
 
 function isMatrixParams(command: any): boolean {
@@ -70,16 +49,44 @@ function isCommandWithOutlets(command: any): command is {outlets: {[key: string]
 }
 
 function tree(
-    segmentGroup: UrlSegmentGroup, queryParams: Params|null, fragment: string|null): UrlTree {
+    oldRoot: UrlSegmentGroup, oldSegmentGroup: UrlSegmentGroup, newSegmentGroup: UrlSegmentGroup,
+    queryParams: Params|null, fragment: string|null): UrlTree {
   let qp: any = {};
   if (queryParams) {
     forEach(queryParams, (value: any, name: any) => {
       qp[name] = Array.isArray(value) ? value.map((v: any) => `${v}`) : `${value}`;
     });
   }
 
-  return createUrlTreeRedirects(
-      new UrlTree(squashSegmentGroup(segmentGroup), qp, fragment).root, qp, fragment);
+  // Needed because `replaceSegment` cannot replace the root segment. It expects the
+  // `oldSegmentGroup` to be some child of the root.
+  if (oldRoot === oldSegmentGroup) {
+    return new UrlTree(squashSegmentGroup(newSegmentGroup), qp, fragment);
+  }
+
+  const newRoot = squashSegmentGroup(replaceSegment(oldRoot, oldSegmentGroup, newSegmentGroup));
+  return createUrlTreeRedirects(new UrlTree(newRoot, qp, fragment).root, qp, fragment);
+}
+
+/**
+ * Replaces the `oldSegment` which is located in some child of the `current` with the `newSegment`.
+ * This also has the effect of creating new `UrlSegmentGroup` copies to update references. This
+ * shouldn't be necessary but the fallback logic for an invalid ActivatedRoute in the creation uses
+ * the Router's current url tree. If we don't create new segment groups, we end up modifying that
+ * value.
+ */
+function replaceSegment(
+    current: UrlSegmentGroup, oldSegment: UrlSegmentGroup,
+    newSegment: UrlSegmentGroup): UrlSegmentGroup {
+  const children: {[key: string]: UrlSegmentGroup} = {};
+  forEach(current.children, (c: UrlSegmentGroup, outletName: string) => {
+    if (c === oldSegment) {
+      children[outletName] = newSegment;
+    } else {
+      children[outletName] = replaceSegment(c, oldSegment, newSegment);
+    }
+  });
+  return new UrlSegmentGroup(current.segments, children);
 }
 
 class Navigation {
@@ -156,6 +163,23 @@ class Position {
   }
 }
 
+function findStartingPosition(
+    nav: Navigation, root: UrlSegmentGroup, target: UrlSegmentGroup): Position {
+  if (nav.isAbsolute) {
+    return new Position(root, true, 0);
+  }
+
+  if (!target) {
+    return new Position(root, false, NaN);
+  }
+  if (target.parent === null) {
+    return new Position(target, true, 0);
+  }
+
+  const modifier = isMatrixParams(nav.commands[0]) ? 0 : 1;
+  const index = target.segments.length - 1 + modifier;
+  return createPositionApplyingDoubleDots(target, index, nav.numberOfDoubleDots);
+}
 
 function createPositionApplyingDoubleDots(
     group: UrlSegmentGroup, index: number, numberOfDoubleDots: number): Position {

diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -1225,9 +1225,20 @@ export class Router {
     try {
       relativeToUrlSegmentGroup = createSegmentGroupFromRoute(a.snapshot);
     } catch (e: unknown) {
-      NG_DEV_MODE &&
-          console.warn(`${
-              a.snapshot} has an invalid structure. This is likely due to an incomplete mock in tests.`);
+      if (NG_DEV_MODE) {
+        console.warn(`${
+            a.snapshot} has an invalid structure. This is likely due to an incomplete mock in tests.`);
+        if (typeof commands[0] !== 'string' || !commands[0].startsWith('/')) {
+          // This is strictly for backwards compatibility with tests that create
+          // invalid `ActivatedRoute` mocks. Navigations that were absolute
+          // would still work because they wouldn't attempt to match the
+          // segments in the `ActivatedRoute` to the `currentUrlTree` but
+          // instead just replace the root segment with the navigation result.
+          // Non-absolute navigations would fail to apply the commands because
+          // the logic could not find the segment to replace.
+          commands = [];
+        }
+      }
     }
     return createUrlTree(
         relativeToUrlSegmentGroup ?? this.currentUrlTree.root, commands, q, f ?? null);