Skip to content

Commit c53310e

Browse files
atscottAndrewKushnir
atscott
authored and
AndrewKushnir
committed
refactor(router): Update RouterLink href to use host binding and signals (#60875)
This commit updates the method of setting the href attribute on `RouterLink` to use built in host binding rather than custom attribute setting and sanitization. The advantage here would be automatic handling of the sanitization and avoiding of writing the same value to the DOM that we had before. This change does mean that we _always_ write to the href attribute where before we only wrote to it when the elemnt was known to support `href`. That said, the implementation attempts to retain behavior that is as close as possible: the original value of `href` is used and never updated. PR Close #60875
1 parent a1bf58e commit c53310e

File tree

3 files changed

+33
-26
lines changed

3 files changed

+33
-26
lines changed

‎goldens/public-api/router/index.api.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,8 @@ export type RouterHashLocationFeature = RouterFeature<RouterFeatureKind.RouterHa
801801
class RouterLink implements OnChanges, OnDestroy {
802802
constructor(router: Router, route: ActivatedRoute, tabIndexAttribute: string | null | undefined, renderer: Renderer2, el: ElementRef, locationStrategy?: LocationStrategy | undefined);
803803
fragment?: string;
804-
href: string | null;
804+
get href(): string | null;
805+
set href(value: string | null);
805806
info?: unknown;
806807
// (undocumented)
807808
static ngAcceptInputType_preserveFragment: unknown;
@@ -818,6 +819,8 @@ class RouterLink implements OnChanges, OnDestroy {
818819
preserveFragment: boolean;
819820
queryParams?: Params | null;
820821
queryParamsHandling?: QueryParamsHandling | null;
822+
// (undocumented)
823+
protected readonly reactiveHref: i0.WritableSignal<string | null>;
821824
relativeTo?: ActivatedRoute | null;
822825
replaceUrl: boolean;
823826
set routerLink(commandsOrUrlTree: readonly any[] | string | UrlTree | null | undefined);

‎packages/core/test/bundling/router/bundle.golden_symbols.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
"GuardsCheckEnd",
8585
"GuardsCheckStart",
8686
"HistoryStateManager",
87+
"HostAttributeToken",
8788
"INITIAL_NAVIGATION",
8889
"INITIAL_VALUE",
8990
"INJECTOR",
@@ -633,6 +634,7 @@
633634
"producerNotifyConsumers",
634635
"producerRemoveLiveConsumerAtIndex",
635636
"producerUpdateValueVersion",
637+
"producerUpdatesAllowed",
636638
"profiler",
637639
"profilerCallbacks",
638640
"readableStreamLikeToAsyncGenerator",
@@ -689,6 +691,9 @@
689691
"shimStylesContent",
690692
"shouldAddViewToDom",
691693
"shouldSearchParent",
694+
"signal",
695+
"signalAsReadonlyFn",
696+
"signalSetFn",
692697
"split",
693698
"squashSegmentGroup",
694699
"standardizeConfig",
@@ -711,6 +716,7 @@
711716
"throwCyclicDependencyError",
712717
"throwError2",
713718
"throwIfEmpty",
719+
"throwInvalidWriteToSignalError",
714720
"throwInvalidWriteToSignalErrorFn",
715721
"throwProviderNotFoundError",
716722
"timeoutProvider",
@@ -748,9 +754,11 @@
748754
"ɵɵelementStart",
749755
"ɵɵgetInheritedFactory",
750756
"ɵɵinject",
757+
"ɵɵinjectAttribute",
751758
"ɵɵlistener",
752759
"ɵɵsanitizeResourceUrl",
753760
"ɵɵsanitizeUrl",
761+
"ɵɵsanitizeUrlOrResourceUrl",
754762
"ɵɵtext",
755763
"ɵɵtextInterpolate1"
756764
]

‎packages/router/src/directives/router_link.ts

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ import {
2020
Renderer2,
2121
ɵRuntimeError as RuntimeError,
2222
SimpleChanges,
23-
ɵɵsanitizeUrlOrResourceUrl,
2423
ɵINTERNAL_APPLICATION_ERROR_HANDLER,
2524
inject,
25+
signal,
26+
untracked,
27+
HostAttributeToken,
2628
} from '@angular/core';
2729
import {Subject, Subscription} from 'rxjs';
2830

@@ -142,14 +144,25 @@ import {RuntimeErrorCode} from '../errors';
142144
*/
143145
@Directive({
144146
selector: '[routerLink]',
147+
host: {
148+
'[attr.href]': 'reactiveHref()',
149+
},
145150
})
146151
export class RouterLink implements OnChanges, OnDestroy {
152+
/** @nodoc */
153+
protected readonly reactiveHref = signal<string | null>(null);
147154
/**
148155
* Represents an `href` attribute value applied to a host element,
149156
* when a host element is an `<a>`/`<area>` tag or a compatible custom element.
150157
* For other tags, the value is `null`.
151158
*/
152-
href: string | null = null;
159+
get href() {
160+
return untracked(this.reactiveHref);
161+
}
162+
/** @deprecated */
163+
set href(value: string | null) {
164+
this.reactiveHref.set(value);
165+
}
153166

154167
/**
155168
* Represents the `target` attribute on a host element.
@@ -222,6 +235,8 @@ export class RouterLink implements OnChanges, OnDestroy {
222235
private readonly el: ElementRef,
223236
private locationStrategy?: LocationStrategy,
224237
) {
238+
// Set the initial href value to whatever exists on the host element already
239+
this.reactiveHref.set(inject(new HostAttributeToken('href'), {optional: true}));
225240
const tagName = el.nativeElement.tagName?.toLowerCase();
226241
this.isAnchorElement =
227242
tagName === 'a' ||
@@ -392,30 +407,11 @@ export class RouterLink implements OnChanges, OnDestroy {
392407

393408
private updateHref(): void {
394409
const urlTree = this.urlTree;
395-
this.href =
410+
this.reactiveHref.set(
396411
urlTree !== null && this.locationStrategy
397-
? this.locationStrategy?.prepareExternalUrl(this.router.serializeUrl(urlTree))
398-
: null;
399-
400-
const sanitizedValue =
401-
this.href === null
402-
? null
403-
: // This class represents a directive that can be added to both `<a>` elements,
404-
// as well as other elements. As a result, we can't define security context at
405-
// compile time. So the security context is deferred to runtime.
406-
// The `ɵɵsanitizeUrlOrResourceUrl` selects the necessary sanitizer function
407-
// based on the tag and property names. The logic mimics the one from
408-
// `packages/compiler/src/schema/dom_security_schema.ts`, which is used at compile time.
409-
//
410-
// Note: we should investigate whether we can switch to using `@HostBinding('attr.href')`
411-
// instead of applying a value via a renderer, after a final merge of the
412-
// `RouterLinkWithHref` directive.
413-
ɵɵsanitizeUrlOrResourceUrl(
414-
this.href,
415-
this.el.nativeElement.tagName.toLowerCase(),
416-
'href',
417-
);
418-
this.applyAttributeValue('href', sanitizedValue);
412+
? (this.locationStrategy?.prepareExternalUrl(this.router.serializeUrl(urlTree)) ?? '')
413+
: null,
414+
);
419415
}
420416

421417
private applyAttributeValue(attrName: string, attrValue: string | null) {

0 commit comments

Comments
 (0)