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
This commit is contained in:
Kara Erickson 2019-07-22 14:44:23 -07:00 committed by Miško Hevery
parent 5a8eb924ba
commit 215ef3c5f4
3 changed files with 55 additions and 2 deletions

View File

@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be * 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 * 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'; import {NgClassImpl, NgClassImplProvider} from './ng_class_impl';
@ -35,6 +35,7 @@ export const ngClassDirectiveDef__POST_R3__ = ɵɵdefineDirective({
factory: () => {}, factory: () => {},
hostBindings: function(rf: ɵRenderFlags, ctx: any, elIndex: number) { hostBindings: function(rf: ɵRenderFlags, ctx: any, elIndex: number) {
if (rf & ɵRenderFlags.Create) { if (rf & ɵRenderFlags.Create) {
ɵɵallocHostVars(1);
ɵɵstyling(); ɵɵstyling();
} }
if (rf & ɵRenderFlags.Update) { if (rf & ɵRenderFlags.Update) {

View File

@ -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 = ` const template = `
function ClassDirective_HostBindings(rf, ctx, elIndex) { function ClassDirective_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {

View File

@ -8,7 +8,7 @@
import {Component, Directive, ElementRef, Input} from '@angular/core'; import {Component, Directive, ElementRef, Input} from '@angular/core';
import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode';
import {TestBed} from '@angular/core/testing'; 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 {expect} from '@angular/platform-browser/testing/src/matchers';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
@ -554,4 +554,54 @@ describe('styling', () => {
expect(capturedMyClassBindingCount).toEqual(1); expect(capturedMyClassBindingCount).toEqual(1);
expect(capturedMyClassBindingValue !).toEqual('foo'); 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: `
<div class="my-class" [ngClass]="classMap" test-dir>
<div *ngIf="showing" child-dir>Hello</div>
</div>
`
})
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');
});
});
}); });