Skip to content

Commit

Permalink
fix(engine): including text nodes to childNodes (#389)
Browse files Browse the repository at this point in the history
* fix(engine): adding text nodes to childNodes

* fix(engine): remove uid module

* fix(engine): lint

* fix(engine): PR changes

* fix(engine): selenium visibility issue

* fix(engine): removing more browser execute statements
  • Loading branch information
davidturissini authored and diervo committed Jun 7, 2018
1 parent d1cf4a7 commit 9cbd853
Show file tree
Hide file tree
Showing 23 changed files with 238 additions and 142 deletions.
10 changes: 5 additions & 5 deletions packages/lwc-engine/src/3rdparty/snabbdom/snabbdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ export function init(
if (isElementVNode(vnode)) {
const { data, tag } = vnode;
const elm = (vnode.elm = isDef((i = data.ns))
? api.createElementNS(i, tag)
: api.createElement(tag));
? api.createElementNS(i, tag, data.uid)
: api.createElement(tag, data.uid));
if (isDef((i = data.hook)) && isDef(i.create)) {
i.create(emptyNode, vnode);
}
Expand All @@ -181,15 +181,15 @@ export function init(
}
}
} else if (!isUndef(vnode.text)) {
api.appendChild(elm, api.createTextNode(vnode.text));
api.appendChild(elm, api.createTextNode(vnode.text, vnode.data.uid));
}
if (isDef((i = data.hook)) && isDef(i.insert)) {
insertedVnodeQueue.push(vnode);
}
} else if (isTextVNode(vnode)) {
vnode.elm = api.createTextNode(vnode.text);
vnode.elm = api.createTextNode(vnode.text, vnode.data.uid);
} else if (isCommentVNode(vnode)) {
vnode.elm = api.createComment(vnode.text);
vnode.elm = api.createComment(vnode.text, vnode.data.uid);
} else if (isFragmentVNode(vnode)) {
vnode.elm = api.createFragment();
} else {
Expand Down
8 changes: 4 additions & 4 deletions packages/lwc-engine/src/3rdparty/snabbdom/types.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
export interface DOMAPI {
createFragment: () => DocumentFragment;
createElement: (tagName: string) => HTMLElement;
createElementNS: (namespaceURI: string, qualifiedName: string) => Element;
createTextNode: (text: string) => Text;
createComment: (text: string) => Comment;
createElement: (tagName: string, uid: number) => HTMLElement;
createElementNS: (namespaceURI: string, qualifiedName: string, uid: number) => Element;
createTextNode: (text: string, uid: number) => Text;
createComment: (text: string, uid: number) => Comment;
insertBefore: (parentNode: Node, newNode: Node, referenceNode: Node | null) => void;
removeChild: (node: Node, child: Node) => void;
appendChild: (node: Node, child: Node) => void;
Expand Down
4 changes: 2 additions & 2 deletions packages/lwc-engine/src/framework/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ export function f(items: any[]): any[] {

// [t]ext node
export function t(text: string): VText {
let sel, data = {}, children, key, elm; // tslint:disable-line
let sel, data = { uid: getCurrentOwnerId() }, children, key, elm; // tslint:disable-line
return {
nt: TEXT_NODE,
sel,
Expand All @@ -384,7 +384,7 @@ export function t(text: string): VText {
}

export function p(text: string): VComment {
let sel = '!', data = {}, children, key, elm; // tslint:disable-line
let sel = '!', data = { uid: getCurrentOwnerId() }, children, key, elm; // tslint:disable-line
return {
nt: COMMENT_NODE,
sel,
Expand Down
164 changes: 164 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 @@ -711,6 +711,128 @@ describe('#childNodes', () => {
const childNodes = child.childNodes;
expect(childNodes).toHaveLength(1);
});

it('should return child text content passed via slot', () => {
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.t('text')
]),
]),
];
}
}
}

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).toHaveLength(1);
expect(childNodes[0].nodeType).toBe(3);
expect(childNodes[0].textContent).toBe('text');
});

it('should not return child text from within template', () => {
function tmpl($api, $cmp, $slotset) {
return [
$api.t('text'),
]
}
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).toHaveLength(0);
});

it('should not return dynamic child text from within template', () => {
function tmpl($api, $cmp, $slotset) {
return [
$api.d($cmp.dynamicText),
]
}

class Parent extends Element {
get dynamicText() {
return 'text';
}

render() {
return tmpl;
}
}

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

it('should return correct childNodes from shadowRoot', () => {
class Parent extends Element {
render() {
return function ($api) {
return [
$api.h('div', {
key: 0,
}, []),
$api.t('text'),
];
}
}
}

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


Expand Down Expand Up @@ -898,4 +1020,46 @@ describe('assignedSlot', () => {
const child = elm.shadowRoot.querySelector('x-default-slot-custom-element');
expect(child.assignedSlot).toBe(null);
});

it('should return correct slot when text is slotted', () => {
class InsideSlot extends Element {}

function slottedHtml($api, $cmp, $slotset) {
return [
$api.s('', {
key: 1,
}, [], $slotset),
];
}

slottedHtml.slots = [""];

class WithSlot extends Element {
render() {
return slottedHtml;
}
}

function html($api) {
return [
$api.c('x-native-slotted-component-child', WithSlot, {
key: 0,
}, [
$api.t('text')
]),
];
}

class MyComponent extends Element {
render() {
return html;
}
}

const elm = createElement('x-native-slotted-component', { is: MyComponent });
document.body.appendChild(elm);
const slot = elm.shadowRoot.querySelector('x-native-slotted-component-child').shadowRoot.querySelector('slot');
const text = elm.shadowRoot.querySelector('x-native-slotted-component-child').childNodes[0];
expect(text.assignedSlot).toBe(slot);
});
});
2 changes: 1 addition & 1 deletion packages/lwc-engine/src/framework/dom/traverse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function parentElementDescriptorValue(this: HTMLElement): HTMLElement | S
}

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

function getAllMatches(vm: VM, nodeList: NodeList): HTMLElement[] {
Expand Down
18 changes: 0 additions & 18 deletions packages/lwc-engine/src/framework/modules/uid.ts

This file was deleted.

28 changes: 17 additions & 11 deletions packages/lwc-engine/src/framework/patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ import styles from "./modules/styles";
import classes from "./modules/classes";
import events from "./modules/events";
import token from "./modules/token";
import uid from "./modules/uid";
import { isNull, isUndefined, isFalse, isTrue } from './language';
import { parentNodeGetter } from "./dom/node";
import { VM } from "./vm";
import { VM, OwnerKey } from "./vm";
import { ViewModelReflection } from "./utils";

const {
Expand Down Expand Up @@ -43,17 +42,25 @@ export const htmlDomApi: DOMAPI = {
createFragment(): DocumentFragment {
return createDocumentFragment.call(document);
},
createElement(tagName: string): HTMLElement {
return createElement.call(document, tagName);
createElement(tagName: string, uid: number): HTMLElement {
const element = createElement.call(document, tagName);
element[OwnerKey] = uid;
return element;
},
createElementNS(namespaceURI: string, qualifiedName: string): Element {
return createElementNS.call(document, namespaceURI, qualifiedName);
createElementNS(namespaceURI: string, qualifiedName: string, uid: number): Element {
const element = createElementNS.call(document, namespaceURI, qualifiedName);
element[OwnerKey] = uid;
return element;
},
createTextNode(text: string): Text {
return createTextNode.call(document, text);
createTextNode(text: string, uid: number): Text {
const textNode = createTextNode.call(document, text);
textNode[OwnerKey] = uid;
return textNode;
},
createComment(text: string): Comment {
return createComment.call(document, text);
createComment(text: string, uid: number): Comment {
const comment = createComment.call(document, text);
comment[OwnerKey] = uid;
return comment;
},
insertBefore(parent: Node, newNode: Node, referenceNode: Node | null) {
const vm: VM = parent[ViewModelReflection];
Expand Down Expand Up @@ -93,7 +100,6 @@ const patchVNode = init([
styles,
events,
token,
uid,
], htmlDomApi);

const patchChildren = patchVNode.children;
Expand Down
16 changes: 1 addition & 15 deletions packages/lwc-engine/src/framework/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { createVM, removeVM, appendVM, renderVM, getCustomElementVM } from "./vm
import { ComponentConstructor } from "./component";
import { ViewModelReflection, resolveCircularModuleDependency } from "./utils";
import { setAttribute } from "./dom/element";
import { shadowRootQuerySelector, shadowRootQuerySelectorAll, shadowRootChildNodes } from "./dom/traverse";
import { shadowRootQuerySelector, shadowRootQuerySelectorAll } from "./dom/traverse";

const { removeChild, appendChild, insertBefore, replaceChild } = Node.prototype;
const ConnectingSlot = Symbol();
Expand Down Expand Up @@ -61,16 +61,6 @@ function querySelectorAllPatchedRoot(this: HTMLElement, selector): HTMLElement[]
return shadowRootQuerySelectorAll(vm, selector);
}

function childNodesPatchedRoot(this: HTMLElement): HTMLElement[] {
const vm = getCustomElementVM(this);
if (process.env.NODE_ENV === 'test') {
// TODO: remove this backward compatibility branch.
assert.logError(`Using elm.childNodes on a root element created via createElement() in a test will return an empty NodeList very soon to enforce ShadowDOM semantics, instead use elm.shadowRoot.childNodes.`);
}

return shadowRootChildNodes(vm, this);
}

const rootNodeFallbackDescriptors = {
querySelectorAll: {
value: querySelectorAllPatchedRoot,
Expand All @@ -80,10 +70,6 @@ const rootNodeFallbackDescriptors = {
value: querySelectorPatchedRoot,
configurable: true,
},
childNodes: {
get: childNodesPatchedRoot,
configurable: true,
},
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,9 @@ describe('event target query selector', () => {
it('should return correct elements', function () {
const hostContainer = browser.element('child-template-element-queryselector');
hostContainer.click();

const values = browser.execute(function () {
var shadowDiv = document.querySelector('.shadow-div');
var lightDiv = document.querySelector('.light-div');
return {
lightDivSelected: lightDiv.getAttribute('data-selected'),
shadowDivSelected: shadowDiv.getAttribute('data-selected'),
};
});
assert.equal(values.value.shadowDivSelected, null);
assert.equal(values.value.lightDivSelected, 'true');
const shadowDiv = browser.element('.shadow-div');
assert.equal(shadowDiv.getAttribute('data-selected'), null);
const lightDiv = browser.element('.light-div');
assert.equal(lightDiv.getAttribute('data-selected'), 'true');
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<x-parent>
<div class="light-div">Not Shadow Div</div>
<span class="light-div">Not Shadow Div</span>
</x-parent>
</template>
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Element } from 'engine';
export default class LightdomQuerySelector extends Element {
connectedCallback() {
this.addEventListener('click', () => {
this.template.querySelector('x-parent').querySelector('div').setAttribute('data-selected', 'true');
this.template.querySelector('x-parent').querySelector('span').setAttribute('data-selected', 'true');
});
}
}
Loading

0 comments on commit 9cbd853

Please sign in to comment.