Skip to content

Commit

Permalink
fix(ivy): shadow all DOM properties in DebugElement.properties
Browse files Browse the repository at this point in the history 
Fixes angular#33695
  • Loading branch information
@mhevery
mhevery committed Nov 13, 2019
1 parent 84a0105 commit 4a3430b
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 68 deletions.
47 changes: 26 additions & 21 deletions packages/core/src/debug/debug_node.ts
View file
Expand Up @@ -282,13 +282,7 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
const tNode = tData[context.nodeIndex] as TNode;

const properties = collectPropertyBindings(tNode, lView, tData);
const className = collectClassNames(this);

if (className) {
properties['className'] =
properties['className'] ? properties['className'] + ` ${className}` : className;
}

shadowDomElements(this, properties);
return properties;
}

Expand Down Expand Up @@ -449,6 +443,31 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
}
}

function shadowDomElements(node: DebugElement, properties: {[name: string]: string}): void {
const native = node.nativeNode;
// Skip own properties (as those are patched)
let obj = Object.getPrototypeOf(native);
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`.
const value = native[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.
*
Expand Down Expand Up @@ -678,20 +697,6 @@ function collectPropertyBindings(
}


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;
}


// Need to keep the nodes in a global Map so that multiple angular apps are supported.
const _nativeNodeToDebugNode = new Map<any, DebugNode>();

Expand Down
100 changes: 57 additions & 43 deletions packages/core/test/debug/debug_node_spec.ts
View file
Expand Up @@ -14,7 +14,7 @@ import {ComponentFixture, TestBed, async} from '@angular/core/testing';
import {By} from '@angular/platform-browser/src/dom/debug/by';
import {hasClass} from '@angular/platform-browser/testing/src/browser_util';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {ivyEnabled} from '@angular/private/testing';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';

@Injectable()
class Logger {
Expand Down Expand Up @@ -736,60 +736,74 @@ class TestCmptWithPropInterpolation {
expect(fixture.componentInstance.customed).toBe(true);
});

it('should include classes in properties.className', () => {
fixture = TestBed.createComponent(HostClassBindingCmp);
fixture.detectChanges();
describe('properties', () => {
it('should include classes in properties.className', () => {
fixture = TestBed.createComponent(HostClassBindingCmp);
fixture.detectChanges();

const debugElement = fixture.debugElement;
const debugElement = fixture.debugElement;

expect(debugElement.properties.className).toBe('class-one class-two');
expect(debugElement.properties.className).toBe('class-one class-two');

fixture.componentInstance.hostClasses = 'class-three';
fixture.detectChanges();
fixture.componentInstance.hostClasses = 'class-three';
fixture.detectChanges();

expect(debugElement.properties.className).toBe('class-three');
expect(debugElement.properties.className).toBe('class-three');

fixture.componentInstance.hostClasses = '';
fixture.detectChanges();
fixture.componentInstance.hostClasses = '';
fixture.detectChanges();

expect(debugElement.properties.className).toBeFalsy();
});
expect(debugElement.properties.className).toBeFalsy();
});

it('should preserve the type of the property values', () => {
const fixture = TestBed.createComponent(TestCmptWithPropBindings);
fixture.detectChanges();
it('should preserve the type of the property values', () => {
const fixture = TestBed.createComponent(TestCmptWithPropBindings);
fixture.detectChanges();

const button = fixture.debugElement.query(By.css('button'));
expect(button.properties).toEqual({disabled: true, tabIndex: 1337, title: 'hello'});
});
const button = fixture.debugElement.query(By.css('button'));
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', () => {
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
fixture.detectChanges();
it('should include interpolated properties in the properties map', () => {
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
fixture.detectChanges();

const buttons = fixture.debugElement.children;

expect(buttons.length).toBe(10);
expect(buttons[0].properties.title).toBe('0');
expect(buttons[1].properties.title).toBe('a1b');
expect(buttons[2].properties.title).toBe('a1b2c');
expect(buttons[3].properties.title).toBe('a1b2c3d');
expect(buttons[4].properties.title).toBe('a1b2c3d4e');
expect(buttons[5].properties.title).toBe('a1b2c3d4e5f');
expect(buttons[6].properties.title).toBe('a1b2c3d4e5f6g');
expect(buttons[7].properties.title).toBe('a1b2c3d4e5f6g7h');
expect(buttons[8].properties.title).toBe('a1b2c3d4e5f6g7h8i');
expect(buttons[9].properties.title).toBe('a1b2c3d4e5f6g7h8i9j');
});
const buttons = fixture.debugElement.children;

expect(buttons.length).toBe(10);
expect(buttons[0].properties.title).toBe('0');
expect(buttons[1].properties.title).toBe('a1b');
expect(buttons[2].properties.title).toBe('a1b2c');
expect(buttons[3].properties.title).toBe('a1b2c3d');
expect(buttons[4].properties.title).toBe('a1b2c3d4e');
expect(buttons[5].properties.title).toBe('a1b2c3d4e5f');
expect(buttons[6].properties.title).toBe('a1b2c3d4e5f6g');
expect(buttons[7].properties.title).toBe('a1b2c3d4e5f6g7h');
expect(buttons[8].properties.title).toBe('a1b2c3d4e5f6g7h8i');
expect(buttons[9].properties.title).toBe('a1b2c3d4e5f6g7h8i9j');
});

it('should not include directive-shadowed properties in the properties map', () => {
TestBed.overrideTemplate(
TestCmptWithPropInterpolation, `<button with-title [title]="'goes to input'"></button>`);
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
fixture.detectChanges();
it('should not include directive-shadowed properties in the properties map', () => {
TestBed.overrideTemplate(
TestCmptWithPropInterpolation, `<button with-title [title]="'ShadowInput'"></button>`);
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
fixture.detectChanges();

const button = fixture.debugElement.query(By.css('button'));
expect(button.properties.title).toBeUndefined();
const button = fixture.debugElement.query(By.css('button'));
expect(button.properties.title).not.toEqual('ShadowInput');
});

onlyInIvy('Ivy shadows native DOM properties').it('should reflect native properties', () => {
TestBed.overrideTemplate(
TestCmptWithPropInterpolation, `<button title="myTitle"></button>`);
const fixture = TestBed.createComponent(TestCmptWithPropInterpolation);
fixture.detectChanges();

const button = fixture.debugElement.query(By.css('button'));
expect(button.properties.title).toBe('myTitle');
});
});

it('should trigger events registered via Renderer2', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/test/linker/integration_spec.ts
View file
Expand Up @@ -921,6 +921,7 @@ function declareTests(config?: {useJit: boolean}) {
@Directive({selector: '[host-properties]', host: {'[id]': 'id', '[title]': 'unknownProp'}})
class DirectiveWithHostProps {
id = 'one';
unknownProp = 'unknownProp';
}

const fixture =
Expand All @@ -933,7 +934,7 @@ function declareTests(config?: {useJit: boolean}) {

const tc = fixture.debugElement.children[0];
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', () => {
Expand Down
7 changes: 4 additions & 3 deletions packages/core/test/test_bed_spec.ts
View file
Expand Up @@ -164,16 +164,17 @@ describe('TestBed', () => {
fixture.detectChanges();

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', () => {
const fixture = TestBed.createComponent(ComponentWithPropBindings);
fixture.detectChanges();

const paragraphEl = fixture.debugElement.query(By.css('p'));
expect(paragraphEl.properties)
.toEqual({title: '( some label - some title )', id: '[ some label ] [ some title ]'});
expect(paragraphEl.properties.title).toEqual('( some label - some title )');
expect(paragraphEl.properties.id).toEqual('[ some label ] [ some title ]');
});

it('should give access to the node injector', () => {
Expand Down

0 comments on commit 4a3430b

Please sign in to comment.