From a08ea3f6433b842b0e342509e5ddd53907a863b0 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 9 Mar 2022 11:48:19 -0800 Subject: [PATCH] refactor(router): Make a few adjustments to createUrlTree for clarity (#45306) * `tree` function now accepts the old root rather than the old `UrlTree`. The `urlTree` argument was only used to get the `root`. This change makes it more clear what that pararmeter is used for and what's actually being used * Move the `oldRoot` (previously `urlTree`) to be the first argument of `tree`. This change now mirrors the argument order for `replaceSegment` and can be read from left to right more easily "in this root, replace this old segment group with this new segment group". * Extract `newRoot` to a variable. This just makes it more clear what's going on at the end rather than combining a bunch of operations into one. These changes are being made so that hopefully a future refactor can be done which does not rely on the `urlTree` argument at all in the `createUrlTree` function. These refactorings will make it easier to see 1:1 functionlity in these various places. PR Close #45306 --- packages/router/src/create_url_tree.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/router/src/create_url_tree.ts b/packages/router/src/create_url_tree.ts index d79a06eba79..a074bce1e11 100644 --- a/packages/router/src/create_url_tree.ts +++ b/packages/router/src/create_url_tree.ts @@ -15,13 +15,13 @@ export function createUrlTree( route: ActivatedRoute, urlTree: UrlTree, commands: any[], queryParams: Params|null, fragment: string|null): UrlTree { if (commands.length === 0) { - return tree(urlTree.root, urlTree.root, urlTree, queryParams, fragment); + return tree(urlTree.root, urlTree.root, urlTree.root, queryParams, fragment); } const nav = computeNavigation(commands); if (nav.toRoot()) { - return tree(urlTree.root, new UrlSegmentGroup([], {}), urlTree, queryParams, fragment); + return tree(urlTree.root, urlTree.root, new UrlSegmentGroup([], {}), queryParams, fragment); } const startingPosition = findStartingPosition(nav, urlTree, route); @@ -30,7 +30,7 @@ export function createUrlTree( updateSegmentGroupChildren( startingPosition.segmentGroup, startingPosition.index, nav.commands) : updateSegmentGroup(startingPosition.segmentGroup, startingPosition.index, nav.commands); - return tree(startingPosition.segmentGroup, segmentGroup, urlTree, queryParams, fragment); + return tree(urlTree.root, startingPosition.segmentGroup, segmentGroup, queryParams, fragment); } function isMatrixParams(command: any): boolean { @@ -46,7 +46,7 @@ function isCommandWithOutlets(command: any): command is {outlets: {[key: string] } function tree( - oldSegmentGroup: UrlSegmentGroup, newSegmentGroup: UrlSegmentGroup, urlTree: UrlTree, + oldRoot: UrlSegmentGroup, oldSegmentGroup: UrlSegmentGroup, newSegmentGroup: UrlSegmentGroup, queryParams: Params|null, fragment: string|null): UrlTree { let qp: any = {}; if (queryParams) { @@ -55,11 +55,12 @@ function tree( }); } - if (urlTree.root === oldSegmentGroup) { + if (oldRoot === oldSegmentGroup) { return new UrlTree(newSegmentGroup, qp, fragment); } - return new UrlTree(replaceSegment(urlTree.root, oldSegmentGroup, newSegmentGroup), qp, fragment); + const newRoot = replaceSegment(oldRoot, oldSegmentGroup, newSegmentGroup); + return new UrlTree(newRoot, qp, fragment); } function replaceSegment(