From b75d2d8cec1235343624c23263c60f9115069cb8 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 29 Aug 2023 12:26:09 -0700 Subject: [PATCH] fix(router): Ensure title observable gets latest values 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 --- packages/router/src/operators/resolve_data.ts | 6 +--- packages/router/src/utils/collection.ts | 17 +++++++--- packages/router/test/BUILD.bazel | 1 + .../router/test/page_title_strategy_spec.ts | 33 ++++++++++++++++--- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/packages/router/src/operators/resolve_data.ts b/packages/router/src/operators/resolve_data.ts index d34a2f20951ca1..fefc7729f82a67 100644 --- 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 { - return [...Object.keys(obj), ...Object.getOwnPropertySymbols(obj)]; -} - function getResolver( injectionToken: ProviderToken|Function, futureARS: ActivatedRouteSnapshot, futureRSS: RouterStateSnapshot, injector: EnvironmentInjector): Observable { diff --git a/packages/router/src/utils/collection.ts b/packages/router/src/utils/collection.ts index 75a3957e629d8a..16affceb5321e5 100644 --- a/packages/router/src/utils/collection.ts +++ b/packages/router/src/utils/collection.ts @@ -9,7 +9,7 @@ import {ɵisPromise as isPromise} from '@angular/core'; import {from, isObservable, Observable, of} from 'rxjs'; -import {Params} from '../shared'; +import {Data} from '../models'; export function shallowEqualArrays(a: any[], b: any[]): boolean { if (a.length !== b.length) return false; @@ -19,15 +19,15 @@ export function shallowEqualArrays(a: any[], b: any[]): boolean { return true; } -export function shallowEqual(a: Params, b: Params): boolean { +export function shallowEqual(a: Data, b: Data): 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 +37,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 { + 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 index de2a978bba8d07..90df4e9693cbc8 100644 --- 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 index 4e3db3cadcefe8..69ad574df03edb 100644 --- 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 = 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', () => {