fix(common): use === operator to match NgSwitch cases (#51504)

pkozlowski-opensource · atscott · commit 28a5925f5379 · 2023-10-09T10:10:21.000-07:00
This change adjust the equality comparator used by NgSwitch - now it defaults to === from previously used ==. This change is based on the following reasoning: - align behaviour with the built-in switch block); - improve performance (avoid type coercion); - enable better type-checking. BREAKING CHANGE: the NgSwitch directive now defaults to the === equality operator, migrating from the previously used ==. NgSwitch expressions and / or individual condition values need adjusting to this stricter equality check. The added warning message should help pinpointing NgSwitch usages where adjustements are needed. Fixes #33873 PR Close #51504
diff --git a/goldens/public-api/common/errors.md b/goldens/public-api/common/errors.md
@@ -6,6 +6,8 @@
 
 // @public
 export const enum RuntimeErrorCode {
+    // (undocumented)
+    EQUALITY_NG_SWITCH_DIFFERENCE = 2001,
     // (undocumented)
     INVALID_INPUT = 2952,
     // (undocumented)
diff --git a/packages/common/src/directives/ng_switch.ts b/packages/common/src/directives/ng_switch.ts
@@ -6,10 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Directive, DoCheck, Host, Input, Optional, TemplateRef, ViewContainerRef, ɵRuntimeError as RuntimeError} from '@angular/core';
+import {Directive, DoCheck, Host, Input, Optional, TemplateRef, ViewContainerRef, ɵformatRuntimeError as formatRuntimeError, ɵRuntimeError as RuntimeError} from '@angular/core';
 
 import {RuntimeErrorCode} from '../errors';
 
+import {NG_SWITCH_USE_STRICT_EQUALS} from './ng_switch_equality';
+
 export class SwitchView {
   private _created = false;
 
@@ -133,7 +135,18 @@ export class NgSwitch {
 
   /** @internal */
   _matchCase(value: any): boolean {
-    const matched = value == this._ngSwitch;
+    const matched =
+        NG_SWITCH_USE_STRICT_EQUALS ? value === this._ngSwitch : value == this._ngSwitch;
+    if ((typeof ngDevMode === 'undefined' || ngDevMode) && matched !== (value == this._ngSwitch)) {
+      console.warn(formatRuntimeError(
+          RuntimeErrorCode.EQUALITY_NG_SWITCH_DIFFERENCE,
+          'As of Angular v17 the NgSwitch directive uses strict equality comparison === instead of == to match different cases. ' +
+              `Previously the case value "${
+                  stringifyValue(value)}" matched switch expression value "${
+                  stringifyValue(
+                      this._ngSwitch)}", but this is no longer the case with the stricter equality check.` +
+              'Your comparison results return different results using === vs. == and you should adjust your ngSwitch expression and / or values to conform with the strict equality requirements.'));
+    }
     this._lastCasesMatched = this._lastCasesMatched || matched;
     this._lastCaseCheckIndex++;
     if (this._lastCaseCheckIndex === this._caseCount) {
@@ -256,3 +269,7 @@ function throwNgSwitchProviderNotFoundError(attrName: string, directiveName: str
               directiveName}" directive) must be located inside an element with the "ngSwitch" attribute ` +
           `(matching "NgSwitch" directive)`);
 }
+
+function stringifyValue(value: unknown): string {
+  return typeof value === 'string' ? `'${value}'` : String(value);
+}
diff --git a/packages/common/src/directives/ng_switch_equality.ts b/packages/common/src/directives/ng_switch_equality.ts
@@ -0,0 +1,14 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+
+/**
+ * A constant indicating a type of comparison that NgSwitch uses to match cases. Extracted to a
+ * separate file to facilitate G3 patches.
+ */
+export const NG_SWITCH_USE_STRICT_EQUALS = true;
diff --git a/packages/common/src/errors.ts b/packages/common/src/errors.ts
@@ -13,6 +13,7 @@
 export const enum RuntimeErrorCode {
   // NgSwitch errors
   PARENT_NG_SWITCH_NOT_FOUND = 2000,
+  EQUALITY_NG_SWITCH_DIFFERENCE = 2001,
 
   // Pipe errors
   INVALID_PIPE_ARGUMENT = 2100,
diff --git a/packages/common/test/directives/ng_switch_spec.ts b/packages/common/test/directives/ng_switch_spec.ts
@@ -91,6 +91,48 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
         getComponent().switchValue = 'b';
         detectChangesAndExpectText('when b1;when b2;');
       });
+
+      it('should use === to match cases', () => {
+        const template = '<ul [ngSwitch]="switchValue">' +
+            '<li *ngSwitchCase="1">when one</li>' +
+            '<li *ngSwitchDefault>when default</li>' +
+            '</ul>';
+
+        fixture = createTestComponent(template);
+        detectChangesAndExpectText('when default');
+
+        getComponent().switchValue = 1;
+        detectChangesAndExpectText('when one');
+
+        getComponent().switchValue = '1';
+        detectChangesAndExpectText('when default');
+      });
+
+      it('should warn if === and == give different results', () => {
+        const template = '<ul [ngSwitch]="switchValue">' +
+            '<li *ngSwitchCase="1">when one</li>' +
+            '<li *ngSwitchDefault>when default</li>' +
+            '</ul>';
+
+        const consoleWarnSpy = spyOn(console, 'warn');
+
+        fixture = createTestComponent(template);
+        getComponent().switchValue = '1';
+        detectChangesAndExpectText('when default');
+
+        expect(consoleWarnSpy.calls.count()).toBe(1);
+        expect(consoleWarnSpy.calls.argsFor(0)[0])
+            .toBe(
+                'NG02001: As of Angular v17 the NgSwitch directive uses strict equality comparison === instead of == to match different cases. ' +
+                `Previously the case value "1" matched switch expression value "'1'", but this is no longer the case with the stricter equality check.` +
+                'Your comparison results return different results using === vs. == and you should adjust your ngSwitch expression and / or values to conform with the strict equality requirements.');
+
+
+        getComponent().switchValue = 1;
+        detectChangesAndExpectText('when one');
+        expect(consoleWarnSpy.calls.count())
+            .toBe(1);  // no calls to warn when both equality operators agree
+      });
     });
 
     describe('when values changes', () => {
