fix(router): Ensure title observable gets latest values (#51561)

atscott · thePunderWoman · commit f464e39364da · 2023-08-31T16:42:44.000Z
The data `Observable` is not updated unless there have been changes to the object. The current diffing does not look at `symbol` keys of the object but the `title` property is stored as a private `symbol`. This commit updates the object diffing to include symbols. fixes #51401 PR Close #51561
diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json
@@ -1265,6 +1265,9 @@
   {
     "name": "getData"
   },
+  {
+    "name": "getDataKeys"
+  },
   {
     "name": "getDeclarationTNode"
   },
diff --git a/packages/router/src/operators/resolve_data.ts b/packages/router/src/operators/resolve_data.ts
@@ -14,7 +14,7 @@ import {ResolveData, Route} from '../models';
 import {NavigationTransition} from '../navigation_transition';
 import {ActivatedRouteSnapshot, inheritedParamsDataResolve, RouterStateSnapshot} from '../router_state';
 import {RouteTitleKey} from '../shared';
-import {wrapIntoObservable} from '../utils/collection';
+import {getDataKeys, wrapIntoObservable} from '../utils/collection';
 import {getClosestRouteInjector} from '../utils/config';
 import {getTokenOrFunctionIdentity} from '../utils/preactivation';
 import {isEmptyError} from '../utils/type_guards';
@@ -79,10 +79,6 @@ function resolveNode(
   );
 }
 
-function getDataKeys(obj: Object): Array<string|symbol> {
-  return [...Object.keys(obj), ...Object.getOwnPropertySymbols(obj)];
-}
-
 function getResolver(
     injectionToken: ProviderToken<any>|Function, futureARS: ActivatedRouteSnapshot,
     futureRSS: RouterStateSnapshot, injector: EnvironmentInjector): Observable<any> {
diff --git a/packages/router/src/utils/collection.ts b/packages/router/src/utils/collection.ts
@@ -9,8 +9,6 @@
 import {ɵisPromise as isPromise} from '@angular/core';
 import {from, isObservable, Observable, of} from 'rxjs';
 
-import {Params} from '../shared';
-
 export function shallowEqualArrays(a: any[], b: any[]): boolean {
   if (a.length !== b.length) return false;
   for (let i = 0; i < a.length; ++i) {
@@ -19,15 +17,16 @@ export function shallowEqualArrays(a: any[], b: any[]): boolean {
   return true;
 }
 
-export function shallowEqual(a: Params, b: Params): boolean {
+export function shallowEqual(
+    a: {[key: string|symbol]: any}, b: {[key: string|symbol]: any}): boolean {
   // While `undefined` should never be possible, it would sometimes be the case in IE 11
   // and pre-chromium Edge. The check below accounts for this edge case.
-  const k1 = a ? Object.keys(a) : undefined;
-  const k2 = b ? Object.keys(b) : undefined;
+  const k1 = a ? getDataKeys(a) : undefined;
+  const k2 = b ? getDataKeys(b) : undefined;
   if (!k1 || !k2 || k1.length != k2.length) {
     return false;
   }
-  let key: string;
+  let key: string|symbol;
   for (let i = 0; i < k1.length; i++) {
     key = k1[i];
     if (!equalArraysOrString(a[key], b[key])) {
@@ -37,6 +36,13 @@ export function shallowEqual(a: Params, b: Params): boolean {
   return true;
 }
 
+/**
+ * Gets the keys of an object, including `symbol` keys.
+ */
+export function getDataKeys(obj: Object): Array<string|symbol> {
+  return [...Object.keys(obj), ...Object.getOwnPropertySymbols(obj)];
+}
+
 /**
  * Test equality for arrays of strings or a string.
  */
diff --git a/packages/router/test/BUILD.bazel b/packages/router/test/BUILD.bazel
@@ -23,6 +23,7 @@ ts_library(
         "//packages/common",
         "//packages/common/testing",
         "//packages/core",
+        "//packages/core/rxjs-interop",
         "//packages/core/testing",
         "//packages/platform-browser",
         "//packages/platform-browser-dynamic",
diff --git a/packages/router/test/page_title_strategy_spec.ts b/packages/router/test/page_title_strategy_spec.ts
@@ -8,11 +8,11 @@
 
 import {DOCUMENT} from '@angular/common';
 import {provideLocationMocks} from '@angular/common/testing';
-import {Component, Inject, Injectable, NgModule} from '@angular/core';
+import {Component, inject, Inject, Injectable, NgModule} from '@angular/core';
+import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
 import {fakeAsync, TestBed, tick} from '@angular/core/testing';
-import {Router, RouterModule, RouterStateSnapshot, TitleStrategy, withRouterConfig} from '@angular/router';
-
-import {provideRouter} from '../src/provide_router';
+import {ActivatedRoute, provideRouter, ResolveFn, Router, RouterModule, RouterStateSnapshot, TitleStrategy, withRouterConfig} from '@angular/router';
+import {RouterTestingHarness} from '@angular/router/testing';
 
 describe('title strategy', () => {
   describe('DefaultTitleStrategy', () => {
@@ -114,6 +114,31 @@ describe('title strategy', () => {
       await router.navigateByUrl('home');
       expect(router.routerState.snapshot.root.firstChild!.title).toEqual('My Application');
     });
+
+    it('pushes updates through the title observable', async () => {
+      @Component({template: '', standalone: true})
+      class HomeCmp {
+        private readonly title$ = inject(ActivatedRoute).title.pipe(takeUntilDestroyed());
+        title?: string;
+
+        constructor() {
+          this.title$.subscribe(v => this.title = v);
+        }
+      }
+      const titleResolver: ResolveFn<string> = route => route.queryParams.id;
+      router.resetConfig([{
+        path: 'home',
+        title: titleResolver,
+        component: HomeCmp,
+        runGuardsAndResolvers: 'paramsOrQueryParamsChange'
+      }]);
+
+      const harness = await RouterTestingHarness.create();
+      const homeCmp = await harness.navigateByUrl('/home?id=1', HomeCmp);
+      expect(homeCmp.title).toEqual('1');
+      await harness.navigateByUrl('home?id=2');
+      expect(homeCmp.title).toEqual('2');
+    });
   });
 
   describe('custom strategies', () => {

Original file line number	Diff line number	Diff line change
`@@ -1265,6 +1265,9 @@`
`1265`	`1265`	`{`
`1266`	`1266`	`"name": "getData"`
`1267`	`1267`	`},`
	`1268`	`+ {`
	`1269`	`+ "name": "getDataKeys"`
	`1270`	`+ },`
`1268`	`1271`	`{`
`1269`	`1272`	`"name": "getDeclarationTNode"`
`1270`	`1273`	`},`