Skip to content

Commit

Permalink
feat(router): merge RouterLinkWithHref into RouterLink (#47630)
Browse files Browse the repository at this point in the history
This commit updates the `RouterLink` to extend the selector to also include `<a>` and `<area>` elements, which were previously matched by the `RouterLinkWithHref` directive. The code of the directives was merged together (since there was a lot of duplication) and this commit finalizes the merge. The `RouterLinkWithHref` becomes an alias of the `RouterLink` directive.

Now there is no need to import and use the `RouterLinkWithHref` class, the `RouterLink` directive will match any element that has the `routerLink` attribute.

DEPRECATED:

The `RouterLinkWithHref` directive is deprecated, use the `RouterLink` directive instead. The `RouterLink` contains the code from the `RouterLinkWithHref` to handle elements with `href` attributes.

PR Close #47630
  • Loading branch information
AndrewKushnir authored and thePunderWoman committed Oct 5, 2022
1 parent 8df8c77 commit f73ef21
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 101 deletions.
Expand Up @@ -8,7 +8,7 @@ import { asyncData } from '../testing';
import { RouterTestingModule } from '@angular/router/testing';
import { SpyLocation } from '@angular/common/testing';

import { Router, RouterLinkWithHref } from '@angular/router';
import { Router, RouterLink } from '@angular/router';

import { By } from '@angular/platform-browser';
import { DebugElement, Type } from '@angular/core';
Expand Down Expand Up @@ -157,7 +157,7 @@ class Page {
fixture: ComponentFixture<AppComponent>;

constructor() {
const links = fixture.debugElement.queryAll(By.directive(RouterLinkWithHref));
const links = fixture.debugElement.queryAll(By.directive(RouterLink));
this.aboutLinkDe = links[2];
this.dashboardLinkDe = links[0];
this.heroesLinkDe = links[1];
Expand Down
2 changes: 2 additions & 0 deletions aio/content/guide/deprecations.md
Expand Up @@ -83,6 +83,7 @@ v15 - v18
| `@angular/router` | [`relativeLinkResolution`](#relativeLinkResolution) | <!-- v14 --> v16 |
| `@angular/router` | [`resolver` argument in `RouterOutletContract.activateWith`](#router) | <!-- v14 --> v16 |
| `@angular/router` | [`resolver` field of the `OutletContext` class](#router) | <!-- v14 --> v16 |
| `@angular/router` | [`RouterLinkWithHref` directive](#router) | <!-- v15 --> v17 |
| `@angular/service-worker` | [`SwUpdate#activated`](api/service-worker/SwUpdate#activated) | <!-- v13 --> v16 |
| `@angular/service-worker` | [`SwUpdate#available`](api/service-worker/SwUpdate#available) | <!-- v13 --> v16 |
| template syntax | [`/deep/`, `>>>`, and `::ng-deep`](#deep-component-style-selector) | <!-- v7 --> unspecified |
Expand Down Expand Up @@ -163,6 +164,7 @@ In the [API reference section](api) of this site, deprecated APIs are indicated
|:--- |:--- |:--- |:--- |
| [`resolver` argument in `RouterOutletContract.activateWith`](api/router/RouterOutletContract#activatewith) | No replacement needed | v14 | Component factories are not required to create an instance of a component dynamically. Passing a factory resolver via `resolver` argument is no longer needed. |
| [`resolver` field of the `OutletContext` class](api/router/OutletContext#resolver) | No replacement needed | v14 | Component factories are not required to create an instance of a component dynamically. Passing a factory resolver via `resolver` class field is no longer needed. |
| [`RouterLinkWithHref` directive](api/router/RouterLinkWithHref) | Use `RouterLink` instead. | v15 | The `RouterLinkWithHref` directive code was merged into `RouterLink`. Now the `RouterLink` directive can be used for all elements that have `routerLink` attribute. |


<a id="platform-browser"></a>
Expand Down
24 changes: 8 additions & 16 deletions goldens/public-api/router/index.md
Expand Up @@ -713,7 +713,7 @@ export interface RouterFeature<FeatureKind extends RouterFeatureKind> {
export type RouterFeatures = PreloadingFeature | DebugTracingFeature | InitialNavigationFeature | InMemoryScrollingFeature | RouterConfigurationFeature;

// @public
export class RouterLink implements OnChanges, OnDestroy {
class RouterLink implements OnChanges, OnDestroy {
constructor(router: Router, route: ActivatedRoute, tabIndexAttribute: string | null | undefined, renderer: Renderer2, el: ElementRef, locationStrategy?: LocationStrategy | undefined);
fragment?: string;
href: string | null;
Expand Down Expand Up @@ -743,23 +743,23 @@ export class RouterLink implements OnChanges, OnDestroy {
// (undocumented)
get urlTree(): UrlTree | null;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLink, ":not(a):not(area)[routerLink]", never, { "target": "target"; "queryParams": "queryParams"; "fragment": "fragment"; "queryParamsHandling": "queryParamsHandling"; "state": "state"; "relativeTo": "relativeTo"; "preserveFragment": "preserveFragment"; "skipLocationChange": "skipLocationChange"; "replaceUrl": "replaceUrl"; "routerLink": "routerLink"; }, {}, never, never, true, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLink, ":not(a):not(area)[routerLink],a[routerLink],area[routerLink]", never, { "target": "target"; "queryParams": "queryParams"; "fragment": "fragment"; "queryParamsHandling": "queryParamsHandling"; "state": "state"; "relativeTo": "relativeTo"; "preserveFragment": "preserveFragment"; "skipLocationChange": "skipLocationChange"; "replaceUrl": "replaceUrl"; "routerLink": "routerLink"; }, {}, never, never, true, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<RouterLink, [null, null, { attribute: "tabindex"; }, null, null, null]>;
}
export { RouterLink }
export { RouterLink as RouterLinkWithHref }

// @public
export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit {
constructor(router: Router, element: ElementRef, renderer: Renderer2, cdr: ChangeDetectorRef, link?: RouterLink | undefined, linkWithHref?: RouterLinkWithHref | undefined);
constructor(router: Router, element: ElementRef, renderer: Renderer2, cdr: ChangeDetectorRef, link?: RouterLink | undefined);
ariaCurrentWhenActive?: 'page' | 'step' | 'location' | 'date' | 'time' | true | false;
// (undocumented)
readonly isActive: boolean;
readonly isActiveChange: EventEmitter<boolean>;
// (undocumented)
links: QueryList<RouterLink>;
// (undocumented)
linksWithHrefs: QueryList<RouterLinkWithHref>;
// (undocumented)
ngAfterContentInit(): void;
// (undocumented)
ngOnChanges(changes: SimpleChanges): void;
Expand All @@ -771,17 +771,9 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
exact: boolean;
} | IsActiveMatchOptions;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLinkActive, "[routerLinkActive]", ["routerLinkActive"], { "routerLinkActiveOptions": "routerLinkActiveOptions"; "ariaCurrentWhenActive": "ariaCurrentWhenActive"; "routerLinkActive": "routerLinkActive"; }, { "isActiveChange": "isActiveChange"; }, ["links", "linksWithHrefs"], never, true, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<RouterLinkActive, [null, null, null, null, { optional: true; }, { optional: true; }]>;
}

// @public
export class RouterLinkWithHref extends RouterLink {
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLinkWithHref, "a[routerLink],area[routerLink]", never, {}, {}, never, never, true, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLinkActive, "[routerLinkActive]", ["routerLinkActive"], { "routerLinkActiveOptions": "routerLinkActiveOptions"; "ariaCurrentWhenActive": "ariaCurrentWhenActive"; "routerLinkActive": "routerLinkActive"; }, { "isActiveChange": "isActiveChange"; }, ["links"], never, true, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<RouterLinkWithHref, never>;
static ɵfac: i0.ɵɵFactoryDeclaration<RouterLinkActive, [null, null, null, null, { optional: true; }]>;
}

// @public
Expand All @@ -794,7 +786,7 @@ export class RouterModule {
// (undocumented)
static ɵinj: i0.ɵɵInjectorDeclaration<RouterModule>;
// (undocumented)
static ɵmod: i0.ɵɵNgModuleDeclaration<RouterModule, never, [typeof i1.RouterOutlet, typeof i2.RouterLink, typeof i2.RouterLinkWithHref, typeof i3.RouterLinkActive, typeof i4EmptyOutletComponent], [typeof i1.RouterOutlet, typeof i2.RouterLink, typeof i2.RouterLinkWithHref, typeof i3.RouterLinkActive, typeof i4EmptyOutletComponent]>;
static ɵmod: i0.ɵɵNgModuleDeclaration<RouterModule, never, [typeof i1.RouterOutlet, typeof i2.RouterLink, typeof i3.RouterLinkActive, typeof i4EmptyOutletComponent], [typeof i1.RouterOutlet, typeof i2.RouterLink, typeof i3.RouterLinkActive, typeof i4EmptyOutletComponent]>;
}

// @public
Expand Down
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
Expand Up @@ -33,7 +33,7 @@
"cli-hello-world-lazy": {
"uncompressed": {
"runtime": 2835,
"main": 226404,
"main": 225113,
"polyfills": 33842,
"src_app_lazy_lazy_routes_ts": 487
}
Expand Down
36 changes: 0 additions & 36 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Expand Up @@ -482,9 +482,6 @@
{
"name": "RouterLinkActive"
},
{
"name": "RouterLinkWithHref"
},
{
"name": "RouterOutlet"
},
Expand Down Expand Up @@ -983,9 +980,6 @@
{
"name": "extractDirectiveDef"
},
{
"name": "fillProperties"
},
{
"name": "filter"
},
Expand Down Expand Up @@ -1091,9 +1085,6 @@
{
"name": "getFactoryDef"
},
{
"name": "getFactoryOf"
},
{
"name": "getFirstLContainer"
},
Expand Down Expand Up @@ -1262,15 +1253,6 @@
{
"name": "incrementInitPhaseFlags"
},
{
"name": "inheritContentQueries"
},
{
"name": "inheritHostBindings"
},
{
"name": "inheritViewQuery"
},
{
"name": "inheritedParamsDataResolve"
},
Expand Down Expand Up @@ -1358,9 +1340,6 @@
{
"name": "isEmptyError"
},
{
"name": "isForwardRef"
},
{
"name": "isFunction"
},
Expand Down Expand Up @@ -1490,9 +1469,6 @@
{
"name": "maybeUnwrapDefaultExport"
},
{
"name": "maybeUnwrapEmpty"
},
{
"name": "maybeUnwrapFn"
},
Expand Down Expand Up @@ -1547,9 +1523,6 @@
{
"name": "noMatch2"
},
{
"name": "noSideEffects"
},
{
"name": "nodeChildrenAsMap"
},
Expand Down Expand Up @@ -1865,9 +1838,6 @@
{
"name": "ɵEmptyOutletComponent"
},
{
"name": "ɵɵInheritDefinitionFeature"
},
{
"name": "ɵɵNgOnChangesFeature"
},
Expand All @@ -1877,9 +1847,6 @@
{
"name": "ɵɵattribute"
},
{
"name": "ɵɵcontentQuery"
},
{
"name": "ɵɵdefineComponent"
},
Expand Down Expand Up @@ -1907,9 +1874,6 @@
{
"name": "ɵɵlistener"
},
{
"name": "ɵɵloadQuery"
},
{
"name": "ɵɵqueryRefresh"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/bundling/router/index.ts
Expand Up @@ -8,7 +8,7 @@
import {APP_BASE_HREF} from '@angular/common';
import {Component, OnInit} from '@angular/core';
import {bootstrapApplication} from '@angular/platform-browser';
import {ActivatedRoute, provideRouter, Router, RouterLinkActive, RouterLinkWithHref, RouterOutlet, Routes} from '@angular/router';
import {ActivatedRoute, provideRouter, Router, RouterLink, RouterLinkActive, RouterOutlet, Routes} from '@angular/router';

@Component({
selector: 'app-list',
Expand All @@ -20,7 +20,7 @@ import {ActivatedRoute, provideRouter, Router, RouterLinkActive, RouterLinkWithH
</ul>
`,
standalone: true,
imports: [RouterLinkWithHref, RouterLinkActive],
imports: [RouterLink, RouterLinkActive],
})
class ListComponent {
}
Expand Down
18 changes: 5 additions & 13 deletions packages/router/src/directives/router_link.ts
Expand Up @@ -116,7 +116,7 @@ import {UrlTree} from '../url_tree';
* @publicApi
*/
@Directive({
selector: ':not(a):not(area)[routerLink]',
selector: ':not(a):not(area)[routerLink],a[routerLink],area[routerLink]',
standalone: true,
})
export class RouterLink implements OnChanges, OnDestroy {
Expand Down Expand Up @@ -375,18 +375,10 @@ export class RouterLink implements OnChanges, OnDestroy {

/**
* @description
* An alias for the `RouterLink` directive.
* Deprecated since v15, use `RouterLink` directive instead.
*
* Lets you link to specific routes in your app.
*
* See `RouterLink` for more information.
*
* @ngModule RouterModule
*
* @deprecated use `RouterLink` directive instead.
* @publicApi
*/
@Directive({
selector: 'a[routerLink],area[routerLink]',
standalone: true,
})
export class RouterLinkWithHref extends RouterLink {
}
export {RouterLink as RouterLinkWithHref};
27 changes: 10 additions & 17 deletions packages/router/src/directives/router_link_active.ts
Expand Up @@ -14,7 +14,7 @@ import {Event, NavigationEnd} from '../events';
import {Router} from '../router';
import {IsActiveMatchOptions} from '../url_tree';

import {RouterLink, RouterLinkWithHref} from './router_link';
import {RouterLink} from './router_link';


/**
Expand Down Expand Up @@ -93,8 +93,6 @@ import {RouterLink, RouterLinkWithHref} from './router_link';
})
export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit {
@ContentChildren(RouterLink, {descendants: true}) links!: QueryList<RouterLink>;
@ContentChildren(RouterLinkWithHref, {descendants: true})
linksWithHrefs!: QueryList<RouterLinkWithHref>;

private classes: string[] = [];
private routerEventsSubscription: Subscription;
Expand Down Expand Up @@ -140,8 +138,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit

constructor(
private router: Router, private element: ElementRef, private renderer: Renderer2,
private readonly cdr: ChangeDetectorRef, @Optional() private link?: RouterLink,
@Optional() private linkWithHref?: RouterLinkWithHref) {
private readonly cdr: ChangeDetectorRef, @Optional() private link?: RouterLink) {
this.routerEventsSubscription = router.events.subscribe((s: Event) => {
if (s instanceof NavigationEnd) {
this.update();
Expand All @@ -152,18 +149,17 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
/** @nodoc */
ngAfterContentInit(): void {
// `of(null)` is used to force subscribe body to execute once immediately (like `startWith`).
of(this.links.changes, this.linksWithHrefs.changes, of(null)).pipe(mergeAll()).subscribe(_ => {
of(this.links.changes, of(null)).pipe(mergeAll()).subscribe(_ => {
this.update();
this.subscribeToEachLinkOnChanges();
});
}

private subscribeToEachLinkOnChanges() {
this.linkInputChangesSubscription?.unsubscribe();
const allLinkChanges =
[...this.links.toArray(), ...this.linksWithHrefs.toArray(), this.link, this.linkWithHref]
.filter((link): link is RouterLink|RouterLinkWithHref => !!link)
.map(link => link.onChanges);
const allLinkChanges = [...this.links.toArray(), this.link]
.filter((link): link is RouterLink => !!link)
.map(link => link.onChanges);
this.linkInputChangesSubscription = from(allLinkChanges).pipe(mergeAll()).subscribe(link => {
if (this.isActive !== this.isLinkActive(this.router)(link)) {
this.update();
Expand All @@ -188,7 +184,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
}

private update(): void {
if (!this.links || !this.linksWithHrefs || !this.router.navigated) return;
if (!this.links || !this.router.navigated) return;
Promise.resolve().then(() => {
const hasActiveLinks = this.hasActiveLinks();
if (this.isActive !== hasActiveLinks) {
Expand All @@ -214,21 +210,18 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
});
}

private isLinkActive(router: Router): (link: (RouterLink|RouterLinkWithHref)) => boolean {
private isLinkActive(router: Router): (link: RouterLink) => boolean {
const options: boolean|IsActiveMatchOptions =
isActiveMatchOptions(this.routerLinkActiveOptions) ?
this.routerLinkActiveOptions :
// While the types should disallow `undefined` here, it's possible without strict inputs
(this.routerLinkActiveOptions.exact || false);
return (link: RouterLink|RouterLinkWithHref) =>
link.urlTree ? router.isActive(link.urlTree, options) : false;
return (link: RouterLink) => link.urlTree ? router.isActive(link.urlTree, options) : false;
}

private hasActiveLinks(): boolean {
const isActiveCheckFn = this.isLinkActive(this.router);
return this.link && isActiveCheckFn(this.link) ||
this.linkWithHref && isActiveCheckFn(this.linkWithHref) ||
this.links.some(isActiveCheckFn) || this.linksWithHrefs.some(isActiveCheckFn);
return this.link && isActiveCheckFn(this.link) || this.links.some(isActiveCheckFn);
}
}

Expand Down
5 changes: 2 additions & 3 deletions packages/router/src/router_module.ts
Expand Up @@ -10,7 +10,7 @@ import {HashLocationStrategy, Location, LocationStrategy, PathLocationStrategy,
import {APP_BOOTSTRAP_LISTENER, ComponentRef, inject, Inject, InjectionToken, ModuleWithProviders, NgModule, NgProbeToken, Optional, Provider, SkipSelf, ɵRuntimeError as RuntimeError} from '@angular/core';

import {EmptyOutletComponent} from './components/empty_outlet';
import {RouterLink, RouterLinkWithHref} from './directives/router_link';
import {RouterLink} from './directives/router_link';
import {RouterLinkActive} from './directives/router_link_active';
import {RouterOutlet} from './directives/router_outlet';
import {RuntimeErrorCode} from './errors';
Expand All @@ -29,8 +29,7 @@ const NG_DEV_MODE = typeof ngDevMode === 'undefined' || ngDevMode;
/**
* The directives defined in the `RouterModule`.
*/
const ROUTER_DIRECTIVES =
[RouterOutlet, RouterLink, RouterLinkWithHref, RouterLinkActive, EmptyOutletComponent];
const ROUTER_DIRECTIVES = [RouterOutlet, RouterLink, RouterLinkActive, EmptyOutletComponent];

/**
* @docsNotRequired
Expand Down

0 comments on commit f73ef21

Please sign in to comment.