Skip to content

Commit

Permalink
fix(compiler): detect dangling properties on custom elements
Browse files Browse the repository at this point in the history
  • Loading branch information
pkozlowski-opensource committed Jul 20, 2015
1 parent 5749692 commit 8ecf200
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 10 deletions.
6 changes: 6 additions & 0 deletions modules/angular2/src/dom/browser_adapter.ts
Expand Up @@ -142,6 +142,12 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
return t;
}
createElement(tagName, doc = document): HTMLElement { return doc.createElement(tagName); }
registerElement(tagName, tagType): void {
if (this.supportsCustomElements()) {
// TODO(pk): I guess TS definition of the Document object should be extended...
(<any>document).registerElement(tagName, tagType);
}
}
createTextNode(text: string, doc = document): Text { return doc.createTextNode(text); }
createScriptTag(attrName: string, attrValue: string, doc = document): HTMLScriptElement {
var el = <HTMLScriptElement>doc.createElement('SCRIPT');
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/dom/dom_adapter.ts
Expand Up @@ -67,6 +67,7 @@ export class DomAdapter {
setChecked(el, value: boolean) { throw _abstract(); }
createTemplate(html): HTMLElement { throw _abstract(); }
createElement(tagName, doc = null): HTMLElement { throw _abstract(); }
registerElement(tagName, tagType): void { throw _abstract(); }
createTextNode(text: string, doc = null): Text { throw _abstract(); }
createScriptTag(attrName: string, attrValue: string, doc = null): HTMLElement {
throw _abstract();
Expand Down Expand Up @@ -116,6 +117,7 @@ export class DomAdapter {
cssToRules(css: string): List<any> { throw _abstract(); }
supportsDOMEvents(): boolean { throw _abstract(); }
supportsNativeShadowDOM(): boolean { throw _abstract(); }
supportsCustomElements(): boolean { throw _abstract(); }
getGlobalEventTarget(target: string): any { throw _abstract(); }
getHistory(): History { throw _abstract(); }
getLocation(): Location { throw _abstract(); }
Expand Down
1 change: 1 addition & 0 deletions modules/angular2/src/dom/generic_browser_adapter.ts
Expand Up @@ -37,4 +37,5 @@ export class GenericBrowserDomAdapter extends DomAdapter {
supportsNativeShadowDOM(): boolean {
return isFunction((<any>this.defaultDoc().body).createShadowRoot);
}
supportsCustomElements(): boolean { return isFunction((<any>this.defaultDoc()).registerElement); }
}
4 changes: 4 additions & 0 deletions modules/angular2/src/dom/parse5_adapter.ts
Expand Up @@ -254,6 +254,9 @@ export class Parse5DomAdapter extends DomAdapter {
createElement(tagName): HTMLElement {
return treeAdapter.createElement(tagName, 'http://www.w3.org/1999/xhtml', []);
}
registerElement(tagName, tagType): void {
// noop
}
createTextNode(text: string): Text { throw _notImplemented('createTextNode'); }
createScriptTag(attrName: string, attrValue: string): HTMLElement {
return treeAdapter.createElement("script", 'http://www.w3.org/1999/xhtml',
Expand Down Expand Up @@ -507,6 +510,7 @@ export class Parse5DomAdapter extends DomAdapter {
}
supportsDOMEvents(): boolean { return false; }
supportsNativeShadowDOM(): boolean { return false; }
supportsCustomElements(): boolean { return false; }
getGlobalEventTarget(target: string): any {
if (target == "window") {
return (<any>this.defaultDoc())._window;
Expand Down
20 changes: 12 additions & 8 deletions modules/angular2/src/render/dom/view/proto_view_builder.ts
Expand Up @@ -306,6 +306,7 @@ export class EventBuilder extends AstTransformer {
}
}

var customElmsCache = new Map<String, any>();
var PROPERTY_PARTS_SEPARATOR = new RegExp('\\.');
const ATTRIBUTE_PREFIX = 'attr';
const CLASS_PREFIX = 'class';
Expand All @@ -330,16 +331,19 @@ function buildElementPropertyBindings(protoElement: /*element*/ any, isNgCompone

function isValidElementPropertyBinding(protoElement: /*element*/ any, isNgComponent: boolean,
binding: api.ElementPropertyBinding): boolean {
var elInstance = protoElement;
var tagName = DOM.tagName(protoElement);

if (binding.type === api.PropertyBindingType.PROPERTY) {
var tagName = DOM.tagName(protoElement);
var possibleCustomElement = tagName.indexOf('-') !== -1;
if (possibleCustomElement && !isNgComponent) {
// can't tell now as we don't know which properties a custom element will get
// once it is instantiated
return true;
} else {
return DOM.hasProperty(protoElement, binding.property);
if (!isNgComponent && DOM.supportsCustomElements() && tagName.indexOf('-') !== -1) {
if (customElmsCache.has(tagName)) {
elInstance = customElmsCache.get(tagName);
} else {
elInstance = DOM.createElement(tagName);
customElmsCache.set(tagName, elInstance);
}
}
return DOM.hasProperty(elInstance, binding.property);
}
return true;
}
Expand Down
26 changes: 24 additions & 2 deletions modules/angular2/test/render/dom/view/proto_view_builder_spec.ts
Expand Up @@ -15,6 +15,7 @@ import {ProtoViewBuilder} from 'angular2/src/render/dom/view/proto_view_builder'
import {ASTWithSource, AST} from 'angular2/change_detection';
import {PropertyBindingType, ViewType} from 'angular2/src/render/api';
import {DOM} from 'angular2/src/dom/dom_adapter';
import {TestCustomElement} from './test_custom_element';

export function main() {
function emptyExpr() { return new ASTWithSource(new AST(), 'empty', 'empty'); }
Expand All @@ -41,12 +42,33 @@ export function main() {
expect(() => builder.build()).not.toThrow();
});

it('should allow unknown properties on custom elements', () => {
it('should throw for unknown properties on custom-like elements', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build()).not.toThrow();
expect(() => builder.build())
.toThrowError(
`Can't bind to 'unknownProperty' since it isn't a known property of the '<some-custom>' element and there are no matching directives with a corresponding property`);
});

if (DOM.supportsCustomElements()) {
DOM.registerElement('some-custom', TestCustomElement);

it('should allow known properties of custom elements', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('knownProperty', emptyExpr());
binder.bindProperty('title', emptyExpr());
expect(() => builder.build()).not.toThrow();
});

it('should throw for unknown properties on custom elements', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build())
.toThrowError(
`Can't bind to 'unknownProperty' since it isn't a known property of the '<some-custom>' element and there are no matching directives with a corresponding property`);
});
}

it('should throw for unknown properties on custom elements if there is an ng component', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
Expand Down
Empty file.
8 changes: 8 additions & 0 deletions modules/angular2/test/render/dom/view/test_custom_element.ts
@@ -0,0 +1,8 @@
import {DOM} from 'angular2/src/dom/dom_adapter';

// TODO(pk): can't use a class here due to https://github.com/Microsoft/TypeScript/issues/574
export var TestCustomElement = {
// should be Object.create(HTMLElement.prototype, { but HTMLElement is an interface in TS :-/
prototype: Object.create(Object.getPrototypeOf(DOM.createElement('div')),
{knownProperty: {value: function() { return 'known value';}}})
};

0 comments on commit 8ecf200

Please sign in to comment.