diff --git a/adev/src/app/app-scroller.ts b/adev/src/app/app-scroller.ts index a7b8eb8cf81..f746899fe2e 100644 --- a/adev/src/app/app-scroller.ts +++ b/adev/src/app/app-scroller.ts @@ -43,7 +43,7 @@ export class AppScroller { this._lastScrollEvent = e; }), filter(() => { - const info = this.router.lastSuccessfulNavigation?.extras.info as Record< + const info = this.router.lastSuccessfulNavigation()?.extras.info as Record< 'disableScrolling', boolean >; diff --git a/goldens/public-api/router/index.api.md b/goldens/public-api/router/index.api.md index 7c3a0de5fd4..9a6ca82ff9d 100644 --- a/goldens/public-api/router/index.api.md +++ b/goldens/public-api/router/index.api.md @@ -709,7 +709,7 @@ export class Router { // (undocumented) config: Routes; createUrlTree(commands: readonly any[], navigationExtras?: UrlCreationOptions): UrlTree; - readonly currentNavigation: i0.Signal; + readonly currentNavigation: Signal; dispose(): void; get events(): Observable; // @deprecated @@ -718,7 +718,7 @@ export class Router { // @deprecated isActive(url: string | UrlTree, exact: boolean): boolean; isActive(url: string | UrlTree, matchOptions: IsActiveMatchOptions): boolean; - get lastSuccessfulNavigation(): Navigation | null; + get lastSuccessfulNavigation(): Signal; navigate(commands: readonly any[], extras?: NavigationExtras): Promise; navigateByUrl(url: string | UrlTree, extras?: NavigationBehaviorOptions): Promise; navigated: boolean; diff --git a/goldens/public-api/router/testing/index.api.md b/goldens/public-api/router/testing/index.api.md index 7a08536ca43..0c27b34f68f 100644 --- a/goldens/public-api/router/testing/index.api.md +++ b/goldens/public-api/router/testing/index.api.md @@ -25,6 +25,7 @@ import { Provider } from '@angular/core'; import { ProviderToken } from '@angular/core'; import { QueryList } from '@angular/core'; import { Renderer2 } from '@angular/core'; +import { Signal } from '@angular/core'; import { SimpleChanges } from '@angular/core'; import { Type } from '@angular/core'; import { WritableSignal } from '@angular/core'; diff --git a/integration/platform-server-hydration/e2e/src/app.e2e-spec.ts b/integration/platform-server-hydration/e2e/src/app.e2e-spec.ts index 11faa0dd802..47ba1571853 100644 --- a/integration/platform-server-hydration/e2e/src/app.e2e-spec.ts +++ b/integration/platform-server-hydration/e2e/src/app.e2e-spec.ts @@ -15,7 +15,13 @@ describe('App E2E Tests', () => { await verifyNoBrowserErrors(); }); - it('should reply click event', async () => { + // TODO: renable this test once the @angular/ssr has been update + // Context: https://github.com/angular/angular/pull/63057 + // SSR relies on lastSuccessfulNavigation which went through a breaking change. + // 1. FW needs to be released with the breaking change. + // 2. @angular/ssr needs to be updated to use the new API & released + // 3. We need to update the @angular/ssr to the said release. + xit('should reply click event', async () => { const divElement = element(by.css('#divElement')); expect(await divElement.getText()).toContain('click not triggered'); diff --git a/packages/core/schematics/BUILD.bazel b/packages/core/schematics/BUILD.bazel index 1eb9f287697..54574536af6 100644 --- a/packages/core/schematics/BUILD.bazel +++ b/packages/core/schematics/BUILD.bazel @@ -113,6 +113,10 @@ bundle_entrypoints = [ "router-current-navigation", "packages/core/schematics/migrations/router-current-navigation/index.js", ], + [ + "router-last-successful-navigation", + "packages/core/schematics/migrations/router-last-successful-navigation/index.js", + ], ] rollup.rollup( @@ -128,6 +132,7 @@ rollup.rollup( "//packages/core/schematics/migrations/document-core", "//packages/core/schematics/migrations/inject-flags", "//packages/core/schematics/migrations/router-current-navigation", + "//packages/core/schematics/migrations/router-last-successful-navigation", "//packages/core/schematics/migrations/test-bed-get", "//packages/core/schematics/ng-generate/cleanup-unused-imports", "//packages/core/schematics/ng-generate/inject-migration", diff --git a/packages/core/schematics/migrations.json b/packages/core/schematics/migrations.json index e3561604437..e321042d76d 100644 --- a/packages/core/schematics/migrations.json +++ b/packages/core/schematics/migrations.json @@ -26,6 +26,11 @@ "description": "Replaces usages of the deprecated Router.getCurrentNavigation method with the Router.currentNavigation signal", "factory": "./bundles/router-current-navigation.cjs#migrate", "optional": true + }, + "router-last-successful-navigation": { + "version": "21.0.0", + "description": "Ensures that the Router.lastSuccessfulNavigation signal is now invoked", + "factory": "./bundles/router-last-successful-navigation.cjs#migrate" } } } diff --git a/packages/core/schematics/migrations/router-last-successful-navigation/BUILD.bazel b/packages/core/schematics/migrations/router-last-successful-navigation/BUILD.bazel new file mode 100644 index 00000000000..3e6a719e4aa --- /dev/null +++ b/packages/core/schematics/migrations/router-last-successful-navigation/BUILD.bazel @@ -0,0 +1,23 @@ +load("//tools:defaults2.bzl", "ts_project") + +package( + default_visibility = [ + "//packages/core/schematics:__pkg__", + "//packages/core/schematics/test:__pkg__", + ], +) + +ts_project( + name = "router-last-successful-navigation", + srcs = glob(["**/*.ts"]), + deps = [ + "//:node_modules/@angular-devkit/schematics", + "//:node_modules/@types/node", + "//:node_modules/typescript", + "//packages/compiler-cli/private", + "//packages/compiler-cli/src/ngtsc/file_system", + "//packages/core/schematics/utils", + "//packages/core/schematics/utils/tsurge", + "//packages/core/schematics/utils/tsurge/helpers/angular_devkit", + ], +) diff --git a/packages/core/schematics/migrations/router-last-successful-navigation/README.md b/packages/core/schematics/migrations/router-last-successful-navigation/README.md new file mode 100644 index 00000000000..0960420bc0a --- /dev/null +++ b/packages/core/schematics/migrations/router-last-successful-navigation/README.md @@ -0,0 +1,28 @@ +## Invoke the `Router.lastSuccessfulNavigation` signal migration +`Router.lastSuccessfulNavigation` is now a signal, this migration ensures `Router.getCurrentNavigation` is invoked: + +### Before +```typescript +import { Router } from '@angular/router'; + +export class MyService { + router = inject(Router); + + someMethod() { + const navigation = this.router.lastSuccessfulNavigation; + } +} +``` + +### After +```typescript +import { Router } from '@angular/router'; + +export class MyService { + router = inject(Router); + + someMethod() { + const navigation = this.router.lastSuccessfulNavigation(); + } +} +``` diff --git a/packages/core/schematics/migrations/router-last-successful-navigation/index.ts b/packages/core/schematics/migrations/router-last-successful-navigation/index.ts new file mode 100644 index 00000000000..376e6757b37 --- /dev/null +++ b/packages/core/schematics/migrations/router-last-successful-navigation/index.ts @@ -0,0 +1,20 @@ +/*! + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {Rule} from '@angular-devkit/schematics'; +import {RouterLastSuccessfulNavigationMigration} from './router_current_navigation_migration'; +import {runMigrationInDevkit} from '../../utils/tsurge/helpers/angular_devkit'; + +export function migrate(): Rule { + return async (tree) => { + await runMigrationInDevkit({ + tree, + getMigration: () => new RouterLastSuccessfulNavigationMigration(), + }); + }; +} diff --git a/packages/core/schematics/migrations/router-last-successful-navigation/router_current_navigation_migration.ts b/packages/core/schematics/migrations/router-last-successful-navigation/router_current_navigation_migration.ts new file mode 100644 index 00000000000..5188980675c --- /dev/null +++ b/packages/core/schematics/migrations/router-last-successful-navigation/router_current_navigation_migration.ts @@ -0,0 +1,139 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import ts from 'typescript'; +import { + confirmAsSerializable, + ProgramInfo, + ProjectFile, + projectFile, + Replacement, + Serializable, + TextUpdate, + TsurgeFunnelMigration, +} from '../../utils/tsurge'; +import {getImportSpecifier} from '../../utils/typescript/imports'; +import {isReferenceToImport} from '../../utils/typescript/symbol'; + +export interface CompilationUnitData { + locations: Location[]; +} + +/** Information about the `getCurrentNavigation` identifier in `Router.getCurrentNavigation`. */ +interface Location { + /** File in which the expression is defined. */ + file: ProjectFile; + + /** Start of the `getCurrentNavigation` identifier. */ + position: number; +} + +/** Name of the method being replaced. */ +const METHOD_NAME = 'lastSuccessfulNavigation'; + +/** Migration that replaces `Router.lastSuccessfulNavigation` usages with `Router.lastSuccessfulNavigation()`. */ +export class RouterLastSuccessfulNavigationMigration extends TsurgeFunnelMigration< + CompilationUnitData, + CompilationUnitData +> { + override async analyze(info: ProgramInfo): Promise> { + const locations: Location[] = []; + + for (const sourceFile of info.sourceFiles) { + const routerSpecifier = getImportSpecifier(sourceFile, '@angular/router', 'Router'); + + if (routerSpecifier === null) { + continue; + } + + const typeChecker = info.program.getTypeChecker(); + sourceFile.forEachChild(function walk(node) { + if ( + ts.isPropertyAccessExpression(node) && + node.name.text === METHOD_NAME && + isRouterType(typeChecker, node.expression, routerSpecifier) + ) { + locations.push({file: projectFile(sourceFile, info), position: node.name.getStart()}); + } else { + node.forEachChild(walk); + } + }); + } + + return confirmAsSerializable({locations}); + } + + override async migrate(globalData: CompilationUnitData) { + const replacements = globalData.locations.map(({file, position}) => { + return new Replacement( + file, + new TextUpdate({ + position: position, + end: position + METHOD_NAME.length, + toInsert: 'lastSuccessfulNavigation()', + }), + ); + }); + + return confirmAsSerializable({replacements}); + } + + override async combine( + unitA: CompilationUnitData, + unitB: CompilationUnitData, + ): Promise> { + const seen = new Set(); + const locations: Location[] = []; + const combined = [...unitA.locations, ...unitB.locations]; + + for (const location of combined) { + const key = `${location.file.id}#${location.position}`; + if (!seen.has(key)) { + seen.add(key); + locations.push(location); + } + } + + return confirmAsSerializable({locations}); + } + + override async globalMeta( + combinedData: CompilationUnitData, + ): Promise> { + return confirmAsSerializable(combinedData); + } + + override async stats() { + return confirmAsSerializable({}); + } +} + +/** + * Checks if the given symbol represents a Router type. + */ +function isRouterType( + typeChecker: ts.TypeChecker, + expression: ts.Expression, + routerSpecifier: ts.ImportSpecifier, +): boolean { + const expressionType = typeChecker.getTypeAtLocation(expression); + const expressionSymbol = expressionType.getSymbol(); + if (!expressionSymbol) { + return false; + } + + const declarations = expressionSymbol.getDeclarations() ?? []; + + for (const declaration of declarations) { + if (isReferenceToImport(typeChecker, declaration, routerSpecifier)) { + return true; + } + } + + return declarations.some((decl) => decl === routerSpecifier); +} diff --git a/packages/core/schematics/test/router_last_successful_navigation_spec.ts b/packages/core/schematics/test/router_last_successful_navigation_spec.ts new file mode 100644 index 00000000000..d734fcad6bc --- /dev/null +++ b/packages/core/schematics/test/router_last_successful_navigation_spec.ts @@ -0,0 +1,134 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {getSystemPath, normalize, virtualFs} from '@angular-devkit/core'; +import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing'; +import {HostTree} from '@angular-devkit/schematics'; +import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing/index.js'; +import {resolve} from 'node:path'; +import shx from 'shelljs'; + +describe('router-last-successful-navigation migration', () => { + let runner: SchematicTestRunner; + let host: TempScopedNodeJsSyncHost; + let tree: UnitTestTree; + let tmpDirPath: string; + + function writeFile(filePath: string, contents: string) { + host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); + } + + function runMigration() { + return runner.runSchematic('router-last-successful-navigation', {}, tree); + } + + const migrationsJsonPath = resolve('../migrations.json'); + beforeEach(() => { + runner = new SchematicTestRunner('test', migrationsJsonPath); + host = new TempScopedNodeJsSyncHost(); + tree = new UnitTestTree(new HostTree(host)); + tmpDirPath = getSystemPath(host.root); + + writeFile('/tsconfig.json', '{}'); + writeFile( + '/angular.json', + JSON.stringify({ + version: 1, + projects: {t: {root: '', architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}, + }), + ); + + writeFile( + '/node_modules/@angular/router/index.d.ts', + ` + export declare class Router { + lastSuccessfulNavigation(): Navigation | null; + } + `, + ); + + shx.cd(tmpDirPath); + }); + + it('should migrate a usage of Router.lastSuccessfulNavigation', async () => { + writeFile( + '/test.ts', + ` + import { Router } from '@angular/router'; + + export class MyService { + constructor(private router: Router) {} + + someMethod() { + const navigation = this.router.lastSuccessfulNavigation; + } + } + `, + ); + + await runMigration(); + expect(tree.readContent('/test.ts')).toContain( + 'const navigation = this.router.lastSuccessfulNavigation();', + ); + }); + + it('should handle a file that is present in multiple projects', async () => { + writeFile('/tsconfig-2.json', '{}'); + writeFile( + '/angular.json', + JSON.stringify({ + version: 1, + projects: { + a: {root: '', architect: {build: {options: {tsConfig: './tsconfig.json'}}}}, + b: {root: '', architect: {build: {options: {tsConfig: './tsconfig-2.json'}}}}, + }, + }), + ); + + writeFile( + 'test.ts', + ` + import { Router } from '@angular/router'; + + export class MyService { + constructor(private router: Router) {} + + someMethod() { + const navigation = this.router.lastSuccessfulNavigation; + } + } + `, + ); + + await runMigration(); + const content = tree.readContent('/test.ts'); + expect(content).toContain('const navigation = this.router.lastSuccessfulNavigation();'); + }); + + it('should not migrate a usage of from non-angular router', async () => { + writeFile( + '/test.ts', + ` + import { Router } from '@not-angular/router'; + + export class MyService { + constructor(private router: Router) {} + + someMethod() { + const navigation = this.router.lastSuccessfulNavigation; + } + } + `, + ); + + await runMigration(); + expect(tree.readContent('/test.ts')).toContain( + 'const navigation = this.router.lastSuccessfulNavigation;', + ); + }); +}); diff --git a/packages/router/src/navigation_transition.ts b/packages/router/src/navigation_transition.ts index 01a27fd1fea..d7a5a2d9aaf 100644 --- a/packages/router/src/navigation_transition.ts +++ b/packages/router/src/navigation_transition.ts @@ -86,7 +86,7 @@ import { } from './router_state'; import type {Params} from './shared'; import {UrlHandlingStrategy} from './url_handling_strategy'; -import {isUrlTree, UrlSerializer, UrlTree} from './url_tree'; +import {UrlSerializer, UrlTree} from './url_tree'; import {Checks, getAllRouteGuards} from './utils/preactivation'; import {CREATE_VIEW_TRANSITION} from './utils/view_transition'; import {getClosestRouteInjector} from './utils/config'; @@ -350,7 +350,7 @@ export class NavigationTransitions { currentNavigation = signal(null, {equal: () => false}); currentTransition: NavigationTransition | null = null; - lastSuccessfulNavigation: Navigation | null = null; + lastSuccessfulNavigation = signal(null); /** * These events are used to communicate back to the Router about the state of the transition. The * Router wants to respond to these events in various ways. Because the `NavigationTransition` @@ -469,6 +469,7 @@ export class NavigationTransitions { return EMPTY; } this.currentTransition = overallTransitionState; + const lastSuccessfulNavigation = this.lastSuccessfulNavigation(); // Store the Navigation object this.currentNavigation.set({ id: t.id, @@ -480,10 +481,10 @@ export class NavigationTransitions { : t.extras.browserUrl, trigger: t.source, extras: t.extras, - previousNavigation: !this.lastSuccessfulNavigation + previousNavigation: !lastSuccessfulNavigation ? null : { - ...this.lastSuccessfulNavigation, + ...lastSuccessfulNavigation, previousNavigation: null, }, abort: () => t.abortController.abort(), @@ -798,7 +799,7 @@ export class NavigationTransitions { tap({ next: (t: NavigationTransition) => { completedOrAborted = true; - this.lastSuccessfulNavigation = untracked(this.currentNavigation); + this.lastSuccessfulNavigation.set(untracked(this.currentNavigation)); this.events.next( new NavigationEnd( t.id, diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 706c04ca515..30379c22b66 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -14,6 +14,7 @@ import { Injectable, ɵPendingTasksInternal as PendingTasks, ɵRuntimeError as RuntimeError, + Signal, Type, untracked, ɵINTERNAL_APPLICATION_ERROR_HANDLER, @@ -357,7 +358,7 @@ export class Router { * The `Navigation` object of the most recent navigation to succeed and `null` if there * has not been a successful navigation yet. */ - get lastSuccessfulNavigation(): Navigation | null { + get lastSuccessfulNavigation(): Signal { return this.navigationTransitions.lastSuccessfulNavigation; } diff --git a/packages/router/test/integration/integration.spec.ts b/packages/router/test/integration/integration.spec.ts index 41aaa33ecdd..86c7c00de25 100644 --- a/packages/router/test/integration/integration.spec.ts +++ b/packages/router/test/integration/integration.spec.ts @@ -748,6 +748,22 @@ for (const browserAPI of ['navigation', 'history'] as const) { expect(fixture.nativeElement).toHaveText('user fedor'); }); + it('should set LastSuccessfulNavigation', async () => { + const router: Router = TestBed.inject(Router); + const fixture = TestBed.createComponent(RootCmp); + router.resetConfig([{path: 'user/:name', component: UserCmp}]); + + expect(router.lastSuccessfulNavigation()).toBe(null); + + router.navigateByUrl('/user/init'); + const navigation = router.getCurrentNavigation(); + expect(router.lastSuccessfulNavigation()).toBe(null); + await advance(fixture); + + expect(router.currentNavigation()).toBe(null); + expect(router.lastSuccessfulNavigation()).toEqual(navigation); + }); + it('should replace state when path is equal to current path', async () => { const router: Router = TestBed.inject(Router); const location: Location = TestBed.inject(Location);