Skip to content

Commit

Permalink
fix(router): Ensure title observable gets latest values (angular#51561)
Browse files Browse the repository at this point in the history
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 angular#51401

PR Close angular#51561
  • Loading branch information
atscott authored and LayZeeDK committed Sep 20, 2023
1 parent 098c79e commit 24d7f17
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 15 deletions.
3 changes: 3 additions & 0 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -1283,6 +1283,9 @@
{
"name": "getData"
},
{
"name": "getDataKeys"
},
{
"name": "getDeclarationTNode"
},
Expand Down
6 changes: 1 addition & 5 deletions packages/router/src/operators/resolve_data.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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> {
Expand Down
18 changes: 12 additions & 6 deletions packages/router/src/utils/collection.ts
Expand Up @@ -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) {
Expand All @@ -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])) {
Expand All @@ -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.
*/
Expand Down
1 change: 1 addition & 0 deletions packages/router/test/BUILD.bazel
Expand Up @@ -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",
Expand Down
33 changes: 29 additions & 4 deletions packages/router/test/page_title_strategy_spec.ts
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 24d7f17

Please sign in to comment.