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

[ShadyDOM] Add option for on-demand patching #189

Merged
merged 14 commits into from
Oct 11, 2019
27 changes: 23 additions & 4 deletions packages/shadydom/src/link-nodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,28 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
import * as utils from './utils.js';
import {shadyDataForNode, ensureShadyDataForNode} from './shady-data.js';
import {patchInsideElementAccessors, patchOutsideElementAccessors} from './patch-instances.js';
import {patchNodeProto} from './patch-prototypes.js';

const OutsideAccessors = 1;
const InsideAcccessors = 2;

const patchOnDemand = utils.settings.patchOnDemand;
const hasDescriptors = utils.settings.hasDescriptors;

function patchNode(node, type) {
if (patchOnDemand) {
patchNodeProto(node);
} else if (!hasDescriptors) {
if (type === OutsideAccessors) {
patchOutsideElementAccessors(node);
} else if (type === InsideAcccessors) {
patchInsideElementAccessors(node);
}
}
}

function linkNode(node, container, containerData, ref_node) {
patchOutsideElementAccessors(node);
patchNode(node, OutsideAccessors);
ref_node = ref_node || null;
const nodeData = ensureShadyDataForNode(node);
const ref_nodeData = ref_node ? ensureShadyDataForNode(ref_node) : null;
Expand Down Expand Up @@ -46,7 +65,7 @@ function linkNode(node, container, containerData, ref_node) {
}

export const recordInsertBefore = (node, container, ref_node) => {
patchInsideElementAccessors(container);
patchNode(container, InsideAcccessors);
const containerData = ensureShadyDataForNode(container);
if (containerData.firstChild !== undefined) {
containerData.childNodes = null;
Expand Down Expand Up @@ -106,13 +125,13 @@ export const recordChildNodes = (node, adoptedParent) => {
nodeData.childNodes = null;
const first = nodeData.firstChild = node[utils.NATIVE_PREFIX + 'firstChild'];
nodeData.lastChild = node[utils.NATIVE_PREFIX + 'lastChild'];
patchInsideElementAccessors(node);
patchNode(node, InsideAcccessors);
for (let n = first, previous; n; (n = n[utils.NATIVE_PREFIX + 'nextSibling'])) {
const sd = ensureShadyDataForNode(n);
sd.parentNode = adoptedParent || node;
sd.nextSibling = n[utils.NATIVE_PREFIX + 'nextSibling'];
sd.previousSibling = previous || null;
previous = n;
patchOutsideElementAccessors(n);
patchNode(n, OutsideAccessors);
}
}
56 changes: 53 additions & 3 deletions packages/shadydom/src/patch-prototypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {EventTargetPatches} from './patches/EventTarget.js';
import {NodePatches} from './patches/Node.js';
import {SlotablePatches} from './patches/Slotable.js';
import {ParentNodePatches, ParentNodeDocumentOrFragmentPatches} from './patches/ParentNode.js';
import {ElementPatches} from './patches/Element.js';
import {ElementPatches, ElementShadowPatches} from './patches/Element.js';
import {ElementOrShadowRootPatches} from './patches/ElementOrShadowRoot.js';
import {HTMLElementPatches} from './patches/HTMLElement.js';
import {SlotPatches} from './patches/Slot.js';
Expand Down Expand Up @@ -71,16 +71,66 @@ const getPatchPrototype = (name) => window[name] && window[name].prototype;
// CustomElements polyfill *only* cares about these accessors.
const disallowedNativePatches = utils.settings.hasDescriptors ? null : ['innerHTML', 'textContent'];

/**
* Patch a group of accessors on an object only if it exists or if the `force`
Copy link
Collaborator

Choose a reason for hiding this comment

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

force argument removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed comment.

* argument is true.
* @param {!Object} proto
* @param {!Array<Object>} list
* @param {string=} prefix
* @param {Array=} disallowed
*/
function applyPatchList(proto, list, prefix, disallowed) {
list.forEach(patch => proto && patch &&
utils.patchProperties(proto, patch, prefix, disallowed));
}

/** @param {string=} prefix */
export const applyPatches = (prefix) => {
const disallowed = prefix ? null : disallowedNativePatches;
for (let p in patchMap) {
const proto = getPatchPrototype(p);
patchMap[p].forEach(patch => proto && patch &&
utils.patchProperties(proto, patch, prefix, disallowed));
applyPatchList(proto, patchMap[p], prefix, disallowed);
}
}

const PROTO_IS_PATCHED = utils.SHADY_PREFIX + 'patchedProto';

const patchedProtos = new Map();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comment that this is making a pre-patched proto for TextNode, since any other instance patching is guaranteed to be an Element, that way patchNodeProto can assume it just needs to apply the element patches if there's not a pre-patched proto, eliminating a nodeType check.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, let's add Comment, ProcessingInstruction, and CDATASection to the pre-patched list.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

const TextPatchedProto = Object.create(Text.prototype);
TextPatchedProto[PROTO_IS_PATCHED] = true;
applyPatchList(TextPatchedProto, patchMap.EventTarget);
applyPatchList(TextPatchedProto, patchMap.Node);
applyPatchList(TextPatchedProto, patchMap.Text);
patchedProtos.set(Text.prototype, TextPatchedProto);

export const patchElementProto = (proto) => {
proto[PROTO_IS_PATCHED] = true;
applyPatchList(proto, patchMap.EventTarget);
applyPatchList(proto, patchMap.Node);
applyPatchList(proto, patchMap.Element);
applyPatchList(proto, patchMap.HTMLElement);
applyPatchList(proto, patchMap.HTMLSlotElement);
return proto;
}

export const patchNodeProto = (node) => {
if (node[PROTO_IS_PATCHED] || utils.isShadyRoot(node)) {
return;
}
const nativeProto = Object.getPrototypeOf(node);
let proto = patchedProtos.get(nativeProto);
if (!proto) {
proto = Object.create(nativeProto);
patchElementProto(proto);
patchedProtos.set(nativeProto, proto);
}
Object.setPrototypeOf(node, proto);
}

export const patchShadowOnElement = () => {
utils.patchProperties(Element.prototype, ElementShadowPatches);
}

export const addShadyPrefixedProperties = () => {
// perform shady patches
applyPatches(utils.SHADY_PREFIX);
Expand Down
34 changes: 22 additions & 12 deletions packages/shadydom/src/patches/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,6 @@ export const ElementPatches = utils.getOwnPropertyDescriptors({
this[utils.SHADY_PREFIX + 'setAttribute']('slot', value);
},

// Note: Can be patched on element prototype on all browsers.
// Must be patched on instance on browsers that support native Shadow DOM
// but do not have builtin accessors (old Chrome).
/** @this {Element} */
get shadowRoot() {
const nodeData = shadyDataForNode(this);
return nodeData && nodeData.publicRoot || null;
},

/** @this {Element} */
get className() {
return this.getAttribute('class') || '';
Expand Down Expand Up @@ -127,14 +118,33 @@ export const ElementPatches = utils.getOwnPropertyDescriptors({
// ensure that "class" attribute is fully removed if ShadyCSS does not keep scoping
this[utils.NATIVE_PREFIX + 'removeAttribute'](attr);
}
},
}

});

export const ElementShadowPatches = utils.getOwnPropertyDescriptors({
/**
* @this {Element}
* @param {!{mode: string}} options
*/
attachShadow(options) {
return attachShadow(this, options);
}
const root = attachShadow(this, options);
// TODO(sorvell): Workaround for CE not seeing shadowRoot in augmented
Copy link
Collaborator

Choose a reason for hiding this comment

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

"augmented" means "on-demand"? Why wasn't this a problem in regular noPatch mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only in on-demand patching mode since we're only patching here. Added better comment.

// noPatch mode. CE's attachShadow patch is overwritten by this patch
// and cannot set its own special tracking for shadowRoot. It does this
// to be able to see closed shadowRoots.
this['__CE_shadowRoot'] = root;
Copy link
Collaborator

Choose a reason for hiding this comment

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

CE polyfill also does internals.patchNode(shadowRoot); in its attachShadow patch -- this isn't needed in noPatch/on-demand because ShadyDOM's innerHTML/textContent instance patches would cause CE reactions via removeChild/appendChild?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's right.

return root;
},

// Note: Can be patched on element prototype on all browsers.
// Must be patched on instance on browsers that support native Shadow DOM
// but do not have builtin accessors (old Chrome).
/** @this {Element} */
get shadowRoot() {
const nodeData = shadyDataForNode(this);
return nodeData && nodeData.publicRoot || null;
},
});

Object.assign(ElementPatches, ElementShadowPatches);
46 changes: 25 additions & 21 deletions packages/shadydom/src/patches/HTMLElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,29 @@ export const HTMLElementPatches = utils.getOwnPropertyDescriptors({

});

eventPropertyNames.forEach(property => {
HTMLElementPatches[property] = {
/** @this {HTMLElement} */
set: function(fn) {
const shadyData = ensureShadyDataForNode(this);
const eventName = property.substring(2);
if (!shadyData.__onCallbackListeners) {
shadyData.__onCallbackListeners = {};
}
shadyData.__onCallbackListeners[property] && this.removeEventListener(eventName, shadyData.__onCallbackListeners[property]);
this[utils.SHADY_PREFIX + 'addEventListener'](eventName, fn);
shadyData.__onCallbackListeners[property] = fn;
},
/** @this {HTMLElement} */
get() {
const shadyData = shadyDataForNode(this);
return shadyData && shadyData.__onCallbackListeners && shadyData.__onCallbackListeners[property];
},
configurable: true
};
});
if (!utils.settings.preferPerformance) {

eventPropertyNames.forEach(property => {
HTMLElementPatches[property] = {
/** @this {HTMLElement} */
set: function(fn) {
const shadyData = ensureShadyDataForNode(this);
const eventName = property.substring(2);
if (!shadyData.__onCallbackListeners) {
shadyData.__onCallbackListeners = {};
}
shadyData.__onCallbackListeners[property] && this.removeEventListener(eventName, shadyData.__onCallbackListeners[property]);
this[utils.SHADY_PREFIX + 'addEventListener'](eventName, fn);
shadyData.__onCallbackListeners[property] = fn;
},
/** @this {HTMLElement} */
get() {
const shadyData = shadyDataForNode(this);
return shadyData && shadyData.__onCallbackListeners && shadyData.__onCallbackListeners[property];
},
configurable: true
};
});

}

4 changes: 3 additions & 1 deletion packages/shadydom/src/patches/Node.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ function scheduleObserver(node, addedNode, removedNode) {
if (observer) {
if (addedNode) {
if (addedNode.nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
observer.addedNodes.push(...addedNode.childNodes);
for (let i = 0, l = addedNode.childNodes.length; i < l; i++) {
observer.addedNodes.push(addedNode.childNodes[i]);
}
} else {
observer.addedNodes.push(addedNode);
}
Expand Down
36 changes: 28 additions & 8 deletions packages/shadydom/src/shadydom.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,17 @@ import {patchInsideElementAccessors, patchOutsideElementAccessors} from './patch
import {patchEvents, patchClick, composedPath} from './patch-events.js';
import {ShadyRoot} from './attach-shadow.js';
import {wrap, Wrapper} from './wrapper.js';
import {addShadyPrefixedProperties, applyPatches} from './patch-prototypes.js';
import {addShadyPrefixedProperties, applyPatches, patchShadowOnElement, patchElementProto} from './patch-prototypes.js';


if (utils.settings.inUse) {

const patch = utils.settings.hasDescriptors ? n => n : (node) => {
patchInsideElementAccessors(node);
patchOutsideElementAccessors(node);
return node;
};

let ShadyDOM = {
// TODO(sorvell): remove when Polymer does not depend on this.
'inUse': utils.settings.inUse,
Expand All @@ -41,11 +47,7 @@ if (utils.settings.inUse) {
// (2) does not have special (slot) children itself
// (3) and setting the property needs to provoke distribution (because
// a nested slot is added/removed)
'patch': (node) => {
patchInsideElementAccessors(node);
patchOutsideElementAccessors(node);
return node;
},
'patch': patch,
'isShadyRoot': utils.isShadyRoot,
'enqueue': enqueue,
'flush': flush,
Expand Down Expand Up @@ -74,7 +76,20 @@ if (utils.settings.inUse) {
// Integration point with ShadyCSS to disable styling MutationObserver,
// as ShadyDOM will now handle dynamic scoping.
'handlesDynamicScoping': true,
'wrap': utils.settings.noPatch ? wrap : (n) => n,
// Ensure the node is wrapped. This should be used when `noPatch` is set
// to ensure Shadow DOM compatible DOM operation.
'wrap': utils.settings.noPatch ? wrap : patch,
// When code should be compatible with `noPatch` `true` and `on-demand`
// settings, `wrapIfNeeded` can be used for optimal performance (v. `wrap`)
// for all DOM operations except the following: `appendChild` and
// `insertBefore` (when the node is being moved from a location where it
// was logically positioned in the DOM); when setting `className`/`class`;
// when calling `querySelector|All`; when setting `textContent` or
// `innerHTML`; `addEventListener`; and all scope specific API's like
// `getRootNode`, `isConnected`, `slot`, `assignedSlot`, `assignedNodes`.
'wrapIfNeeded': utils.settings.noPatch === true ||
(utils.settings.noPatch === 'on-demand' && !utils.settings.hasDescriptors) ?
wrap : patch,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's review the logic for what wrap and wrapIfNeeded get set to together (and cross-check against Polymer's versions)...

'Wrapper': Wrapper,
'composedPath': composedPath,
// Set to true to avoid patching regular platform property names. When set,
Expand All @@ -83,8 +98,10 @@ if (utils.settings.inUse) {
// This setting provides a small performance boost, but requires all DOM API
// access that requires Shadow DOM behavior to be proxied via `ShadyDOM.wrap`.
'noPatch': utils.settings.noPatch,
'patchOnDemand': utils.settings.patchOnDemand,
'nativeMethods': nativeMethods,
'nativeTree': nativeTree
'nativeTree': nativeTree,
'patchElementProto': patchElementProto
};

window['ShadyDOM'] = ShadyDOM;
Expand Down Expand Up @@ -121,6 +138,9 @@ if (utils.settings.inUse) {
applyPatches();
// Patch click event behavior only if we're patching
patchClick()
} else if (utils.settings.patchOnDemand) {
// in noPatch, do patch `attachShadow` and `shadowRoot`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// in noPatch, do patch `attachShadow` and `shadowRoot`
// in "on-demand" patching, do patch `attachShadow` and `shadowRoot`. This
// unconditional patch is what kicks off all other on-demand patching

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clarified.

patchShadowOnElement();
}

// For simplicity, patch events unconditionally.
Expand Down
3 changes: 2 additions & 1 deletion packages/shadydom/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ const desc = Object.getOwnPropertyDescriptor(Node.prototype, 'firstChild');
/* eslint-disable */
settings.hasDescriptors = Boolean(desc && desc.configurable && desc.get);
settings.inUse = settings['force'] || !settings.hasNativeShadowDOM;
settings.noPatch = settings['noPatch'] || false;
settings.noPatch = /** @type {string|boolean} */(settings['noPatch'] || false);
settings.preferPerformance = settings['preferPerformance'];
settings.patchOnDemand = (settings.noPatch === 'on-demand') && settings.hasDescriptors;
/* eslint-enable */

const IS_IE = navigator.userAgent.match('Trident');
Expand Down
Loading