feat: introduce SSR variable for platform checks (#1080)

* add rule for using new SSR variable * use SSR variable to determine PLATFORM_ID * migration note for using 'SSR' varibale instead of platform checks BREAKING CHANGE: Introduced the build variable `SSR` that is now used for all checks if the application is running in SSR or Browser context (see [Migrations / 2.4 to 3.0](https://github.com/intershop/intershop-pwa/blob/develop/docs/guides/migrations.md#2.4-to-30) for more details).
intershop · Jul 19, 2022 · 7c6a226 · 7c6a226
1 parent 491d78b
commit 7c6a226
Show file tree

Hide file tree

Showing 48 changed files with 232 additions and 247 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -657,6 +657,7 @@
         "ish-custom-rules/use-component-change-detection": "warn",
         "ish-custom-rules/use-correct-component-overrides": "warn",
         "ish-custom-rules/use-jest-extended-matchers-in-tests": "warn",
+        "ish-custom-rules/use-ssr-variable-instead-of-platform-id": "warn",
         "ish-custom-rules/require-formly-code-documentation": "warn",
         "jest/no-commented-out-tests": "warn",
         "jest/no-disabled-tests": "warn",

diff --git a/docs/concepts/configuration.md b/docs/concepts/configuration.md
@@ -66,18 +66,13 @@ The actual transfer is handled by the [`app.server.module.ts`](../../src/app/app
 Accessing the transferred state in a service or a component is pretty straight forward:
 
 ```typescript
-import { isPlatformBrowser } from '@angular/common';
-import { PLATFORM_ID } from '@angular/core';
 import { NEW_KEY } from 'ish-core/configurations/state-keys';
 
 newKey string;
 
-constructor(
-  @Inject(PLATFORM_ID) private platformId: string,
-  private transferState: TransferState,
-) {}
+constructor(private transferState: TransferState) {}
 
-if (isPlatformBrowser(this.platformId)) {
+if (!SSR) {
   this.newKey = this.transferState.get<string>(NEW_KEY, 'default value');
 }
 ```

diff --git a/docs/guides/migrations.md b/docs/guides/migrations.md
@@ -37,6 +37,11 @@ You might need to keep this dependency if you are loading translations different
 
 The deprecated `customized-copy` schematic for copying components and replacing all usages was removed.
 
+We introduced a build variable `SSR` that is now used for all checks if the application is running in SSR or Browser context.
+We no longer use the verbose way of injecting the `PLATFORM_ID` and check it with the methods `isPlatformBrowser` or `isPlatformServer`.
+This way still works but it is discouraged by a new ESLint rule that suggests using the new `SSR` variable instead.
+So running `npm run lint` will help with finding custom code that still relies on the platform checks.
+
 ## 2.3 to 2.4
 
 The PWA 2.4 contains an Angular update to version 13.3.10 and many other dependencies updates.

diff --git a/eslint-rules/src/rules/use-ssr-variable-instead-of-platform-id.ts b/eslint-rules/src/rules/use-ssr-variable-instead-of-platform-id.ts
@@ -0,0 +1,42 @@
+import { TSESLint } from '@typescript-eslint/utils';
+
+const useSsrVariableInsteadOfPlatformIdRule: TSESLint.RuleModule<string> = {
+  meta: {
+    docs: {
+      description:
+        'Instead of using `isPlatformBrowser` and `isPlatformServer` together with the injected `PLATFORM_ID`, the Intershop PWA provides a `SSR` variable that can be used for this.',
+      recommended: 'warn',
+      url: '',
+    },
+    messages: {
+      useSsrVariableInsteadOfPlatformIdForBrowser: 'Use the expression !SSR instead.',
+      useSsrVariableInsteadOfPlatformIdForServer: 'Use the variable SSR instead.',
+    },
+    type: 'problem',
+    schema: [],
+    fixable: 'code',
+    hasSuggestions: true,
+  },
+  create: context => ({
+    'CallExpression[callee.name=isPlatformBrowser]'(node) {
+      context.report({
+        messageId: 'useSsrVariableInsteadOfPlatformIdForBrowser',
+        node,
+        suggest: [
+          { fix: fixer => fixer.replaceText(node, '!SSR'), messageId: 'useSsrVariableInsteadOfPlatformIdForBrowser' },
+        ],
+      });
+    },
+    'CallExpression[callee.name=isPlatformServer]'(node) {
+      context.report({
+        messageId: 'useSsrVariableInsteadOfPlatformIdForServer',
+        node,
+        suggest: [
+          { fix: fixer => fixer.replaceText(node, 'SSR'), messageId: 'useSsrVariableInsteadOfPlatformIdForServer' },
+        ],
+      });
+    },
+  }),
+};
+
+export default useSsrVariableInsteadOfPlatformIdRule;
diff --git a/eslint-rules/tests/use-ssr-variable-instead-of-platform-id.spec.ts b/eslint-rules/tests/use-ssr-variable-instead-of-platform-id.spec.ts
@@ -0,0 +1,39 @@
+import useSsrVariableInsteadOfPlatformIdRule from '../src/rules/use-ssr-variable-instead-of-platform-id';
+
+import testRule from './rule-tester';
+
+testRule(useSsrVariableInsteadOfPlatformIdRule, {
+  valid: [],
+  invalid: [
+    {
+      filename: 'file.ts',
+      code: `if (isPlatformBrowser(this.platformId)) {}`,
+      errors: [
+        {
+          messageId: 'useSsrVariableInsteadOfPlatformIdForBrowser',
+          suggestions: [
+            {
+              messageId: 'useSsrVariableInsteadOfPlatformIdForBrowser',
+              output: `if (!SSR) {}`,
+            },
+          ],
+        },
+      ],
+    },
+    {
+      filename: 'file.ts',
+      code: `if (isPlatformServer(this.platformId)) {}`,
+      errors: [
+        {
+          messageId: 'useSsrVariableInsteadOfPlatformIdForServer',
+          suggestions: [
+            {
+              messageId: 'useSsrVariableInsteadOfPlatformIdForServer',
+              output: `if (SSR) {}`,
+            },
+          ],
+        },
+      ],
+    },
+  ],
+});
diff --git a/src/app/app.component.ts b/src/app/app.component.ts
@@ -1,5 +1,4 @@
-import { isPlatformBrowser } from '@angular/common';
-import { ChangeDetectionStrategy, Component, Inject, OnInit, PLATFORM_ID, ViewChild } from '@angular/core';
+import { ChangeDetectionStrategy, Component, OnInit, ViewChild } from '@angular/core';
 import { Observable } from 'rxjs';
 
 import { AppFacade } from 'ish-core/facades/app.facade';
@@ -17,13 +16,11 @@ import { DeviceType } from 'ish-core/models/viewtype/viewtype.types';
 })
 export class AppComponent implements OnInit {
   @ViewChild('cookie', { static: true })
-  isBrowser: boolean;
+  isBrowser = !SSR;
   wrapperClasses$: Observable<string[]>;
   deviceType$: Observable<DeviceType>;
 
-  constructor(private appFacade: AppFacade, @Inject(PLATFORM_ID) platformId: string) {
-    this.isBrowser = isPlatformBrowser(platformId);
-  }
+  constructor(private appFacade: AppFacade) {}
 
   ngOnInit() {
     this.deviceType$ = this.appFacade.deviceType$;

diff --git a/src/app/core/directives/intersection-observer.directive.ts b/src/app/core/directives/intersection-observer.directive.ts
@@ -1,15 +1,4 @@
-import { isPlatformBrowser } from '@angular/common';
-import {
-  Directive,
-  ElementRef,
-  EventEmitter,
-  Inject,
-  Input,
-  OnDestroy,
-  OnInit,
-  Output,
-  PLATFORM_ID,
-} from '@angular/core';
+import { Directive, ElementRef, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core';
 import { Observable, Subject } from 'rxjs';
 import { debounceTime, filter, takeUntil } from 'rxjs/operators';
 
@@ -31,10 +20,10 @@ export class IntersectionObserverDirective implements OnInit, OnDestroy {
 
   private destroy$ = new Subject<void>();
 
-  constructor(private element: ElementRef, @Inject(PLATFORM_ID) private platformId: string) {}
+  constructor(private element: ElementRef) {}
 
   ngOnInit() {
-    if (isPlatformBrowser(this.platformId)) {
+    if (!SSR) {
       const element = this.element.nativeElement;
       const config = {
         root: this.intersectionRoot,

diff --git a/src/app/core/facades/checkout.facade.ts b/src/app/core/facades/checkout.facade.ts
@@ -1,5 +1,4 @@
-import { isPlatformBrowser } from '@angular/common';
-import { Inject, Injectable, PLATFORM_ID } from '@angular/core';
+import { Injectable } from '@angular/core';
 import { Store, createSelector, select } from '@ngrx/store';
 import { Subject, combineLatest, merge } from 'rxjs';
 import { debounceTime, distinctUntilChanged, map, sample, switchMap, take, tap } from 'rxjs/operators';
@@ -57,8 +56,8 @@ import { whenFalsy, whenTruthy } from 'ish-core/utils/operators';
 export class CheckoutFacade {
   private basketChangeInternal$ = new Subject<void>();
 
-  constructor(private store: Store, @Inject(PLATFORM_ID) platformId: string) {
-    if (isPlatformBrowser(platformId)) {
+  constructor(private store: Store) {
+    if (!SSR) {
       this.store
         .pipe(
           select(getBasketLastTimeProductAdded),

diff --git a/src/app/core/guards/auth.guard.ts b/src/app/core/guards/auth.guard.ts
@@ -1,5 +1,4 @@
-import { isPlatformServer } from '@angular/common';
-import { Inject, Injectable, PLATFORM_ID } from '@angular/core';
+import { Injectable } from '@angular/core';
 import {
   ActivatedRouteSnapshot,
   CanActivate,
@@ -22,12 +21,7 @@ import { whenTruthy } from 'ish-core/utils/operators';
  */
 @Injectable({ providedIn: 'root' })
 export class AuthGuard implements CanActivate, CanActivateChild {
-  constructor(
-    private store: Store,
-    private router: Router,
-    @Inject(PLATFORM_ID) private platformId: string,
-    private cookieService: CookiesService
-  ) {}
+  constructor(private store: Store, private router: Router, private cookieService: CookiesService) {}
 
   canActivate(snapshot: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
     return this.guardAccess({ ...snapshot.data?.queryParams, ...snapshot.queryParams, returnUrl: state.url });
@@ -41,7 +35,7 @@ export class AuthGuard implements CanActivate, CanActivateChild {
     const defaultRedirect = this.router.createUrlTree(['/login'], { queryParams });
 
     return iif(
-      () => isPlatformServer(this.platformId),
+      () => SSR,
       // shortcut on ssr
       of(defaultRedirect),
       race(

diff --git a/src/app/core/identity-provider.module.ts b/src/app/core/identity-provider.module.ts
@@ -1,6 +1,5 @@
-import { isPlatformBrowser } from '@angular/common';
 import { HttpHandler, HttpRequest } from '@angular/common/http';
-import { ModuleWithProviders, NgModule, PLATFORM_ID } from '@angular/core';
+import { ModuleWithProviders, NgModule } from '@angular/core';
 import { OAuthModule, OAuthStorage } from 'angular-oauth2-oidc';
 import { noop } from 'rxjs';
 
@@ -15,16 +14,16 @@ import { IdentityProviderCapabilities } from './identity-provider/identity-provi
  * provider factory for storage
  * We need a factory, since localStorage is not available during AOT build time.
  */
-export function storageFactory(platformId: string): OAuthStorage {
-  if (isPlatformBrowser(platformId)) {
+export function storageFactory(): OAuthStorage {
+  if (!SSR) {
     return localStorage;
   }
 }
 
 @NgModule({
   imports: [OAuthModule.forRoot({ resourceServer: { sendAccessToken: false } }), PunchoutIdentityProviderModule],
   providers: [
-    { provide: OAuthStorage, useFactory: storageFactory, deps: [PLATFORM_ID] },
+    { provide: OAuthStorage, useFactory: storageFactory },
     {
       provide: IDENTITY_PROVIDER_IMPLEMENTOR,
       multi: true,

diff --git a/src/app/core/identity-provider/identity-provider.factory.ts b/src/app/core/identity-provider/identity-provider.factory.ts
@@ -1,5 +1,4 @@
-import { isPlatformBrowser } from '@angular/common';
-import { Inject, Injectable, InjectionToken, Injector, PLATFORM_ID, Type } from '@angular/core';
+import { Injectable, InjectionToken, Injector, Type } from '@angular/core';
 import { Store, select } from '@ngrx/store';
 import { noop } from 'rxjs';
 import { first } from 'rxjs/operators';
@@ -28,13 +27,8 @@ export class IdentityProviderFactory {
     type?: string;
   };
 
-  constructor(
-    private store: Store,
-    private injector: Injector,
-    private featureToggleService: FeatureToggleService,
-    @Inject(PLATFORM_ID) platformId: string
-  ) {
-    if (isPlatformBrowser(platformId)) {
+  constructor(private store: Store, private injector: Injector, private featureToggleService: FeatureToggleService) {
+    if (!SSR) {
       this.store.pipe(select(getIdentityProvider), whenTruthy(), first()).subscribe(config => {
         const provider = this.injector
           .get<IdentityProviderImplementor[]>(IDENTITY_PROVIDER_IMPLEMENTOR, [])

diff --git a/src/app/core/interceptors/icm-error-mapper.interceptor.ts b/src/app/core/interceptors/icm-error-mapper.interceptor.ts
@@ -1,6 +1,5 @@
-import { isPlatformBrowser } from '@angular/common';
 import { HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http';
-import { ErrorHandler, Inject, Injectable, InjectionToken, Injector, PLATFORM_ID } from '@angular/core';
+import { ErrorHandler, Injectable, InjectionToken, Injector } from '@angular/core';
 import { pick } from 'lodash-es';
 import { Observable, throwError } from 'rxjs';
 import { catchError } from 'rxjs/operators';
@@ -18,11 +17,7 @@ export const SPECIAL_HTTP_ERROR_HANDLER = new InjectionToken<SpecialHttpErrorHan
 
 @Injectable()
 export class ICMErrorMapperInterceptor implements HttpInterceptor {
-  constructor(
-    private injector: Injector,
-    @Inject(PLATFORM_ID) private platformId: string,
-    private errorHandler: ErrorHandler
-  ) {}
+  constructor(private injector: Injector, private errorHandler: ErrorHandler) {}
 
   // eslint-disable-next-line complexity
   private mapError(httpError: HttpErrorResponse, request: HttpRequest<unknown>): HttpError {
@@ -104,7 +99,7 @@ export class ICMErrorMapperInterceptor implements HttpInterceptor {
   intercept(req: HttpRequest<unknown>, next: HttpHandler): Observable<HttpEvent<unknown>> {
     return next.handle(req).pipe(
       catchError((error: HttpErrorResponse) => {
-        if (!isPlatformBrowser(this.platformId)) {
+        if (SSR) {
           this.errorHandler.handleError(error);
         }
         if (error.name === 'HttpErrorResponse') {

diff --git a/src/app/core/interceptors/universal-mock.interceptor.ts b/src/app/core/interceptors/universal-mock.interceptor.ts
@@ -1,6 +1,5 @@
-import { isPlatformServer } from '@angular/common';
 import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpResponse } from '@angular/common/http';
-import { Inject, Injectable, PLATFORM_ID } from '@angular/core';
+import { Injectable } from '@angular/core';
 import { existsSync, readFileSync } from 'fs';
 import { join } from 'path';
 import { Observable, Observer } from 'rxjs';
@@ -10,10 +9,8 @@ import { Observable, Observer } from 'rxjs';
  */
 @Injectable()
 export class UniversalMockInterceptor implements HttpInterceptor {
-  constructor(@Inject(PLATFORM_ID) private platformId: string) {}
-
   intercept(req: HttpRequest<unknown>, next: HttpHandler): Observable<HttpEvent<unknown>> {
-    if (!isPlatformServer(this.platformId)) {
+    if (!SSR) {
       console.warn('UniversalMockInterceptor is active for non-server platform');
     }
     if (!req.url.startsWith('http')) {

diff --git a/src/app/core/store/core/configuration/configuration.effects.ts b/src/app/core/store/core/configuration/configuration.effects.ts
@@ -1,5 +1,5 @@
-import { DOCUMENT, isPlatformBrowser, isPlatformServer } from '@angular/common';
-import { ApplicationRef, Inject, Injectable, PLATFORM_ID, isDevMode } from '@angular/core';
+import { DOCUMENT } from '@angular/common';
+import { ApplicationRef, Inject, Injectable, isDevMode } from '@angular/core';
 import { TransferState } from '@angular/platform-browser';
 import { Actions, ROOT_EFFECTS_INIT, createEffect, ofType } from '@ngrx/effects';
 import { Store, select } from '@ngrx/store';
@@ -29,7 +29,6 @@ export class ConfigurationEffects {
     private store: Store,
     private stateProperties: StatePropertiesService,
     private transferState: TransferState,
-    @Inject(PLATFORM_ID) private platformId: string,
     @Inject(MEDIUM_BREAKPOINT_WIDTH) private mediumBreakpointWidth: number,
     @Inject(LARGE_BREAKPOINT_WIDTH) private largeBreakpointWidth: number,
     @Inject(DOCUMENT) document: Document,
@@ -38,13 +37,13 @@ export class ConfigurationEffects {
     private localizationsService: LocalizationsService
   ) {
     appRef.isStable
-      .pipe(takeWhile(() => isPlatformBrowser(platformId)))
+      .pipe(takeWhile(() => !SSR))
       // eslint-disable-next-line @typescript-eslint/no-explicit-any -- window can only be used with any here
       .subscribe(stable => ((window as any).angularStable = stable));
 
     store
       .pipe(
-        takeWhile(() => isPlatformServer(this.platformId) || isDevMode()),
+        takeWhile(() => SSR || isDevMode()),
         select(getCurrentLocale),
         whenTruthy(),
         distinctUntilChanged()
@@ -121,7 +120,7 @@ export class ConfigurationEffects {
 
   setDeviceType$ = createEffect(() =>
     iif(
-      () => isPlatformBrowser(this.platformId),
+      () => !SSR,
       defer(() =>
         merge(this.actions$.pipe(ofType(ROOT_EFFECTS_INIT)), fromEvent(window, 'resize')).pipe(
           map<unknown, DeviceType>(() => {

diff --git a/src/app/core/store/core/server-config/server-config.effects.ts b/src/app/core/store/core/server-config/server-config.effects.ts
@@ -1,5 +1,4 @@
-import { isPlatformServer } from '@angular/common';
-import { Inject, Injectable, PLATFORM_ID } from '@angular/core';
+import { Injectable } from '@angular/core';
 import { Actions, createEffect, ofType } from '@ngrx/effects';
 import { routerNavigationAction } from '@ngrx/router-store';
 import { Store, select } from '@ngrx/store';
@@ -14,20 +13,15 @@ import { isServerConfigurationLoaded } from './server-config.selectors';
 
 @Injectable()
 export class ServerConfigEffects {
-  constructor(
-    private actions$: Actions,
-    private store: Store,
-    private configService: ConfigurationService,
-    @Inject(PLATFORM_ID) private platformId: string
-  ) {}
+  constructor(private actions$: Actions, private store: Store, private configService: ConfigurationService) {}
 
   /**
    * get server configuration on routing event, if it is not already loaded
    */
   loadServerConfigOnInit$ = createEffect(() =>
     this.actions$.pipe(
       ofType(routerNavigationAction),
-      isPlatformServer(this.platformId) ? first() : identity,
+      SSR ? first() : identity,
       switchMap(() => this.store.pipe(select(isServerConfigurationLoaded))),
       whenFalsy(),
       map(() => loadServerConfig())