From 215ef3c5f4a99b4b88a45f292c623b6ee4f4af4f Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Mon, 22 Jul 2019 14:44:23 -0700 Subject: [PATCH] fix(ivy): ensure NgClass does not overwrite other dir data (#31788) We currently have a handwritten version of the Ivy directive def for NgClass so we can switch between Ivy and View Engine behavior. This generated code needs to be kept up-to-date with what the Ivy compiler generates. PR 30742 recently changed `classMap` such that it now requires allocation of host binding slots. This means that the `allocHostVars()` function must be called in the NgClass directive def to match compiler output, but the handwritten directive def was not updated. This caused a bug where NgClass was inappropriately overwriting data for other directives because space was not allocated for its values. PR Close #31788 --- packages/common/src/directives/ng_class.ts | 3 +- .../r3_view_compiler_styling_spec.ts | 2 + packages/core/test/acceptance/styling_spec.ts | 52 ++++++++++++++++++- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/packages/common/src/directives/ng_class.ts b/packages/common/src/directives/ng_class.ts index 43fe9d5551..61f42cefd4 100644 --- a/packages/common/src/directives/ng_class.ts +++ b/packages/common/src/directives/ng_class.ts @@ -5,7 +5,7 @@ * 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 */ -import {Directive, DoCheck, Input, ɵRenderFlags, ɵɵclassMap, ɵɵdefineDirective, ɵɵstyling, ɵɵstylingApply} from '@angular/core'; +import {Directive, DoCheck, Input, ɵRenderFlags, ɵɵallocHostVars, ɵɵclassMap, ɵɵdefineDirective, ɵɵstyling, ɵɵstylingApply} from '@angular/core'; import {NgClassImpl, NgClassImplProvider} from './ng_class_impl'; @@ -35,6 +35,7 @@ export const ngClassDirectiveDef__POST_R3__ = ɵɵdefineDirective({ factory: () => {}, hostBindings: function(rf: ɵRenderFlags, ctx: any, elIndex: number) { if (rf & ɵRenderFlags.Create) { + ɵɵallocHostVars(1); ɵɵstyling(); } if (rf & ɵRenderFlags.Update) { diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts index 01bc7c9619..b452bf516c 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts @@ -1278,6 +1278,8 @@ describe('compiler compliance: styling', () => { } }; + // NOTE: IF YOU ARE CHANGING THIS COMPILER SPEC, YOU MAY NEED TO CHANGE THE DIRECTIVE + // DEF THAT'S HARD-CODED IN `ng_class.ts`. const template = ` function ClassDirective_HostBindings(rf, ctx, elIndex) { if (rf & 1) { diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index 40b3664634..106a175173 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -8,7 +8,7 @@ import {Component, Directive, ElementRef, Input} from '@angular/core'; import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed} from '@angular/core/testing'; -import {DomSanitizer, SafeStyle} from '@angular/platform-browser'; +import {By, DomSanitizer, SafeStyle} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; @@ -554,4 +554,54 @@ describe('styling', () => { expect(capturedMyClassBindingCount).toEqual(1); expect(capturedMyClassBindingValue !).toEqual('foo'); }); + + describe('NgClass', () => { + + // We had a bug where NgClass would not allocate sufficient slots for host bindings, + // so it would overwrite information about other directives nearby. This test checks + // that TestDir's injector is not overwritten by NgClass, so TestDir should still + // be found by DI when ChildDir is instantiated. + it('should not overwrite other directive info when using NgClass', () => { + @Directive({selector: '[test-dir]'}) + class TestDir { + } + + @Directive({selector: '[child-dir]'}) + class ChildDir { + constructor(public parent: TestDir) {} + } + + @Component({ + selector: 'app', + template: ` +
+
Hello
+
+ ` + }) + class AppComponent { + classMap = {'with-button': true}; + showing = false; + } + + TestBed.configureTestingModule({declarations: [AppComponent, TestDir, ChildDir]}); + const fixture = TestBed.createComponent(AppComponent); + fixture.detectChanges(); + const testDirDiv = fixture.debugElement.nativeElement.querySelector('div'); + expect(testDirDiv.classList).toContain('with-button'); + expect(fixture.debugElement.nativeElement.textContent).not.toContain('Hello'); + + fixture.componentInstance.classMap = {'with-button': false}; + fixture.componentInstance.showing = true; + fixture.detectChanges(); + + const childDir = fixture.debugElement.query(By.directive(ChildDir)).injector.get(ChildDir); + expect(childDir.parent).toBeAnInstanceOf(TestDir); + expect(testDirDiv.classList).not.toContain('with-button'); + expect(fixture.debugElement.nativeElement.textContent).toContain('Hello'); + + }); + + + }); });