From 46aec4a58f08a9a0d0600a3e0188f35c2ec1764b Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 22 Jan 2019 15:02:52 -0800 Subject: [PATCH] feat(ivy): support host properties in DebugElement.properties (#28355) DebugElement.properties should contain a map of element property names to element property values, with entries for both normal property bindings and host bindings. Many Angular core tests depend on this map being present. This commit adds support for host property bindings in DebugElement.properties, which fixes the Angular core tests. There is still work to be done for normal property bindings. PR Close #28355 --- packages/core/src/debug/debug_node.ts | 31 ++- packages/core/src/render3/instructions.ts | 8 + packages/core/src/render3/interfaces/view.ts | 5 +- packages/core/test/linker/integration_spec.ts | 34 ++- .../test/linker/ng_module_integration_spec.ts | 252 +++++++++--------- packages/core/test/test_bed_spec.ts | 21 +- .../test/testing_public_spec.ts | 37 ++- 7 files changed, 213 insertions(+), 175 deletions(-) diff --git a/packages/core/src/debug/debug_node.ts b/packages/core/src/debug/debug_node.ts index de2feda2c9..c31e082490 100644 --- a/packages/core/src/debug/debug_node.ts +++ b/packages/core/src/debug/debug_node.ts @@ -239,14 +239,35 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme get name(): string { return this.nativeElement !.nodeName; } + /** + * Returns a map of property names to property values for an element. + * + * This map includes: + * - Regular property bindings (e.g. `[id]="id"`) - TODO + * - Host property bindings (e.g. `host: { '[id]': "id" }`) + * - Interpolated property bindings (e.g. `id="{{ value }}") - TODO + * + * It should NOT include input property bindings or attribute bindings. + */ get properties(): {[key: string]: any;} { const context = loadLContext(this.nativeNode) !; const lView = context.lView; - const tView = lView[TVIEW]; - const tNode = tView.data[context.nodeIndex] as TNode; - const properties = {}; - // TODO: https://angular-team.atlassian.net/browse/FW-681 - // Missing implementation here... + const tData = lView[TVIEW].data; + const tNode = tData[context.nodeIndex] as TNode; + + // TODO(kara): include regular property binding values (not just host properties) + const properties: {[key: string]: string} = {}; + + // Host binding values for a node are stored after directives on that node + let index = tNode.directiveEnd; + let propertyName = tData[index]; + + // When we reach a value in TView.data that is not a string, we know we've + // hit the next node's providers and directives and should stop copying data. + while (typeof propertyName === 'string') { + properties[propertyName] = lView[index]; + propertyName = tData[++index]; + } return properties; } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index a4e496bed0..5dc30ac629 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -1154,6 +1154,14 @@ function elementPropertyInternal( validateProperty(propName); ngDevMode.rendererSetProperty++; } + const tView = lView[TVIEW]; + const lastBindingIndex = lView[BINDING_INDEX] - 1; + if (nativeOnly && tView.data[lastBindingIndex] == null) { + // We need to store the host property name so it can be accessed by DebugElement.properties. + // Host properties cannot have interpolations, so using the last binding index is + // sufficient. + tView.data[lastBindingIndex] = propName; + } const renderer = loadRendererFn ? loadRendererFn(tNode, lView) : lView[RENDERER]; // It is assumed that the sanitizer is only added when the compiler determines that the property // is risky, so sanitization can be done without further checks. diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 2196950b38..6ac11c11ac 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -556,11 +556,14 @@ export type HookData = (number | (() => void))[]; * Each pipe's definition is stored here at the same index as its pipe instance in * the data array. * + * Each host property's name is stored here at the same index as its value in the + * data array. + * * Injector bloom filters are also stored here. */ export type TData = (TNode | PipeDef| DirectiveDef| ComponentDef| number | Type| - InjectionToken| TI18n | I18nUpdateOpCodes | null)[]; + InjectionToken| TI18n | I18nUpdateOpCodes | null | string)[]; // Note: This hack is necessary so we don't erroneously get a circular dependency // failure based on types. diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index 90e11bdd4b..9205e85b3e 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -924,26 +924,24 @@ function declareTests(config?: {useJit: boolean}) { expect(getDOM().getProperty(tc.nativeElement, 'id')).toEqual('newId'); }); - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should not use template variables for expressions in hostProperties', () => { - @Directive( - {selector: '[host-properties]', host: {'[id]': 'id', '[title]': 'unknownProp'}}) - class DirectiveWithHostProps { - id = 'one'; - } + it('should not use template variables for expressions in hostProperties', () => { + @Directive({selector: '[host-properties]', host: {'[id]': 'id', '[title]': 'unknownProp'}}) + class DirectiveWithHostProps { + id = 'one'; + } - const fixture = - TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostProps]}) - .overrideComponent(MyComp, { - set: {template: `
`} - }) - .createComponent(MyComp); - fixture.detectChanges(); + const fixture = + TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostProps]}) + .overrideComponent( + MyComp, + {set: {template: `
`}}) + .createComponent(MyComp); + fixture.detectChanges(); - const tc = fixture.debugElement.children[0]; - expect(tc.properties['id']).toBe('one'); - expect(tc.properties['title']).toBe(undefined); - }); + const tc = fixture.debugElement.children[0]; + expect(tc.properties['id']).toBe('one'); + expect(tc.properties['title']).toBe(undefined); + }); fixmeIvy('FW-725: Pipes in host bindings fail with a cryptic error') .it('should not allow pipes in hostProperties', () => { diff --git a/packages/core/test/linker/ng_module_integration_spec.ts b/packages/core/test/linker/ng_module_integration_spec.ts index 53512bdde1..bbc3bdc15b 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -423,163 +423,155 @@ function declareTests(config?: {useJit: boolean}) { }); describe('directives and pipes', () => { - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .describe('declarations', () => { - it('should be supported in root modules', () => { - @NgModule({ - declarations: [CompUsingModuleDirectiveAndPipe, SomeDirective, SomePipe], - entryComponents: [CompUsingModuleDirectiveAndPipe] - }) - class SomeModule { - } + describe('declarations', () => { + it('should be supported in root modules', () => { + @NgModule({ + declarations: [CompUsingModuleDirectiveAndPipe, SomeDirective, SomePipe], + entryComponents: [CompUsingModuleDirectiveAndPipe] + }) + class SomeModule { + } - const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); + const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].properties['title']) - .toBe('transformed someValue'); - }); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); + }); - it('should be supported in imported modules', () => { - @NgModule({ - declarations: [CompUsingModuleDirectiveAndPipe, SomeDirective, SomePipe], - entryComponents: [CompUsingModuleDirectiveAndPipe] - }) - class SomeImportedModule { - } + it('should be supported in imported modules', () => { + @NgModule({ + declarations: [CompUsingModuleDirectiveAndPipe, SomeDirective, SomePipe], + entryComponents: [CompUsingModuleDirectiveAndPipe] + }) + class SomeImportedModule { + } - @NgModule({imports: [SomeImportedModule]}) - class SomeModule { - } + @NgModule({imports: [SomeImportedModule]}) + class SomeModule { + } - const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].properties['title']) - .toBe('transformed someValue'); - }); + const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); + }); - it('should be supported in nested components', () => { - @Component({ - selector: 'parent', - template: '', - }) - class ParentCompUsingModuleDirectiveAndPipe { - } + it('should be supported in nested components', () => { + @Component({ + selector: 'parent', + template: '', + }) + class ParentCompUsingModuleDirectiveAndPipe { + } - @NgModule({ - declarations: [ - ParentCompUsingModuleDirectiveAndPipe, CompUsingModuleDirectiveAndPipe, - SomeDirective, SomePipe - ], - entryComponents: [ParentCompUsingModuleDirectiveAndPipe] - }) - class SomeModule { - } + @NgModule({ + declarations: [ + ParentCompUsingModuleDirectiveAndPipe, CompUsingModuleDirectiveAndPipe, SomeDirective, + SomePipe + ], + entryComponents: [ParentCompUsingModuleDirectiveAndPipe] + }) + class SomeModule { + } - const compFixture = createComp(ParentCompUsingModuleDirectiveAndPipe, SomeModule); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].children[0].properties['title']) - .toBe('transformed someValue'); - }); - }); + const compFixture = createComp(ParentCompUsingModuleDirectiveAndPipe, SomeModule); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].children[0].properties['title']) + .toBe('transformed someValue'); + }); + }); describe('import/export', () => { - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should support exported directives and pipes', () => { - @NgModule( - {declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) - class SomeImportedModule { - } + it('should support exported directives and pipes', () => { + @NgModule({declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) + class SomeImportedModule { + } - @NgModule({ - declarations: [CompUsingModuleDirectiveAndPipe], - imports: [SomeImportedModule], - entryComponents: [CompUsingModuleDirectiveAndPipe] - }) - class SomeModule { - } + @NgModule({ + declarations: [CompUsingModuleDirectiveAndPipe], + imports: [SomeImportedModule], + entryComponents: [CompUsingModuleDirectiveAndPipe] + }) + class SomeModule { + } - const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].properties['title']) - .toBe('transformed someValue'); - }); + const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); + }); - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should support exported directives and pipes if the module is wrapped into an `ModuleWithProviders`', - () => { - @NgModule( - {declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) - class SomeImportedModule { - } + it('should support exported directives and pipes if the module is wrapped into an `ModuleWithProviders`', + () => { + @NgModule( + {declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) + class SomeImportedModule { + } - @NgModule({ - declarations: [CompUsingModuleDirectiveAndPipe], - imports: [{ngModule: SomeImportedModule}], - entryComponents: [CompUsingModuleDirectiveAndPipe] - }) - class SomeModule { - } + @NgModule({ + declarations: [CompUsingModuleDirectiveAndPipe], + imports: [{ngModule: SomeImportedModule}], + entryComponents: [CompUsingModuleDirectiveAndPipe] + }) + class SomeModule { + } - const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].properties['title']) - .toBe('transformed someValue'); - }); + const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); + }); - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should support reexported modules', () => { - @NgModule( - {declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) - class SomeReexportedModule { - } + it('should support reexported modules', () => { + @NgModule({declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) + class SomeReexportedModule { + } - @NgModule({exports: [SomeReexportedModule]}) - class SomeImportedModule { - } + @NgModule({exports: [SomeReexportedModule]}) + class SomeImportedModule { + } - @NgModule({ - declarations: [CompUsingModuleDirectiveAndPipe], - imports: [SomeImportedModule], - entryComponents: [CompUsingModuleDirectiveAndPipe] - }) - class SomeModule { - } + @NgModule({ + declarations: [CompUsingModuleDirectiveAndPipe], + imports: [SomeImportedModule], + entryComponents: [CompUsingModuleDirectiveAndPipe] + }) + class SomeModule { + } - const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].properties['title']) - .toBe('transformed someValue'); - }); + const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); + }); - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should support exporting individual directives of an imported module', () => { - @NgModule( - {declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) - class SomeReexportedModule { - } + it('should support exporting individual directives of an imported module', () => { + @NgModule({declarations: [SomeDirective, SomePipe], exports: [SomeDirective, SomePipe]}) + class SomeReexportedModule { + } - @NgModule({imports: [SomeReexportedModule], exports: [SomeDirective, SomePipe]}) - class SomeImportedModule { - } + @NgModule({imports: [SomeReexportedModule], exports: [SomeDirective, SomePipe]}) + class SomeImportedModule { + } - @NgModule({ - declarations: [CompUsingModuleDirectiveAndPipe], - imports: [SomeImportedModule], - entryComponents: [CompUsingModuleDirectiveAndPipe] - }) - class SomeModule { - } + @NgModule({ + declarations: [CompUsingModuleDirectiveAndPipe], + imports: [SomeImportedModule], + entryComponents: [CompUsingModuleDirectiveAndPipe] + }) + class SomeModule { + } - const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].properties['title']) - .toBe('transformed someValue'); - }); + const compFixture = createComp(CompUsingModuleDirectiveAndPipe, SomeModule); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']) + .toBe('transformed someValue'); + }); it('should not use non exported pipes of an imported module', () => { @NgModule({ diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 2b055bc58d..a8f1f82cde 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -52,6 +52,15 @@ export class WithRefsCmp { export class InheritedCmp extends SimpleCmp { } +@Directive({selector: '[dir]', host: {'[id]': 'id'}}) +export class HostBindingDir { + id = 'one'; +} + +@Component({selector: 'host-binding-parent', template: '
'}) +export class HostBindingParent { +} + @Component({ selector: 'simple-app', template: ` @@ -62,7 +71,9 @@ export class SimpleApp { } @NgModule({ - declarations: [HelloWorld, SimpleCmp, WithRefsCmp, InheritedCmp, SimpleApp], + declarations: [ + HelloWorld, SimpleCmp, WithRefsCmp, InheritedCmp, SimpleApp, HostBindingParent, HostBindingDir + ], imports: [GreetingModule], providers: [ {provide: NAME, useValue: 'World!'}, @@ -112,6 +123,14 @@ describe('TestBed', () => { expect(greetingByCss.nativeElement).toHaveText('Hello TestBed!'); }); + it('should give the ability to access host properties', () => { + const fixture = TestBed.createComponent(HostBindingParent); + fixture.detectChanges(); + + const divElement = fixture.debugElement.children[0]; + expect(divElement.properties).toEqual({id: 'one'}); + }); + it('should give access to the node injector', () => { const fixture = TestBed.createComponent(HelloWorld); fixture.detectChanges(); diff --git a/packages/platform-browser/test/testing_public_spec.ts b/packages/platform-browser/test/testing_public_spec.ts index c0a52012cb..8df3b9ba17 100644 --- a/packages/platform-browser/test/testing_public_spec.ts +++ b/packages/platform-browser/test/testing_public_spec.ts @@ -251,14 +251,13 @@ class CompWithUrlTemplate { expect(compFixture.componentInstance).toBeAnInstanceOf(CompUsingModuleDirectiveAndPipe); }); - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should use set up directives and pipes', () => { - const compFixture = TestBed.createComponent(CompUsingModuleDirectiveAndPipe); - const el = compFixture.debugElement; + it('should use set up directives and pipes', () => { + const compFixture = TestBed.createComponent(CompUsingModuleDirectiveAndPipe); + const el = compFixture.debugElement; - compFixture.detectChanges(); - expect(el.children[0].properties['title']).toBe('transformed someValue'); - }); + compFixture.detectChanges(); + expect(el.children[0].properties['title']).toBe('transformed someValue'); + }); it('should use set up imported modules', inject([SomeLibModule], (libModule: SomeLibModule) => { @@ -289,14 +288,13 @@ class CompWithUrlTemplate { expect(service.value).toEqual('real value'); })); - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should use set up directives and pipes', withModule(moduleConfig, () => { - const compFixture = TestBed.createComponent(CompUsingModuleDirectiveAndPipe); - const el = compFixture.debugElement; + it('should use set up directives and pipes', withModule(moduleConfig, () => { + const compFixture = TestBed.createComponent(CompUsingModuleDirectiveAndPipe); + const el = compFixture.debugElement; - compFixture.detectChanges(); - expect(el.children[0].properties['title']).toBe('transformed someValue'); - })); + compFixture.detectChanges(); + expect(el.children[0].properties['title']).toBe('transformed someValue'); + })); it('should use set up library modules', withModule(moduleConfig).inject([SomeLibModule], (libModule: SomeLibModule) => { @@ -371,12 +369,11 @@ class CompWithUrlTemplate { .overrideDirective( SomeDirective, {set: {selector: '[someDir]', host: {'[title]': 'someProp'}}}); }); - fixmeIvy('FW-681: not possible to retrieve host property bindings from TView') - .it('should work', () => { - const compFixture = TestBed.createComponent(SomeComponent); - compFixture.detectChanges(); - expect(compFixture.debugElement.children[0].properties['title']).toEqual('hello'); - }); + it('should work', () => { + const compFixture = TestBed.createComponent(SomeComponent); + compFixture.detectChanges(); + expect(compFixture.debugElement.children[0].properties['title']).toEqual('hello'); + }); }); describe('pipe', () => {