fix(ivy): shadow all DOM properties in `DebugElement.properties` (#33781)
Fixes #33695 PR Close #33781
This commit is contained in:
parent
f4caf263d4
commit
a681c8553a
|
@ -278,14 +278,12 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
|
||||||
const tData = lView[TVIEW].data;
|
const tData = lView[TVIEW].data;
|
||||||
const tNode = tData[context.nodeIndex] as TNode;
|
const tNode = tData[context.nodeIndex] as TNode;
|
||||||
|
|
||||||
const properties = collectPropertyBindings(tNode, lView, tData);
|
const properties: {[key: string]: string} = {};
|
||||||
const className = collectClassNames(this);
|
// Collect properties from the DOM.
|
||||||
|
copyDomProperties(this.nativeElement, properties);
|
||||||
if (className) {
|
// Collect properties from the bindings. This is needed for animation renderer which has
|
||||||
properties['className'] =
|
// synthetic properties which don't get reflected into the DOM.
|
||||||
properties['className'] ? properties['className'] + ` ${className}` : className;
|
collectPropertyBindings(properties, tNode, lView, tData);
|
||||||
}
|
|
||||||
|
|
||||||
return properties;
|
return properties;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -446,6 +444,34 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function copyDomProperties(element: Element | null, properties: {[name: string]: string}): void {
|
||||||
|
if (element) {
|
||||||
|
// Skip own properties (as those are patched)
|
||||||
|
let obj = Object.getPrototypeOf(element);
|
||||||
|
const NodePrototype: any = Node.prototype;
|
||||||
|
while (obj !== null && obj !== NodePrototype) {
|
||||||
|
const descriptors = Object.getOwnPropertyDescriptors(obj);
|
||||||
|
for (let key in descriptors) {
|
||||||
|
if (!key.startsWith('__') && !key.startsWith('on')) {
|
||||||
|
// don't include properties starting with `__` and `on`.
|
||||||
|
// `__` are patched values which should not be included.
|
||||||
|
// `on` are listeners which also should not be included.
|
||||||
|
const value = (element as any)[key];
|
||||||
|
if (isPrimitiveValue(value)) {
|
||||||
|
properties[key] = value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
obj = Object.getPrototypeOf(obj);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function isPrimitiveValue(value: any): boolean {
|
||||||
|
return typeof value === 'string' || typeof value === 'boolean' || typeof value === 'number' ||
|
||||||
|
value === null;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Walk the TNode tree to find matches for the predicate.
|
* Walk the TNode tree to find matches for the predicate.
|
||||||
*
|
*
|
||||||
|
@ -649,8 +675,7 @@ function _queryNativeNodeDescendants(
|
||||||
* defined in templates, not in host bindings.
|
* defined in templates, not in host bindings.
|
||||||
*/
|
*/
|
||||||
function collectPropertyBindings(
|
function collectPropertyBindings(
|
||||||
tNode: TNode, lView: LView, tData: TData): {[key: string]: string} {
|
properties: {[key: string]: string}, tNode: TNode, lView: LView, tData: TData): void {
|
||||||
const properties: {[key: string]: string} = {};
|
|
||||||
let bindingIndexes = tNode.propertyBindings;
|
let bindingIndexes = tNode.propertyBindings;
|
||||||
|
|
||||||
if (bindingIndexes !== null) {
|
if (bindingIndexes !== null) {
|
||||||
|
@ -670,22 +695,6 @@ function collectPropertyBindings(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return properties;
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
function collectClassNames(debugElement: DebugElement__POST_R3__): string {
|
|
||||||
const classes = debugElement.classes;
|
|
||||||
let output = '';
|
|
||||||
|
|
||||||
for (const className of Object.keys(classes)) {
|
|
||||||
if (classes[className]) {
|
|
||||||
output = output ? output + ` ${className}` : className;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return output;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -14,7 +14,7 @@ import {ComponentFixture, TestBed, async} from '@angular/core/testing';
|
||||||
import {By} from '@angular/platform-browser/src/dom/debug/by';
|
import {By} from '@angular/platform-browser/src/dom/debug/by';
|
||||||
import {hasClass} from '@angular/platform-browser/testing/src/browser_util';
|
import {hasClass} from '@angular/platform-browser/testing/src/browser_util';
|
||||||
import {expect} from '@angular/platform-browser/testing/src/matchers';
|
import {expect} from '@angular/platform-browser/testing/src/matchers';
|
||||||
import {ivyEnabled} from '@angular/private/testing';
|
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
class Logger {
|
class Logger {
|
||||||
|
@ -192,6 +192,11 @@ class TestApp {
|
||||||
class TestCmpt {
|
class TestCmpt {
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Component({selector: 'test-cmpt-renderer', template: ``})
|
||||||
|
class TestCmptWithRenderer {
|
||||||
|
constructor(public renderer: Renderer2) {}
|
||||||
|
}
|
||||||
|
|
||||||
@Component({selector: 'host-class-binding', template: ''})
|
@Component({selector: 'host-class-binding', template: ''})
|
||||||
class HostClassBindingCmp {
|
class HostClassBindingCmp {
|
||||||
@HostBinding('class')
|
@HostBinding('class')
|
||||||
|
@ -259,6 +264,7 @@ class TestCmptWithPropInterpolation {
|
||||||
SimpleContentComp,
|
SimpleContentComp,
|
||||||
TestCmptWithPropBindings,
|
TestCmptWithPropBindings,
|
||||||
TestCmptWithPropInterpolation,
|
TestCmptWithPropInterpolation,
|
||||||
|
TestCmptWithRenderer,
|
||||||
WithTitleDir,
|
WithTitleDir,
|
||||||
],
|
],
|
||||||
providers: [Logger],
|
providers: [Logger],
|
||||||
|
@ -736,6 +742,7 @@ class TestCmptWithPropInterpolation {
|
||||||
expect(fixture.componentInstance.customed).toBe(true);
|
expect(fixture.componentInstance.customed).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('properties', () => {
|
||||||
it('should include classes in properties.className', () => {
|
it('should include classes in properties.className', () => {
|
||||||
fixture = TestBed.createComponent(HostClassBindingCmp);
|
fixture = TestBed.createComponent(HostClassBindingCmp);
|
||||||
fixture.detectChanges();
|
fixture.detectChanges();
|
||||||
|
@ -760,7 +767,9 @@ class TestCmptWithPropInterpolation {
|
||||||
fixture.detectChanges();
|
fixture.detectChanges();
|
||||||
|
|
||||||
const button = fixture.debugElement.query(By.css('button'));
|
const button = fixture.debugElement.query(By.css('button'));
|
||||||
expect(button.properties).toEqual({disabled: true, tabIndex: 1337, title: 'hello'});
|
expect(button.properties.disabled).toEqual(true);
|
||||||
|
expect(button.properties.tabIndex).toEqual(1337);
|
||||||
|
expect(button.properties.title).toEqual('hello');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should include interpolated properties in the properties map', () => {
|
it('should include interpolated properties in the properties map', () => {
|
||||||
|
@ -784,12 +793,50 @@ class TestCmptWithPropInterpolation {
|
||||||
|
|
||||||
it('should not include directive-shadowed properties in the properties map', () => {
|
it('should not include directive-shadowed properties in the properties map', () => {
|
||||||
TestBed.overrideTemplate(
|
TestBed.overrideTemplate(
|
||||||
TestCmptWithPropInterpolation, `<button with-title [title]="'goes to input'"></button>`);
|
TestCmptWithPropInterpolation,
|
||||||
|
`<button with-title [title]="'goes to input'"></button>`);
|
||||||
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
|
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
|
||||||
fixture.detectChanges();
|
fixture.detectChanges();
|
||||||
|
|
||||||
const button = fixture.debugElement.query(By.css('button'));
|
const button = fixture.debugElement.query(By.css('button'));
|
||||||
expect(button.properties.title).toBeUndefined();
|
expect(button.properties.title).not.toEqual('goes to input');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return native properties from DOM element even if no binding present', () => {
|
||||||
|
TestBed.overrideTemplate(TestCmptWithRenderer, `<button></button>`);
|
||||||
|
const fixture = TestBed.createComponent(TestCmptWithRenderer);
|
||||||
|
fixture.detectChanges();
|
||||||
|
const button = fixture.debugElement.query(By.css('button'));
|
||||||
|
fixture.componentInstance.renderer.setProperty(button.nativeElement, 'title', 'myTitle');
|
||||||
|
expect(button.properties.title).toBe('myTitle');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not include patched properties (starting with __) and on* properties', () => {
|
||||||
|
TestBed.overrideTemplate(
|
||||||
|
TestCmptWithPropInterpolation, `<button title="myTitle" (click)="true;"></button>`);
|
||||||
|
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const host = fixture.debugElement;
|
||||||
|
const button = fixture.debugElement.query(By.css('button'));
|
||||||
|
expect(Object.keys(host.properties).filter(key => key.startsWith('__'))).toEqual([]);
|
||||||
|
expect(Object.keys(host.properties).filter(key => key.startsWith('on'))).toEqual([]);
|
||||||
|
expect(Object.keys(button.properties).filter(key => key.startsWith('__'))).toEqual([]);
|
||||||
|
expect(Object.keys(button.properties).filter(key => key.startsWith('on'))).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
onlyInIvy('Show difference in behavior')
|
||||||
|
.it('should pickup all of the element properties', () => {
|
||||||
|
TestBed.overrideTemplate(
|
||||||
|
TestCmptWithPropInterpolation, `<button title="myTitle"></button>`);
|
||||||
|
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const host = fixture.debugElement;
|
||||||
|
const button = fixture.debugElement.query(By.css('button'));
|
||||||
|
|
||||||
|
expect(button.properties.title).toEqual('myTitle');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should trigger events registered via Renderer2', () => {
|
it('should trigger events registered via Renderer2', () => {
|
||||||
|
|
|
@ -921,6 +921,7 @@ function declareTests(config?: {useJit: boolean}) {
|
||||||
@Directive({selector: '[host-properties]', host: {'[id]': 'id', '[title]': 'unknownProp'}})
|
@Directive({selector: '[host-properties]', host: {'[id]': 'id', '[title]': 'unknownProp'}})
|
||||||
class DirectiveWithHostProps {
|
class DirectiveWithHostProps {
|
||||||
id = 'one';
|
id = 'one';
|
||||||
|
unknownProp = 'unknownProp';
|
||||||
}
|
}
|
||||||
|
|
||||||
const fixture =
|
const fixture =
|
||||||
|
@ -933,7 +934,7 @@ function declareTests(config?: {useJit: boolean}) {
|
||||||
|
|
||||||
const tc = fixture.debugElement.children[0];
|
const tc = fixture.debugElement.children[0];
|
||||||
expect(tc.properties['id']).toBe('one');
|
expect(tc.properties['id']).toBe('one');
|
||||||
expect(tc.properties['title']).toBe(undefined);
|
expect(tc.properties['title']).toBe('unknownProp');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should not allow pipes in hostProperties', () => {
|
it('should not allow pipes in hostProperties', () => {
|
||||||
|
|
|
@ -164,7 +164,8 @@ describe('TestBed', () => {
|
||||||
fixture.detectChanges();
|
fixture.detectChanges();
|
||||||
|
|
||||||
const divElement = fixture.debugElement.query(By.css('div'));
|
const divElement = fixture.debugElement.query(By.css('div'));
|
||||||
expect(divElement.properties).toEqual({id: 'one', title: 'some title'});
|
expect(divElement.properties.id).toEqual('one');
|
||||||
|
expect(divElement.properties.title).toEqual('some title');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should give the ability to access interpolated properties on a node', () => {
|
it('should give the ability to access interpolated properties on a node', () => {
|
||||||
|
@ -172,8 +173,8 @@ describe('TestBed', () => {
|
||||||
fixture.detectChanges();
|
fixture.detectChanges();
|
||||||
|
|
||||||
const paragraphEl = fixture.debugElement.query(By.css('p'));
|
const paragraphEl = fixture.debugElement.query(By.css('p'));
|
||||||
expect(paragraphEl.properties)
|
expect(paragraphEl.properties.title).toEqual('( some label - some title )');
|
||||||
.toEqual({title: '( some label - some title )', id: '[ some label ] [ some title ]'});
|
expect(paragraphEl.properties.id).toEqual('[ some label ] [ some title ]');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should give access to the node injector', () => {
|
it('should give access to the node injector', () => {
|
||||||
|
|
|
@ -57,6 +57,27 @@ import {NgModelCustomComp, NgModelCustomWrapper} from './value_accessor_integrat
|
||||||
expect(form.valid).toBe(false);
|
expect(form.valid).toBe(false);
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
it('should report properties which are written outside of template bindings', async() => {
|
||||||
|
// For example ngModel writes to `checked` property programmatically
|
||||||
|
// (template does not contain binding to `checked` explicitly)
|
||||||
|
// https://github.com/angular/angular/issues/33695
|
||||||
|
@Component({
|
||||||
|
selector: 'app-root',
|
||||||
|
template: `<input type="radio" value="one" [(ngModel)]="active"/>`
|
||||||
|
})
|
||||||
|
class AppComponent {
|
||||||
|
active = 'one';
|
||||||
|
}
|
||||||
|
TestBed.configureTestingModule({imports: [FormsModule], declarations: [AppComponent]});
|
||||||
|
const fixture = TestBed.createComponent(AppComponent);
|
||||||
|
// We need the Await as `ngModel` writes data asynchronously into the DOM
|
||||||
|
await fixture.detectChanges();
|
||||||
|
const input = fixture.debugElement.query(By.css('input'));
|
||||||
|
expect(input.properties.checked).toBe(true);
|
||||||
|
expect(input.nativeElement.checked).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
|
||||||
it('should add novalidate by default to form element', fakeAsync(() => {
|
it('should add novalidate by default to form element', fakeAsync(() => {
|
||||||
const fixture = initTest(NgModelForm);
|
const fixture = initTest(NgModelForm);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue