Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate binding-specific code from template stamp. Expose override points #4432

Merged
merged 6 commits into from
Apr 4, 2017

Conversation

kevinpschaaf
Copy link
Member

No description provided.

// since a template may be re-used, memo-ize notes.
if (!content._notes) {
let notes = content._notes = [];
notes.ownerDocument = ownerDocument || template.ownerDocument;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May not be worth threading this through since it's only used for inertness.

return content._notes;
}

// add annotations gleaned from subtree at `node` to `notes`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps (node, note, notes)

let noted;
if (node.localName == 'template' &&
!node.hasAttribute('preserve-content')) {
noted |= this._parseTemplate(node, note);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted = this._parseTemplate(node, note) || noted

!node.hasAttribute('preserve-content')) {
noted |= this._parseTemplate(node, note);
} else if (node.localName === 'slot') {
node._hasInsertionPoint = true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add note that this is a Shady DOM optimziation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be note.notes._hasInsertionPoints = true

if (node.firstChild) {
noted |= this._parseTemplateChildNodes(node, note);
}
if (node.attributes) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use hasAttributes

if (note.notes.stripWhiteSpace && !node.textContent.trim()) {
root.removeChild(node);
// decrement index since node is removed
i--;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this can just continue and remove the if (node.parentNode) check below

// children and avoids a bug in Chrome where nested template children
// upgrade)
static _parseTemplate(node, note) {
let content = node.content.ownerDocument.createDocumentFragment();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this and just have 1 inert document?

static _parseTemplateNodeAttributes(node, note) {
// Make copy of original attribute list, since the order may change
// as attributes are added and removed
let attrs = Array.prototype.slice.call(node.attributes);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array.from

name = propertyName;
}
note.bindings = note.bindings || [];
note.bindings.push({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

es6-ify

// m[1]: '{{' '[['
// m[2]: '' '!'
// m[3]: 'prop' 'compute(foo,bar)'
while ((m = bindingRegex.exec(text)) !== null) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove !== null

@kevinpschaaf
Copy link
Member Author

Consider:

  • note -> nodeInfo
  • notes -> templateInfo = { nodes: [ ... ] }
  • Pass templateInfo through tree walk (and _parseBindings). Hang stripWhitespace, hostProps, dynamicFns, etc. on templateInfo

// (this is both an optimization to avoid re-stamping nested template
// children and avoids a bug in Chrome where nested template children
// upgrade)
static _parseTemplateNestedContent(node, outerTemplateInfo, nodeInfo) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth trying to collapse with _parseTemplate? Seems like if there's an outerTemplate, you can do this content stripping.

function applyTemplateContent(inst, node, note) {
if (note.templateContent) {
node._content = note.templateContent;
function applyTemplateContent(inst, node, nodeInfo) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change argument node -> template

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants