Skip to content

Commit

Permalink
Updates from review.
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinpschaaf committed Jul 2, 2019
1 parent ff25283 commit f0cbc83
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 10 deletions.
34 changes: 28 additions & 6 deletions lib/mixins/property-effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -2691,17 +2691,24 @@ export const PropertyEffects = dedupingMixin(superClass => {
}
if (instanceBinding) {
// For instance-time binding, create instance of template metadata
// and link into list of templates if necessary
// and link into tree of templates if necessary
templateInfo = /** @type {!TemplateInfo} */(Object.create(templateInfo));
templateInfo.wasPreBound = wasPreBound;
if (!this.__templateInfo) {
// Set the info to the root of the tree
this.__templateInfo = templateInfo;
} else {
// Append this template info onto the end of its parent template's
// nested template list; if this template was not nested in another
// list, which will determine the tree structure via which property
// effects are run; if this template was not nested in another
// template, use the root template (the first stamped one) as the
// parent
// parent. Note, `parent` is the `templateInfo` instance for this
// template's containing template, which was set up in
// `applyTemplateContent`. While a given template's `parent` is
// given, it is only added to the parent list at the point that it is
// being bound, since a template may or may not ever be stamped, and
// may be stamped more than once (in which case it will be in the
// tree more than once).
const parent = templateInfo.parent || this.__templateInfo;
const previous = parent.lastChild;
parent.lastChild = templateInfo;
Expand Down Expand Up @@ -2800,7 +2807,10 @@ export const PropertyEffects = dedupingMixin(superClass => {
* @protected
*/
_removeBoundDom(dom) {
// Unlink template info
// Unlink template info; Note that while the child is unlinked from its
// parent list, a template's `parent` reference is never removed, since
// this is is determined by the tree structure and applied at
// `applyTemplateContent` time.
let templateInfo = dom.templateInfo;
const {previousSibling, nextSibling, parent} = templateInfo;
if (previousSibling) {
Expand Down Expand Up @@ -2955,19 +2965,31 @@ export const PropertyEffects = dedupingMixin(superClass => {
if (removeNestedTemplates && (isDomIf || isDomRepeat)) {
parent.removeChild(node);
nodeInfo.parentInfo.templateInfo = nestedTemplateInfo;
// Ensure the parent dom-if/repeat is noted since it now has bindings;
// it may not have been if it did not have its own bindings
nodeInfo = nodeInfo.parentInfo;
nodeInfo.noted = true;
noted = false;
}
// Merge host props into outer template and add bindings
let hostProps = nestedTemplateInfo.hostProps;
if (fastDomIf && isDomIf) {
// `fastDomIf` mode uses runtime-template stamping to add accessors/
// effects to properties used in its template; as such we don't need to
// tax the host element with `_host_` bindings for the `dom-if`.
// However, in the event it is nested in a `dom-repeat`, it is still
// important that its host properties are added to the
// TemplateInstance's `hostProps` so that they are forwarded to the
// TemplateInstance.
if (hostProps) {
templateInfo.hostProps =
Object.assign(templateInfo.hostProps || {}, hostProps);
// Ensure the dom-if is noted so that it has a __dataHost, since
// `fastDomIf` uses the host for runtime template stamping; note this
// was already ensured above in the `removeNestedTemplates` case
if (!removeNestedTemplates) {
nodeInfo.parentInfo.noted = true;
}
templateInfo.hostProps =
Object.assign(templateInfo.hostProps || {}, hostProps);
}
} else {
let mode = '{';
Expand Down
7 changes: 7 additions & 0 deletions lib/mixins/template-stamp.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,10 @@ export const TemplateStamp = dedupingMixin(
if (element.hasAttributes && element.hasAttributes()) {
noted = this._parseTemplateNodeAttributes(element, templateInfo, nodeInfo) || noted;
}
// Checking `nodeInfo.noted` allows a child node of this node (who gets
// access to `parentInfo`) to cause the parent to be noted, which
// otherwise has no return path via `_parseTemplateChildNodes` (used by
// some optimizations)
return noted || nodeInfo.noted;
}

Expand Down Expand Up @@ -448,6 +452,9 @@ export const TemplateStamp = dedupingMixin(
window.HTMLTemplateElement && HTMLTemplateElement.decorate) {
HTMLTemplateElement.decorate(template);
}
// Accepting the `templateInfo` via an argument allows for creating
// instances of the `templateInfo` by the caller, useful for adding
// instance-time information to the prototypical data
templateInfo = templateInfo || this.constructor._parseTemplate(template);
let nodeInfo = templateInfo.nodeInfoList;
let content = templateInfo.content || template.content;
Expand Down
6 changes: 3 additions & 3 deletions lib/utils/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,12 @@ export let removeNestedTemplates =
* Sets `removeNestedTemplates` globally, to eliminate nested templates
* inside `dom-if` and `dom-repeat` as part of template parsing.
*
* @param {boolean} useCemoveNestedTemplates enable or disable removing nested
* @param {boolean} useRemoveNestedTemplates enable or disable removing nested
* templates during parsing
* @return {void}
*/
export const setRemoveNestedTemplates = function(useCemoveNestedTemplates) {
removeNestedTemplates = useCemoveNestedTemplates;
export const setRemoveNestedTemplates = function(useRemoveNestedTemplates) {
removeNestedTemplates = useRemoveNestedTemplates;
};

/**
Expand Down
3 changes: 2 additions & 1 deletion lib/utils/templatize.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ function addPropagateEffects(target, templateInfo, options, methodHost) {
// Create a cached subclass of the base custom element class onto which
// to put the template-specific propagate effects
/** @private */
klass = class TemplatizedTemplateExtension extends templatizedBase {};
klass = templateInfo.templatizeTemplateClass =
class TemplatizedTemplateExtension extends templatizedBase {};
}
// Add template - >instances effects
// and host <- template effects
Expand Down

0 comments on commit f0cbc83

Please sign in to comment.