fix(platform-server): don't clobber parse5 properties when setting (#18237)

element properties.

Fixes #17050.

We now store all element properties in a separate 'properties' bag.

PR Close #18237
This commit is contained in:
Vikram Subramanian 2017-07-19 13:58:23 -07:00 committed by Miško Hevery
parent b4c98305da
commit a094769bca
3 changed files with 44 additions and 11 deletions

View File

@ -103,7 +103,8 @@ function declareTests({useJit}: {useJit: boolean}) {
fixture.componentInstance.ctxProp = 'Hello World!';
fixture.detectChanges();
expect(fixture.debugElement.children[0].nativeElement.id).toEqual('Hello World!');
expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'id'))
.toEqual('Hello World!');
});
it('should consume binding to aria-* attributes', () => {
@ -165,11 +166,13 @@ function declareTests({useJit}: {useJit: boolean}) {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
expect(fixture.debugElement.children[0].nativeElement.tabIndex).toEqual(0);
expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'tabIndex'))
.toEqual(0);
fixture.componentInstance.ctxNumProp = 5;
fixture.detectChanges();
expect(fixture.debugElement.children[0].nativeElement.tabIndex).toEqual(5);
expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'tabIndex'))
.toEqual(5);
});
it('should consume binding to camel-cased properties', () => {
@ -179,11 +182,13 @@ function declareTests({useJit}: {useJit: boolean}) {
const fixture = TestBed.createComponent(MyComp);
fixture.detectChanges();
expect(fixture.debugElement.children[0].nativeElement.readOnly).toBeFalsy();
expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'readOnly'))
.toBeFalsy();
fixture.componentInstance.ctxBoolProp = true;
fixture.detectChanges();
expect(fixture.debugElement.children[0].nativeElement.readOnly).toBeTruthy();
expect(getDOM().getProperty(fixture.debugElement.children[0].nativeElement, 'readOnly'))
.toBeTruthy();
});
it('should consume binding to innerHtml', () => {
@ -228,7 +233,7 @@ function declareTests({useJit}: {useJit: boolean}) {
fixture.debugElement.componentInstance.ctxProp = 'foo';
fixture.detectChanges();
expect(nativeEl.htmlFor).toBe('foo');
expect(getDOM().getProperty(nativeEl, 'htmlFor')).toBe('foo');
});
it('should consume directive watch expression change.', () => {
@ -897,7 +902,7 @@ function declareTests({useJit}: {useJit: boolean}) {
fixture.detectChanges();
expect(tc.nativeElement.id).toEqual('newId');
expect(getDOM().getProperty(tc.nativeElement, 'id')).toEqual('newId');
});
it('should not use template variables for expressions in hostProperties', () => {
@ -1573,7 +1578,7 @@ function declareTests({useJit}: {useJit: boolean}) {
fixture.detectChanges();
const el = getDOM().querySelector(fixture.nativeElement, 'span');
expect(el.title).toEqual('TITLE');
expect(getDOM().getProperty(el, 'title')).toEqual('TITLE');
});
});

View File

@ -85,12 +85,17 @@ export class Parse5DomAdapter extends DomAdapter {
} else if (name === 'className') {
el.attribs['class'] = el.className = value;
} else {
el[name] = value;
// Store the property in a separate property bag so that it doesn't clobber
// actual parse5 properties on the Element.
el.properties = el.properties || {};
el.properties[name] = value;
}
}
// TODO(tbosch): don't even call this method when we run the tests on server side
// by not using the DomRenderer in tests. Keeping this for now to make tests happy...
getProperty(el: any, name: string): any { return el[name]; }
getProperty(el: any, name: string): any {
return el.properties ? el.properties[name] : undefined;
}
logError(error: string) { console.error(error); }

View File

@ -197,6 +197,20 @@ class MyHostComponent {
class FalseAttributesModule {
}
@Component({selector: 'app', template: '<input [name]="name">'})
class MyInputComponent {
@Input()
name = '';
}
@NgModule({
declarations: [MyInputComponent],
bootstrap: [MyInputComponent],
imports: [ServerModule, BrowserModule.withServerTransition({appId: 'name-attributes'})]
})
class NameModule {
}
export function main() {
if (getDOM().supportsDOMEvents()) return; // NODE only
@ -439,13 +453,22 @@ export function main() {
}));
it('should handle false values on attributes', async(() => {
renderModule(FalseAttributesModule, {document: doc}).then((output) => {
renderModule(FalseAttributesModule, {document: doc}).then(output => {
expect(output).toBe(
'<html><head></head><body><app ng-version="0.0.0-PLACEHOLDER">' +
'<my-child ng-reflect-attr="false">Works!</my-child></app></body></html>');
called = true;
});
}));
it('should handle element property "name"', async(() => {
renderModule(NameModule, {document: doc}).then(output => {
expect(output).toBe(
'<html><head></head><body><app ng-version="0.0.0-PLACEHOLDER">' +
'<input name=""></app></body></html>');
called = true;
});
}));
});
describe('http', () => {