Skip to content

Commit

Permalink
Updates based on code review.
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinpschaaf committed Jul 21, 2018
1 parent 7897d0a commit ed79071
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 55 deletions.
2 changes: 2 additions & 0 deletions lib/elements/dom-if.html
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@
if (c$ && c$.length) {
// use first child parent, for case when dom-if may have been detached
let parent = c$[0].parentNode;
// Instance children may be disconnected from parents when dom-if
// detaches if a tree was innerHTML'ed
if (parent) {
for (let i=0, n; (i<c$.length) && (n=c$[i]); i++) {
parent.removeChild(n);
Expand Down
20 changes: 12 additions & 8 deletions lib/elements/dom-module.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@

let modules = {};
let lcModules = {};
function setModule(id, module) {
// store id separate from lowercased id so that
// in all cases mixedCase id will stored distinctly
// and lowercase version is a fallback
modules[id] = lcModules[id.toLowerCase()] = module;
}
function findModule(id) {
return modules[id] || lcModules[id.toLowerCase()];
}
Expand Down Expand Up @@ -124,16 +130,14 @@
register(id) {
id = id || this.id;
if (id) {
if (Polymer.strictTemplatePolicy && findModule(id)) {
modules[id] = lcModules[id.toLowerCase()] = null;
throw new Error(`strictTemplatePolicy: dom-module ${id} registered twice`);
// Under strictTemplatePolicy, reject and null out any re-registered
// dom-module since it is ambiguous whether first-in or last-in is trusted
if (Polymer.strictTemplatePolicy && findModule(id) !== undefined) {
setModule(id, null);
throw new Error(`strictTemplatePolicy: dom-module ${id} re-registered`);
}
this.id = id;
// store id separate from lowercased id so that
// in all cases mixedCase id will stored distinctly
// and lowercase version is a fallback
modules[id] = this;
lcModules[id.toLowerCase()] = this;
setModule(id, this);
styleOutsideTemplateCheck(this);
}
}
Expand Down
28 changes: 5 additions & 23 deletions lib/legacy/class.html
Original file line number Diff line number Diff line change
Expand Up @@ -152,29 +152,6 @@
return info.observers;
}

/**
* @return {HTMLTemplateElement} template for this class
*/
static get template() {
// get template first from any imperative set in `info._template`
if (info._template !== undefined) {
return info._template;
}
const template = info._template ||
// next look in dom-module associated with this element's is.
Polymer.DomModule && Polymer.DomModule.import(this.is, 'template') ||
// next look for superclass template (note: use superclass symbol
// to ensure correct `this.is`)
Base.template ||
// finally fall back to `_template` in element's prototype.
this.prototype._template ||
null;
if (Polymer.strictTemplatePolicy && !template) {
throw new Error(`strictTemplatePolicy: expecting dom-module or null _template for ${this.is}`);
}
return template;
}

/**
* @return {void}
*/
Expand Down Expand Up @@ -373,6 +350,11 @@
Polymer.LegacyElementMixin(HTMLElement));
// decorate klass with registration info
klass.is = info.is;
// if user provided template on info, make sure the static _template
// is set so the static template getter uses it
if (info._template !== undefined) {
klass._template = info._template;
}
return klass;
};

Expand Down
42 changes: 31 additions & 11 deletions lib/mixins/element-mixin.html
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,27 @@
}
}

/**
* Look up template from dom-module for element
*
* @param {!string} is Element name to look up
* @return {!HTMLTemplateElement} Template found in dom module, or
* undefined if not found
* @private
*/
function getTemplateFromDomModule(is) {
let template = null;
if (is && Polymer.DomModule) {
template = Polymer.DomModule.import(is, 'template');
// Under strictTemplatePolicy, require any element with an `is`
// specified to have a dom-module
if (Polymer.strictTemplatePolicy && !template) {
throw new Error(`strictTemplatePolicy: expecting dom-module or null template for ${is}`);
}
}
return template;
}

/**
* @polymer
* @mixinClass
Expand Down Expand Up @@ -373,7 +394,7 @@
* class MySubClass extends MySuperClass {
* static get template() {
* if (!memoizedTemplate) {
* memoizedTemplate = super.template.cloneNode(true);
* memoizedTemplate = MySuperClass.template.cloneNode(true);
* let subContent = document.createElement('div');
* subContent.textContent = 'This came from MySubClass';
* memoizedTemplate.content.appendChild(subContent);
Expand All @@ -386,16 +407,15 @@
*/
static get template() {
if (!this.hasOwnProperty(JSCompiler_renameProperty('_template', this))) {
const template = Polymer.DomModule && Polymer.DomModule.import(
/** @type {PolymerElementConstructor}*/ (this).is, 'template') ||
// note: implemented so a subclass can retrieve the super
// template; call the super impl this way so that `this` points
// to the superclass.
Object.getPrototypeOf(/** @type {PolymerElementConstructor}*/ (this).prototype).constructor.template;
if (Polymer.strictTemplatePolicy && this.is && !template) {
throw new Error(`strictTemplatePolicy: expecting dom-module or null template for ${this.is}`);
}
this._template = template;
this._template =
// Look in dom-module associated with this element's is
getTemplateFromDomModule(/** @type {PolymerElementConstructor}*/ (this).is) ||
// Next look for superclass template (call the super impl this
// way so that `this` points to the superclass)
Object.getPrototypeOf(/** @type {PolymerElementConstructor}*/ (this).prototype).constructor.template ||
// Finally, fall back to any _template set on prototype, e.g.
// via registered callback
this.prototype._template;
}
return this._template;
}
Expand Down
3 changes: 3 additions & 0 deletions lib/utils/templatize.html
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,9 @@
* @suppress {invalidCasts}
*/
templatize(template, owner, options) {
// Under strictTemplatePolicy, the templatized element must be owned
// by a (trusted) Polymer element, indicated by existence of _methodHost;
// e.g. for dom-if & dom-repeat in main document, _methodHost is null
if (Polymer.strictTemplatePolicy && !owner._methodHost) {
throw new Error('strictTemplatePolicy: template owner not trusted');
}
Expand Down
25 changes: 12 additions & 13 deletions test/unit/strict-template-policy.html
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
});
</script>
</dom-module>

<div id="target"></div>

<script>
Expand All @@ -93,24 +94,22 @@
// Catch uncaught errors; note when running in iframe sometimes
// Safari errors are thrown on the top window, sometimes not, so
// catch in both places
let uncaughtError = null;
window.onerror = window.top.onerror = function(err) {
if (!uncaughtError) {
uncaughtError = new Error(err);
if (!window.uncaughtError) {
window.uncaughtError = new Error(err);
}
};
assert.throws(function() {
fn();
// Re-throw any uncaughtErrors
let err;
if ((err = (uncaughtError || window.uncaughtError))) {
throw new Error(err.message);
if (window.uncaughtError) {
throw new Error(window.uncaughtError.message);
}
// Force polyfill reactions and/or async template stamping
Polymer.flush();
// Re-throw any uncaughtErrors
if ((err = (uncaughtError || window.uncaughtError))) {
throw new Error(err.message);
if (window.uncaughtError) {
throw new Error(window.uncaughtError.message);
}
}, re);
}
Expand Down Expand Up @@ -160,7 +159,7 @@
' </template>`' +
'</dom-module>' +
'<trusted-element></trusted-element>';
}, /trusted-element registered twice/);
}, /trusted-element re-registered/);
const el = document.querySelector('trusted-element');
assert.notOk(el && el.shadowRoot && el.shadowRoot.querySelector('#injected'));
});
Expand Down Expand Up @@ -191,7 +190,7 @@
' </template>`' +
'</dom-module>' +
'<trusted-element-legacy></trusted-element-legacy>';
}, /trusted-element-legacy registered twice/);
}, /trusted-element-legacy re-registered/);
const el = document.querySelector('trusted-element-legacy');
assert.notOk(el && el.shadowRoot && el.shadowRoot.querySelector('#injected'));
});
Expand All @@ -207,9 +206,9 @@
is: 'has-no-template-legacy',
_template: null
});
let el = document.createElement('has-no-template');
let el = document.createElement('has-no-template-legacy');
document.getElementById('target').appendChild(el);
assert.notOk(document.querySelector('has-no-template').shadowRoot);
assert.notOk(document.querySelector('has-no-template-legacy').shadowRoot);
});

test('element without explicit template throws', function() {
Expand All @@ -230,7 +229,7 @@
});
var el = document.createElement('has-no-template-throws-legacy');
document.getElementById('target').appendChild(el);
}, /expecting dom-module or null _template/);
}, /expecting dom-module or null template/);
});

});
Expand Down

0 comments on commit ed79071

Please sign in to comment.