Home > Net >  Why is referencing `window.location` considered a bad practice?
Why is referencing `window.location` considered a bad practice?

Time:10-12

While building a shared library for internal use, which will be used by several teams for Angular applications. These applications will only run in the browser. I've used the code below to construct a returnUrl - to which a user returns after logging in with an external identity provider like Keycloak.

private _iframeRedirectUri = `${window.location.origin} ... the rest of the url`;

In the PR I made, a co-worker made comments regarding window.location.origin. He stated that referencing the native web apis is considered a bad practice and I should use a library like ng-web-apis (https://github.com/ng-web-apis/) and getting the location.origin like this:

private _iframeRedirectUri = `${this.location.origin} ... the rest of the url`;

constructor(@Inject(LOCATION) readonly location: Location) {}

When looking at the source code of ng-web-apis, the LOCATION token looks like this:

export const LOCATION = new InjectionToken<Location>(
    'An abstraction over window.location object',
    {
        factory: () => inject(WINDOW).location,
    },
);

// And window:

export const WINDOW = new InjectionToken<Window>(
    'An abstraction over global window object',
    {
        factory: () => {
            const {defaultView} = inject(DOCUMENT);

            if (!defaultView) {
                throw new Error('Window is not available');
            }

            return defaultView;
        },
    },
);

What's the point in this abstraction when these application will never run in a backend and always in the browser? Discussing this with my co-worker results in "it's just evil" and "never reference stuff like window or document", so that's not helping me why. Can anyone help me understand this issue?

CodePudding user response:

From my experience - it is not good from a testing perspective. If you have actions that must change your routing data (as an example) - your tests will do the same and the page will reload once hit window.location change statement.

Declaring it as an injection token allows you to use dependency injection to instantiate proper window instance per use - for tests you can bind to mock object and to native window otherwise.

    TestBed.configureTestingModule({
      providers: [
        {provide: WINDOW, useValue: mockWindow}
      ]
    });

CodePudding user response:

I would say that you can not be sure what changes will be made to your application in the future. But I see at least 2 advantages of this approach

  • SSR
  • Much easier way to mock the browser API via DI in unit tests
  • Related