From c53310ee81db5fc64776f6038df69dba42564e9d Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 15 Apr 2025 12:34:10 -0700 Subject: [PATCH] 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 --- goldens/public-api/router/index.api.md | 5 +- .../router/bundle.golden_symbols.json | 8 ++++ packages/router/src/directives/router_link.ts | 46 +++++++++---------- 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/goldens/public-api/router/index.api.md b/goldens/public-api/router/index.api.md index 8fade13d998..72007e4d023 100644 --- a/goldens/public-api/router/index.api.md +++ b/goldens/public-api/router/index.api.md @@ -801,7 +801,8 @@ export type RouterHashLocationFeature = RouterFeature; relativeTo?: ActivatedRoute | null; replaceUrl: boolean; set routerLink(commandsOrUrlTree: readonly any[] | string | UrlTree | null | undefined); diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index bc84bd940ab..bc1a58c474f 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -84,6 +84,7 @@ "GuardsCheckEnd", "GuardsCheckStart", "HistoryStateManager", + "HostAttributeToken", "INITIAL_NAVIGATION", "INITIAL_VALUE", "INJECTOR", @@ -633,6 +634,7 @@ "producerNotifyConsumers", "producerRemoveLiveConsumerAtIndex", "producerUpdateValueVersion", + "producerUpdatesAllowed", "profiler", "profilerCallbacks", "readableStreamLikeToAsyncGenerator", @@ -689,6 +691,9 @@ "shimStylesContent", "shouldAddViewToDom", "shouldSearchParent", + "signal", + "signalAsReadonlyFn", + "signalSetFn", "split", "squashSegmentGroup", "standardizeConfig", @@ -711,6 +716,7 @@ "throwCyclicDependencyError", "throwError2", "throwIfEmpty", + "throwInvalidWriteToSignalError", "throwInvalidWriteToSignalErrorFn", "throwProviderNotFoundError", "timeoutProvider", @@ -748,9 +754,11 @@ "ɵɵelementStart", "ɵɵgetInheritedFactory", "ɵɵinject", + "ɵɵinjectAttribute", "ɵɵlistener", "ɵɵsanitizeResourceUrl", "ɵɵsanitizeUrl", + "ɵɵsanitizeUrlOrResourceUrl", "ɵɵtext", "ɵɵtextInterpolate1" ] \ No newline at end of file diff --git a/packages/router/src/directives/router_link.ts b/packages/router/src/directives/router_link.ts index a78f19b67f3..b1e214cfa59 100644 --- a/packages/router/src/directives/router_link.ts +++ b/packages/router/src/directives/router_link.ts @@ -20,9 +20,11 @@ import { Renderer2, ɵRuntimeError as RuntimeError, SimpleChanges, - ɵɵsanitizeUrlOrResourceUrl, ɵINTERNAL_APPLICATION_ERROR_HANDLER, inject, + signal, + untracked, + HostAttributeToken, } from '@angular/core'; import {Subject, Subscription} from 'rxjs'; @@ -142,14 +144,25 @@ import {RuntimeErrorCode} from '../errors'; */ @Directive({ selector: '[routerLink]', + host: { + '[attr.href]': 'reactiveHref()', + }, }) export class RouterLink implements OnChanges, OnDestroy { + /** @nodoc */ + protected readonly reactiveHref = signal(null); /** * Represents an `href` attribute value applied to a host element, * when a host element is an ``/`` tag or a compatible custom element. * For other tags, the value is `null`. */ - href: string | null = null; + get href() { + return untracked(this.reactiveHref); + } + /** @deprecated */ + set href(value: string | null) { + this.reactiveHref.set(value); + } /** * Represents the `target` attribute on a host element. @@ -222,6 +235,8 @@ export class RouterLink implements OnChanges, OnDestroy { private readonly el: ElementRef, private locationStrategy?: LocationStrategy, ) { + // Set the initial href value to whatever exists on the host element already + this.reactiveHref.set(inject(new HostAttributeToken('href'), {optional: true})); const tagName = el.nativeElement.tagName?.toLowerCase(); this.isAnchorElement = tagName === 'a' || @@ -392,30 +407,11 @@ export class RouterLink implements OnChanges, OnDestroy { private updateHref(): void { const urlTree = this.urlTree; - this.href = + this.reactiveHref.set( urlTree !== null && this.locationStrategy - ? this.locationStrategy?.prepareExternalUrl(this.router.serializeUrl(urlTree)) - : null; - - const sanitizedValue = - this.href === null - ? null - : // This class represents a directive that can be added to both `` elements, - // as well as other elements. As a result, we can't define security context at - // compile time. So the security context is deferred to runtime. - // The `ɵɵsanitizeUrlOrResourceUrl` selects the necessary sanitizer function - // based on the tag and property names. The logic mimics the one from - // `packages/compiler/src/schema/dom_security_schema.ts`, which is used at compile time. - // - // Note: we should investigate whether we can switch to using `@HostBinding('attr.href')` - // instead of applying a value via a renderer, after a final merge of the - // `RouterLinkWithHref` directive. - ɵɵsanitizeUrlOrResourceUrl( - this.href, - this.el.nativeElement.tagName.toLowerCase(), - 'href', - ); - this.applyAttributeValue('href', sanitizedValue); + ? (this.locationStrategy?.prepareExternalUrl(this.router.serializeUrl(urlTree)) ?? '') + : null, + ); } private applyAttributeValue(attrName: string, attrValue: string | null) {