fix(common): `NgSwitch` - don’t create the default case if another case matches (#12726)

This also simplifies the implementation of `NgSwitch`.

Closes #11297
Closes #9420

modules/@angular/common/src/directives/ng_switch.ts

@@ -6,19 +6,31 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Directive, Host, Input, TemplateRef, ViewContainerRef} from '@angular/core';
-
-import {ListWrapper} from '../facade/collection';
-
-const _CASE_DEFAULT = {};
+import {Directive, DoCheck, Host, Input, TemplateRef, ViewContainerRef} from '@angular/core';
 
 export class SwitchView {
+  private _created = false;
+
   constructor(
       private _viewContainerRef: ViewContainerRef, private _templateRef: TemplateRef<Object>) {}
 
-  create(): void { this._viewContainerRef.createEmbeddedView(this._templateRef); }
+  create(): void {
+    this._created = true;
+    this._viewContainerRef.createEmbeddedView(this._templateRef);
+  }
 
-  destroy(): void { this._viewContainerRef.clear(); }
+  destroy(): void {
+    this._created = false;
+    this._viewContainerRef.clear();
+  }
+
+  enforceState(created: boolean) {
+    if (created && !this._created) {
+      this.create();
+    } else if (!created && this._created) {
+      this.destroy();
+    }
+  }
 }
 
 /**
@@ -64,92 +76,52 @@ export class SwitchView {
  */
 @Directive({selector: '[ngSwitch]'})
 export class NgSwitch {
-  private _switchValue: any;
-  private _useDefault: boolean = false;
-  private _valueViews = new Map<any, SwitchView[]>();
-  private _activeViews: SwitchView[] = [];
+  private _defaultViews: SwitchView[];
+  private _defaultUsed = false;
+  private _caseCount = 0;
+  private _lastCaseCheckIndex = 0;
+  private _lastCasesMatched = false;
+  private _ngSwitch: any;
 
   @Input()
-  set ngSwitch(value: any) {
-    // Set of views to display for this value
-    let views = this._valueViews.get(value);
-
-    if (views) {
-      this._useDefault = false;
-    } else {
-      // No view to display for the current value -> default case
-      // Nothing to do if the default case was already active
-      if (this._useDefault) {
-        return;
-      }
-      this._useDefault = true;
-      views = this._valueViews.get(_CASE_DEFAULT);
-    }
-
-    this._emptyAllActiveViews();
-    this._activateViews(views);
-    this._switchValue = value;
-  }
-
-  /** @internal */
-  _onCaseValueChanged(oldCase: any, newCase: any, view: SwitchView): void {
-    this._deregisterView(oldCase, view);
-    this._registerView(newCase, view);
-
-    if (oldCase === this._switchValue) {
-      view.destroy();
-      ListWrapper.remove(this._activeViews, view);
-    } else if (newCase === this._switchValue) {
-      if (this._useDefault) {
-        this._useDefault = false;
-        this._emptyAllActiveViews();
-      }
-      view.create();
-      this._activeViews.push(view);
-    }
-
-    // Switch to default when there is no more active ViewContainers
-    if (this._activeViews.length === 0 && !this._useDefault) {
-      this._useDefault = true;
-      this._activateViews(this._valueViews.get(_CASE_DEFAULT));
-    }
-  }
-
-  private _emptyAllActiveViews(): void {
-    const activeContainers = this._activeViews;
-    for (var i = 0; i < activeContainers.length; i++) {
-      activeContainers[i].destroy();
-    }
-    this._activeViews = [];
-  }
-
-  private _activateViews(views?: SwitchView[]): void {
-    if (views) {
-      for (var i = 0; i < views.length; i++) {
-        views[i].create();
-      }
-      this._activeViews = views;
+  set ngSwitch(newValue: any) {
+    this._ngSwitch = newValue;
+    if (this._caseCount === 0) {
+      this._updateDefaultCases(true);
     }
   }
 
   /** @internal */
-  _registerView(value: any, view: SwitchView): void {
-    let views = this._valueViews.get(value);
-    if (!views) {
-      views = [];
-      this._valueViews.set(value, views);
+  _addCase(): number { return this._caseCount++; }
+
+  /** @internal */
+  _addDefault(view: SwitchView) {
+    if (!this._defaultViews) {
+      this._defaultViews = [];
     }
-    views.push(view);
+    this._defaultViews.push(view);
   }
 
-  private _deregisterView(value: any, view: SwitchView): void {
-    // `_CASE_DEFAULT` is used a marker for non-registered cases
-    if (value === _CASE_DEFAULT) return;
-    const views = this._valueViews.get(value);
-    if (views.length == 1) {
-      this._valueViews.delete(value);
-    } else {
-      ListWrapper.remove(views, view);
+  /** @internal */
+  _matchCase(value: any): boolean {
+    const matched = value == this._ngSwitch;
+    this._lastCasesMatched = this._lastCasesMatched || matched;
+    this._lastCaseCheckIndex++;
+    if (this._lastCaseCheckIndex === this._caseCount) {
+      this._updateDefaultCases(!this._lastCasesMatched);
+      this._lastCaseCheckIndex = 0;
+      this._lastCasesMatched = false;
+    }
+    return matched;
+  }
+
+  private _updateDefaultCases(useDefault: boolean) {
+    if (this._defaultViews && useDefault !== this._defaultUsed) {
+      this._defaultUsed = useDefault;
+      for (var i = 0; i < this._defaultViews.length; i++) {
+        const defaultView = this._defaultViews[i];
+        defaultView.enforceState(useDefault);
+      }
     }
   }
 }
@@ -179,24 +151,20 @@ export class NgSwitch {
  * @stable
  */
 @Directive({selector: '[ngSwitchCase]'})
-export class NgSwitchCase {
-  // `_CASE_DEFAULT` is used as a marker for a not yet initialized value
-  private _value: any = _CASE_DEFAULT;
+export class NgSwitchCase implements DoCheck {
   private _view: SwitchView;
-  private _switch: NgSwitch;
+
+  @Input()
+  ngSwitchCase: any;
 
   constructor(
       viewContainer: ViewContainerRef, templateRef: TemplateRef<Object>,
-      @Host() ngSwitch: NgSwitch) {
-    this._switch = ngSwitch;
+      @Host() private ngSwitch: NgSwitch) {
+    ngSwitch._addCase();
     this._view = new SwitchView(viewContainer, templateRef);
   }
 
-  @Input()
-  set ngSwitchCase(value: any) {
-    this._switch._onCaseValueChanged(this._value, value, this._view);
-    this._value = value;
-  }
+  ngDoCheck() { this._view.enforceState(this.ngSwitch._matchCase(this.ngSwitchCase)); }
 }
 
 /**
@@ -226,7 +194,7 @@ export class NgSwitchCase {
 export class NgSwitchDefault {
   constructor(
       viewContainer: ViewContainerRef, templateRef: TemplateRef<Object>,
-      @Host() sswitch: NgSwitch) {
-    sswitch._registerView(_CASE_DEFAULT, new SwitchView(viewContainer, templateRef));
+      @Host() ngSwitch: NgSwitch) {
+    ngSwitch._addDefault(new SwitchView(viewContainer, templateRef));
   }
 }

modules/@angular/common/test/directives/ng_switch_spec.ts

@@ -7,8 +7,8 @@
  */
 
 import {CommonModule} from '@angular/common';
-import {Component} from '@angular/core';
-import {ComponentFixture, TestBed, async} from '@angular/core/testing';
+import {Attribute, Component, Directive} from '@angular/core';
+import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/matchers';
 
 export function main() {
@@ -32,7 +32,7 @@ export function main() {
     });
 
     describe('switch value changes', () => {
-      it('should switch amongst when values', async(() => {
+      it('should switch amongst when values', () => {
         const template = '<div>' +
             '<ul [ngSwitch]="switchValue">' +
             '<template ngSwitchCase="a"><li>when a</li></template>' +
@@ -48,9 +48,9 @@ export function main() {
 
         getComponent().switchValue = 'b';
         detectChangesAndExpectText('when b');
-         }));
+      });
 
-      it('should switch amongst when values with fallback to default', async(() => {
+      it('should switch amongst when values with fallback to default', () => {
         const template = '<div>' +
             '<ul [ngSwitch]="switchValue">' +
             '<li template="ngSwitchCase \'a\'">when a</li>' +
@@ -68,9 +68,9 @@ export function main() {
 
         getComponent().switchValue = 'c';
         detectChangesAndExpectText('when default');
-         }));
+      });
 
-      it('should support multiple whens with the same value', async(() => {
+      it('should support multiple whens with the same value', () => {
         const template = '<div>' +
             '<ul [ngSwitch]="switchValue">' +
             '<template ngSwitchCase="a"><li>when a1;</li></template>' +
@@ -89,11 +89,11 @@ export function main() {
 
         getComponent().switchValue = 'b';
         detectChangesAndExpectText('when b1;when b2;');
-         }));
+      });
     });
 
     describe('when values changes', () => {
-      it('should switch amongst when values', async(() => {
+      it('should switch amongst when values', () => {
         const template = '<div>' +
             '<ul [ngSwitch]="switchValue">' +
             '<template [ngSwitchCase]="when1"><li>when 1;</li></template>' +
@@ -118,7 +118,67 @@ export function main() {
 
         getComponent().when1 = 'd';
         detectChangesAndExpectText('when default;');
-         }));
+      });
+    });
+
+    describe('corner cases', () => {
+
+      it('should not create the default case if another case matches', () => {
+        const log: string[] = [];
+
+        @Directive({selector: '[test]'})
+        class TestDirective {
+          constructor(@Attribute('test') test: string) { log.push(test); }
+        }
+
+        const template = '<div [ngSwitch]="switchValue">' +
+            '<div *ngSwitchCase="\'a\'" test="aCase"></div>' +
+            '<div *ngSwitchDefault test="defaultCase"></div>' +
+            '</div>';
+
+        TestBed.configureTestingModule({declarations: [TestDirective]});
+        TestBed.overrideComponent(TestComponent, {set: {template: template}})
+            .createComponent(TestComponent);
+        const fixture = TestBed.createComponent(TestComponent);
+        fixture.componentInstance.switchValue = 'a';
+
+        fixture.detectChanges();
+
+        expect(log).toEqual(['aCase']);
+      });
+
+      it('should create the default case if there is no other case', () => {
+        const template = '<div>' +
+            '<ul [ngSwitch]="switchValue">' +
+            '<template ngSwitchDefault><li>when default1;</li></template>' +
+            '<template ngSwitchDefault><li>when default2;</li></template>' +
+            '</ul></div>';
+
+        fixture = createTestComponent(template);
+        detectChangesAndExpectText('when default1;when default2;');
+
+      });
+
+      it('should allow defaults before cases', () => {
+        const template = '<div>' +
+            '<ul [ngSwitch]="switchValue">' +
+            '<template ngSwitchDefault><li>when default1;</li></template>' +
+            '<template ngSwitchDefault><li>when default2;</li></template>' +
+            '<template ngSwitchCase="a"><li>when a1;</li></template>' +
+            '<template ngSwitchCase="b"><li>when b1;</li></template>' +
+            '<template ngSwitchCase="a"><li>when a2;</li></template>' +
+            '<template ngSwitchCase="b"><li>when b2;</li></template>' +
+            '</ul></div>';
+
+        fixture = createTestComponent(template);
+        detectChangesAndExpectText('when default1;when default2;');
+
+        getComponent().switchValue = 'a';
+        detectChangesAndExpectText('when a1;when a2;');
+
+        getComponent().switchValue = 'b';
+        detectChangesAndExpectText('when b1;when b2;');
+      });
     });
   });
 }

tools/public_api_guard/common/index.d.ts

@@ -166,14 +166,15 @@ export declare class NgSwitch {
 }
 
 /** @stable */
-export declare class NgSwitchCase {
+export declare class NgSwitchCase implements DoCheck {
     ngSwitchCase: any;
     constructor(viewContainer: ViewContainerRef, templateRef: TemplateRef<Object>, ngSwitch: NgSwitch);
+    ngDoCheck(): void;
 }
 
 /** @stable */
 export declare class NgSwitchDefault {
-    constructor(viewContainer: ViewContainerRef, templateRef: TemplateRef<Object>, sswitch: NgSwitch);
+    constructor(viewContainer: ViewContainerRef, templateRef: TemplateRef<Object>, ngSwitch: NgSwitch);
 }
 
 /** @experimental */