Skip to content

Commit 1fe759d

Browse files
atscottAndrewKushnir
authored andcommitted
refactor(common): Align PathLocationStrategy constructor with default factory (angular#46929)
When using the Angular Router, one of `APP_BASE_HREF` or a `<base>` in the header must be provided. When _not_ using the `RouterModule`, injecting the `LocationStrategy` will result in the `PathLocationStrategy` being provided with a default value used in place of `APP_BASE_HREF` that is `document?.location?.origin ?? ''`. It can be quite surprising and annoying that once you add `RouterModule` to the application, suddenly the `APP_BASE_HREF` must be specifically provide something new when it could use a sensible default instead. The current behavior (before this commit) is as follows: * When `RouterModule` is not provided (or the dev doesn't specifically provide `PathLocationStrategy`): use `DOCUMENT.location?.origin ?? ''`. Note that the base href in the dom and `APP_BASE_HREF` are not used. * When `RouterModule` _is_ provided: 1. APP_BASE_HREF if defined 2. Get base href from DOM 3. throw if neither of the two above are defined This commit updates this behavior to be aligned regardless of `RouterModule` usage. The order (by default) is now: 1. Developer provided `APP_BASE_HREF` 2. base href from the DOM 3. `location.origin` 4. If none of the above exist, use `''` This is slightly different than the behavior before. However, I believe it is more appropriate. For the case without `RouterModule`, it would likely be surprising that `APP_BASE_HREF` and the base href from the DOM are ignored by default. For the case with `RouterModule`, we now have a more sensible fallback/default when neither `APP_BASE_HREF` nor `<base>` are defined (instead of just throwing an error). PR Close angular#46929
1 parent 57d4628 commit 1fe759d

File tree

1 file changed

+7
-23
lines changed

1 file changed

+7
-23
lines changed

packages/common/src/location/location_strategy.ts

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Inject, Injectable, InjectionToken, OnDestroy, Optional, ɵɵinject} from '@angular/core';
9+
import {Inject, inject, Injectable, InjectionToken, OnDestroy, Optional} from '@angular/core';
1010

1111
import {DOCUMENT} from '../dom_tokens';
1212

@@ -30,7 +30,7 @@ import {joinWithSlash, normalizeQueryParams} from './util';
3030
*
3131
* @publicApi
3232
*/
33-
@Injectable({providedIn: 'root', useFactory: provideLocationStrategy})
33+
@Injectable({providedIn: 'root', useFactory: () => inject(PathLocationStrategy)})
3434
export abstract class LocationStrategy {
3535
abstract path(includeHash?: boolean): string;
3636
abstract prepareExternalUrl(internal: string): string;
@@ -46,14 +46,6 @@ export abstract class LocationStrategy {
4646
abstract getBaseHref(): string;
4747
}
4848

49-
export function provideLocationStrategy() {
50-
// See #23917
51-
const location = ɵɵinject(DOCUMENT).location;
52-
return new PathLocationStrategy(
53-
ɵɵinject(PlatformLocation as any), location && location.origin || '');
54-
}
55-
56-
5749
/**
5850
* A predefined [DI token](guide/glossary#di-token) for the base href
5951
* to be used with the `PathLocationStrategy`.
@@ -86,8 +78,8 @@ export const APP_BASE_HREF = new InjectionToken<string>('appBaseHref');
8678
* [path](https://en.wikipedia.org/wiki/Uniform_Resource_Locator#Syntax) of the
8779
* browser's URL.
8880
*
89-
* If you're using `PathLocationStrategy`, you must provide a {@link APP_BASE_HREF}
90-
* or add a `<base href>` element to the document.
81+
* If you're using `PathLocationStrategy`, you may provide a {@link APP_BASE_HREF}
82+
* or add a `<base href>` element to the document to override the default.
9183
*
9284
* For instance, if you provide an `APP_BASE_HREF` of `'/my/app/'` and call
9385
* `location.go('/foo')`, the browser's URL will become
@@ -110,7 +102,7 @@ export const APP_BASE_HREF = new InjectionToken<string>('appBaseHref');
110102
*
111103
* @publicApi
112104
*/
113-
@Injectable()
105+
@Injectable({providedIn: 'root'})
114106
export class PathLocationStrategy extends LocationStrategy implements OnDestroy {
115107
private _baseHref: string;
116108
private _removeListenerFns: (() => void)[] = [];
@@ -120,16 +112,8 @@ export class PathLocationStrategy extends LocationStrategy implements OnDestroy
120112
@Optional() @Inject(APP_BASE_HREF) href?: string) {
121113
super();
122114

123-
if (href == null) {
124-
href = this._platformLocation.getBaseHrefFromDOM();
125-
}
126-
127-
if (href == null) {
128-
throw new Error(
129-
`No base href set. Please provide a value for the APP_BASE_HREF token or add a base element to the document.`);
130-
}
131-
132-
this._baseHref = href;
115+
this._baseHref = href ?? this._platformLocation.getBaseHrefFromDOM() ??
116+
inject(DOCUMENT).location?.origin ?? '';
133117
}
134118

135119
/** @nodoc */

0 commit comments

Comments
 (0)