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

fix(engine): shadow root childNodes #374

Merged
merged 9 commits into from
Jun 6, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,37 @@ describe('root', () => {
});

});

describe('childNodes', () => {
it('should return array of childnodes', () => {
function tmpl($api) {
return [
$api.h('div', {
key: 0,
}, [
$api.h('span', {
key: 1,
} , []),
]),
$api.h('p', {
key: 2,
} , [

]),
];
}
class MyComponent extends Element {
render() {
return tmpl;
}
}

const elem = createElement('x-shadow-child-nodes', { is: MyComponent });
document.body.appendChild(elem);
const children = elem.shadowRoot.childNodes;
expect(children.length).toBe(2);
expect(children[0]).toBe(elem.shadowRoot.querySelector('div'));
expect(children[1]).toBe(elem.shadowRoot.querySelector('p'));
});
});
});
182 changes: 182 additions & 0 deletions packages/lwc-engine/src/framework/dom/__tests__/traverse.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -530,3 +530,185 @@ describe('#parentNode and #parentElement', () => {
});
});
});


describe('#childNodes', () => {
it('should always return an empty array for slots not rendering default content', () => {
function tmpl($api, $cmp, $slotset) {
return [
$api.s('', {
key: 0,
}, [
$api.h('div', {
key: 2,
} ,[]),
], $slotset),
];
}
tmpl.slots = [''];

class HasSlot extends Element {
render() {
return tmpl;
}
}

class Parent extends Element {
render() {
return function ($api) {
return [
$api.c('x-child-node-with-slot', HasSlot, {}, [
$api.h('p', {
key: 1,
}, []),
]),
];
}
}
}

const elm = createElement('x-child-node-parent', { is: Parent });
document.body.appendChild(elm);
const slot = elm.shadowRoot.querySelector('x-child-node-with-slot').shadowRoot.querySelector('slot');
expect(slot.childNodes.length).toBe(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

expect(slot.childNodes).toHaveLength(0); will give a better failure message.

Same comment for cases below.

});

it('should return correct elements for slots rendering default content', () => {
function tmpl($api, $cmp, $slotset) {
return [
$api.s('', {
key: 0,
}, [
$api.h('div', {
key: 2,
} ,[]),
], $slotset),
];
}
tmpl.slots = [''];

class HasSlot extends Element {
render() {
return tmpl;
}
}

class Parent extends Element {
render() {
return function ($api) {
return [
$api.c('x-child-node-with-slot', HasSlot, {}, []),
];
}
}
}

const elm = createElement('x-child-node-parent', { is: Parent });
document.body.appendChild(elm);
const slot = elm.shadowRoot.querySelector('x-child-node-with-slot').shadowRoot.querySelector('slot');
expect(slot.childNodes.length).toBe(1);
});

it('should return correct elements for non-slot elements', () => {
class Parent extends Element {
render() {
return function ($api) {
return [
$api.h('div', {
key: 0,
}, [
$api.h('p', {
key: 1,
}, []),
]),
];
}
}
}

const elm = createElement('x-child-node-parent', { is: Parent });
document.body.appendChild(elm);
const child = elm.shadowRoot.querySelector('div');
const childNodes = child.childNodes;
expect(childNodes.length).toBe(1);
expect(childNodes[0]).toBe(elm.shadowRoot.querySelector('p'));
});

it('should return correct elements for custom elements when no children present', () => {
function tmpl($api) {
return [
$api.h('div', {
key: 3,
}, []),
]
}
class Child extends Element {
render() {
return tmpl;
}
}

class Parent extends Element {
render() {
return function ($api) {
return [
$api.h('div', {
key: 0,
}, [
$api.c('x-child', Child, {
key: 1,
}, []),
]),
];
}
}
}

const elm = createElement('x-child-node-parent', { is: Parent });
document.body.appendChild(elm);
const child = elm.shadowRoot.querySelector('x-child');
const childNodes = child.childNodes;
expect(childNodes.length).toBe(0);
});

it('should return correct elements for custom elements when children present', () => {
function tmpl($api, $cmp, $slotset) {
return [
$api.s('', {
key: 3,
}, [], $slotset),
]
}
class Child extends Element {
render() {
return tmpl;
}
}

class Parent extends Element {
render() {
return function ($api) {
return [
$api.h('div', {
key: 0,
}, [
$api.c('x-child', Child, {
key: 1,
}, [
$api.h('p', {
key: 4,
} ,[])
]),
]),
];
}
}
}

const elm = createElement('x-child-node-parent', { is: Parent });
document.body.appendChild(elm);
const child = elm.shadowRoot.querySelector('x-child');
const childNodes = child.childNodes;
expect(childNodes.length).toBe(1);
});
});
5 changes: 5 additions & 0 deletions packages/lwc-engine/src/framework/dom/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ const parentElementGetter = hasOwnProperty.call(Node.prototype, 'parentElement')
getOwnPropertyDescriptor(Node.prototype, 'parentElement')!.get! :
getOwnPropertyDescriptor(HTMLElement.prototype, 'parentElement')!.get!; // IE11

const childNodesGetter = hasOwnProperty.call(Node.prototype, 'childNodes') ?
getOwnPropertyDescriptor(Node.prototype, 'childNodes')!.get! :
getOwnPropertyDescriptor(HTMLElement.prototype, 'childNodes')!.get!; // IE11

export {
// Node.prototype
compareDocumentPosition,
Expand All @@ -87,6 +91,7 @@ export {
appendChild,
parentNodeGetter,
parentElementGetter,
childNodesGetter,

// Node
DOCUMENT_POSITION_CONTAINS,
Expand Down
13 changes: 10 additions & 3 deletions packages/lwc-engine/src/framework/dom/shadow-root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import assert from "../assert";
import { isUndefined, defineProperty, isNull, defineProperties, create, getOwnPropertyNames, forEach, hasOwnProperty, toString, isFalse } from "../language";
import { getShadowRootVM, VM } from "../vm";
import { addRootEventListener, removeRootEventListener } from "../events";
import { shadowRootQuerySelector, shadowRootQuerySelectorAll } from "./traverse";
import { shadowRootQuerySelector, shadowRootQuerySelectorAll, shadowRootChildNodes } from "./traverse";
import {
GlobalAOMProperties,
} from './attributes';
Expand Down Expand Up @@ -43,7 +43,15 @@ export function linkShadow(shadowRoot: ShadowRoot, vm: VM) {

const ArtificialShadowRootDescriptors: PropertyDescriptorMap = {
mode: { value: 'closed' },
childNodes: { value : [] },
childNodes: {
get(this: ShadowRoot): Element[] {
if (process.env.NODE_ENV !== 'production') {
Copy link
Contributor

Choose a reason for hiding this comment

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

this warning is tricky because it only appears when in fallback mode. On the other hands, blacklist will always throw. Maybe we need a grayList that wraps the original functionality with a warning, but still allow you to use it.

assert.logWarning(`this.template.childNodes returns a live nodelist and should not be relied upon. Instead, use this.template.querySelectorAll.`);
}
const vm = getShadowRootVM(this);
return shadowRootChildNodes(vm, vm.elm);
}
},
delegatesFocus: { value: false },
querySelector: {
value(this: ShadowRoot, selector: string): Element | null {
Expand Down Expand Up @@ -178,7 +186,6 @@ if (process.env.NODE_ENV !== 'production') {
});

const BlackListedShadowRootProperties = {
childNodes: 0,
firstChild: 0,
lastChild: 0,
ownerDocument: 0,
Expand Down
36 changes: 35 additions & 1 deletion packages/lwc-engine/src/framework/dom/traverse.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import assert from "../assert";
import { VM, getElementOwnerVM, isNodeOwnedByVM, OwnerKey } from "../vm";
import { VM, getElementOwnerVM, isNodeOwnedByVM, OwnerKey, getCustomElementVM } from "../vm";
import {
parentNodeGetter as nativeParentNodeGetter,
parentElementGetter as nativeParentElementGetter,
childNodesGetter as nativeChildNodesGetter,
} from "./node";
import {
querySelectorAll as nativeQuerySelectorAll,
Expand All @@ -13,6 +14,8 @@ import {
defineProperty,
defineProperties,
hasOwnProperty,
ArrayReduce,
ArraySlice,
} from "../language";
import { isBeingConstructed } from "../invoker";

Expand Down Expand Up @@ -47,6 +50,10 @@ export function parentElementDescriptorValue(this: HTMLElement): HTMLElement | S
return getShadowParent(this, vm, value);
}

export function shadowRootChildNodes(vm: VM, elm: Element) {
return getAllMatches(vm, elm.children);
}

function getAllMatches(vm: VM, nodeList: NodeList): HTMLElement[] {
return ArrayFilter.call(nodeList, (match) => {
const isOwned = isNodeOwnedByVM(vm, match);
Expand Down Expand Up @@ -96,6 +103,29 @@ export function shadowRootQuerySelectorAll(vm: VM, selector: string): HTMLElemen
return getAllMatches(vm, nodeList);
}

export function lightDomCustomElementChildNodes(this: HTMLElement) {
if (process.env.NODE_ENV !== 'production') {
assert.logWarning(`childNodes on ${this} returns a live nodelist which is not stable. Use querySelectorAll instead.`);
}
const ownerVM = getElementOwnerVM(this) as VM;
const customElementVM = getCustomElementVM(this);
const slots = shadowRootQuerySelectorAll(customElementVM, 'slot');
const children = ArrayReduce.call(slots, (seed, slot) => {
return seed.concat(ArraySlice.call(nativeChildNodesGetter.call(slot)));
}, []);

return getAllMatches(ownerVM, children);
}

export function lightDomChildNodes(this: HTMLElement) {
if (process.env.NODE_ENV !== 'production') {
assert.logWarning(`childNodes on ${this} returns a live nodelist which is not stable. Use querySelectorAll instead.`);
}
const ownerVM = getElementOwnerVM(this) as VM;
const children = nativeChildNodesGetter.call(this);
return getAllMatches(ownerVM, children);
}

const shadowDescriptors: PropertyDescriptorMap = {
querySelector: {
value: lightDomQuerySelector,
Expand All @@ -113,6 +143,10 @@ const shadowDescriptors: PropertyDescriptorMap = {
parentElement: {
get: parentElementDescriptorValue,
configurable: true,
},
childNodes: {
get: lightDomChildNodes,
configurable: true,
}
};

Expand Down
8 changes: 6 additions & 2 deletions packages/lwc-engine/src/framework/html-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { ArrayReduce, isString, isFunction } from "./language";
import { observeMutation, notifyMutation } from "./watcher";
import { CustomEvent, addEventListenerPatched, removeEventListenerPatched } from "./dom/event";
import { dispatchEvent } from "./dom/event-target";
import { lightDomQuerySelector, lightDomQuerySelectorAll } from "./dom/traverse";
import { lightDomQuerySelector, lightDomQuerySelectorAll, lightDomCustomElementChildNodes } from "./dom/traverse";

function ElementShadowRootGetter(this: HTMLElement): ShadowRoot | null {
const vm = getCustomElementVM(this);
Expand Down Expand Up @@ -50,7 +50,11 @@ const fallbackDescriptors = {
removeEventListener: {
value: removeEventListenerPatched,
configurable: true,
}
},
childNodes: {
get: lightDomCustomElementChildNodes,
configurable: true,
},
};

function getHTMLPropDescriptor(propName: string, descriptor: PropertyDescriptor) {
Expand Down
Loading