From 4c3165c8a37fde48cb9acedc745c9f60613330c7 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 3 Oct 2016 09:30:42 -0600 Subject: [PATCH 01/41] Implement render for custom virtual DOM implementation --- src/dom.js | 48 ++-------------------------------- src/render.js | 33 +++++++++++++++++++++++ test/unit/render.test.js | 56 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 46 deletions(-) create mode 100644 src/render.js create mode 100644 test/unit/render.test.js diff --git a/src/dom.js b/src/dom.js index 337bb69..3fb2061 100644 --- a/src/dom.js +++ b/src/dom.js @@ -1,47 +1,3 @@ -import h from 'virtual-dom/h' -import svg from 'virtual-dom/virtual-hyperscript/svg' -import RefHook from './ref-hook' -import ComponentWidget from './component-widget' -import SVG_TAGS from './svg-tags' -import SVG_ATTRIBUTE_TRANSLATIONS from './svg-attribute-translations' - -// This function is invoked by JSX expressions to construct `virtual-dom` trees. -// -// For normal HTML tags (when the `tag` parameter is a string), we call through -// to `h`, the virtual-dom library's method for building virtual nodes. -// -// If the user passes a *constructor*, however, we build a special "widget" -// instance to manage the component. [Widgets](https://github.com/Matt-Esch/virtual-dom/blob/master/docs/widget.md) -// are an extension mechanism in the virtual-dom API that allows us to control -// a particular DOM element directly using native DOM APIs. This allows the -// component to manage its DOM element using whatever mechanism it desires, -// independent of the fact that its containing DOM tree is managed by this -// particular library. For more information, see `./component-widget.js`. -export default function dom (tag, properties, ...children) { - if (typeof tag === 'function') { - return new ComponentWidget(tag, properties || {}, children) - } else { - // Etch allows for a special `ref` property, which will automatically create - // named references to DOM elements containing the property. We implement - // this with virtual-dom's [hook system](https://github.com/Matt-Esch/virtual-dom/blob/master/docs/hooks.md), - // which allows a particular property to be associated with behavior when - // the element is created or destroyed. - if (properties && properties.ref) { - properties.ref = new RefHook(properties.ref) - } - - if (SVG_TAGS.has(tag)) { - if (properties) { - for (let property of Object.keys(properties)) { - if (SVG_ATTRIBUTE_TRANSLATIONS.hasOwnProperty(property)) { - properties[SVG_ATTRIBUTE_TRANSLATIONS[property]] = properties[property] - delete properties[property] - } - } - } - return svg(tag, properties, children) - } else { - return h(tag, properties, children) - } - } +export default function vdom (tag, props, ...children) { + return {tag, props, children} } diff --git a/src/render.js b/src/render.js new file mode 100644 index 0000000..f7161d7 --- /dev/null +++ b/src/render.js @@ -0,0 +1,33 @@ +export default function render (virtualNode) { + if (typeof virtualNode === 'string') { + return document.createTextNode(virtualNode) + } else { + const {tag, props, children} = virtualNode + + if (typeof tag === 'function') { + const component = new tag(props, children) + return component.element + } else { + const element = document.createElement(tag) + if (props) setAttributes(element, props) + if (children) addChildren(element, children) + return element + } + } +} + +function setAttributes (element, props) { + for (let name in props) { + element.setAttribute(name, props[name]) + } +} + +function addChildren (parent, children) { + for (let child of children) { + if (Array.isArray(child)) { + addChildren(parent, child) + } else { + parent.appendChild(render(child)) + } + } +} diff --git a/test/unit/render.test.js b/test/unit/render.test.js new file mode 100644 index 0000000..946d4b9 --- /dev/null +++ b/test/unit/render.test.js @@ -0,0 +1,56 @@ +/** @jsx dom */ + +import {assert} from 'chai' + +import dom from '../../src/dom' +import render from '../../src/render' + +describe('render', () => { + it('constructs DOM elements from virtual DOM trees', function () { + const element = render( +
+
+ Hello World + +
+ ) + + assert.equal(element.outerHTML, ` +
+
+ Hello World + +
+ `.replace(/\n\s*/g, '')) + }) + + it('constructs child components and embeds their elements', function () { + class Component { + constructor (props, children) { + this.element = render( + + {children} + + ) + } + } + + const element = render( +
+ +
+
+ +
+ ) + + assert.equal(element.outerHTML, ` +
+ +
+
+
+
+ `.replace(/\n\s*/g, '')) + }) +}) From f2f7ff224e1d68b984c8ee0f3c41a978e5bc1e9e Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 8 Oct 2016 22:29:40 -0600 Subject: [PATCH 02/41] Implement attribute patching --- src/patch.js | 21 +++++++++++++++++++++ src/render.js | 12 ++++++++---- src/virtual-nodes-by-element.js | 2 ++ test/unit/patch.test.js | 17 +++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 src/patch.js create mode 100644 src/virtual-nodes-by-element.js create mode 100644 test/unit/patch.test.js diff --git a/src/patch.js b/src/patch.js new file mode 100644 index 0000000..0fe0725 --- /dev/null +++ b/src/patch.js @@ -0,0 +1,21 @@ +import virtualNodesByElement from './virtual-nodes-by-element' + +export default function patch (element, newVirtualNode) { + const oldVirtualNode = virtualNodesByElement.get(element) + if (!oldVirtualNode) throw new Error('No virtual node found for element. You can currently only patch elements produced via the render or patch functions.') + + patchAttributes(element, oldVirtualNode, newVirtualNode) +} + +function patchAttributes(element, oldVirtualNode, newVirtualNode) { + const oldProps = oldVirtualNode.props + const newProps = newVirtualNode.props + + for (let prop in oldProps) { + if (!(prop in newProps)) element.removeAttribute(prop) + } + for (let prop in newProps) { + const newValue = newProps[prop] + if (newValue !== oldProps[prop]) element.setAttribute(prop, newValue) + } +} diff --git a/src/render.js b/src/render.js index f7161d7..a0a808c 100644 --- a/src/render.js +++ b/src/render.js @@ -1,19 +1,23 @@ +import virtualNodesByElement from './virtual-nodes-by-element' + export default function render (virtualNode) { + let element if (typeof virtualNode === 'string') { - return document.createTextNode(virtualNode) + element = document.createTextNode(virtualNode) } else { const {tag, props, children} = virtualNode if (typeof tag === 'function') { const component = new tag(props, children) - return component.element + element = component.element } else { - const element = document.createElement(tag) + element = document.createElement(tag) if (props) setAttributes(element, props) if (children) addChildren(element, children) - return element } } + virtualNodesByElement.set(element, virtualNode) + return element } function setAttributes (element, props) { diff --git a/src/virtual-nodes-by-element.js b/src/virtual-nodes-by-element.js new file mode 100644 index 0000000..244dd1b --- /dev/null +++ b/src/virtual-nodes-by-element.js @@ -0,0 +1,2 @@ +const virtualNodesByElement = new WeakMap() +export default virtualNodesByElement diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js new file mode 100644 index 0000000..24c070d --- /dev/null +++ b/test/unit/patch.test.js @@ -0,0 +1,17 @@ +/** @jsx dom */ + +import {assert} from 'chai' + +import dom from '../../src/dom' +import render from '../../src/render' +import patch from '../../src/patch' + +describe('patch', () => { + describe('attributes', function () { + it('can add, remove, and update attributes', function () { + const element = render(
) + patch(element,
) + assert.deepEqual(element, render(
)) + }) + }) +}) From acf5ea5eb08c55b533c7421454f5cd2b1f262065 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 8 Oct 2016 23:40:13 -0600 Subject: [PATCH 03/41] Start on patchChildren Implementation is cribbed from snabbdom, but pruned down a bit. Still need to test all the cases. --- src/dom.js | 15 ++++++- src/patch.js | 95 ++++++++++++++++++++++++++++++++++++++--- src/render.js | 11 ++--- test/unit/patch.test.js | 23 ++++++++-- 4 files changed, 128 insertions(+), 16 deletions(-) diff --git a/src/dom.js b/src/dom.js index 3fb2061..92fc8b9 100644 --- a/src/dom.js +++ b/src/dom.js @@ -1,3 +1,16 @@ -export default function vdom (tag, props, ...children) { +export default function dom (tag, props, ...children) { + + for (let i = 0; i < children.length;) { + const child = children[i] + if (Array.isArray(child)) { + children.splice(i, 1, ...child) + } else if (typeof child === 'string') { + children.splice(i, 1, {text: child}) + i++ + } else { + i++ + } + } + return {tag, props, children} } diff --git a/src/patch.js b/src/patch.js index 0fe0725..19dd8e7 100644 --- a/src/patch.js +++ b/src/patch.js @@ -1,16 +1,20 @@ +import render from './render' import virtualNodesByElement from './virtual-nodes-by-element' export default function patch (element, newVirtualNode) { const oldVirtualNode = virtualNodesByElement.get(element) if (!oldVirtualNode) throw new Error('No virtual node found for element. You can currently only patch elements produced via the render or patch functions.') - - patchAttributes(element, oldVirtualNode, newVirtualNode) + patchVirtualNode(oldVirtualNode, newVirtualNode) } -function patchAttributes(element, oldVirtualNode, newVirtualNode) { - const oldProps = oldVirtualNode.props - const newProps = newVirtualNode.props +function patchVirtualNode (oldVirtualNode, newVirtualNode) { + const element = oldVirtualNode.element + patchAttributes(element, oldVirtualNode.props, newVirtualNode.props) + patchChildren(element, oldVirtualNode.children, newVirtualNode.children) + newVirtualNode.element = element +} +function patchAttributes(element, oldProps, newProps) { for (let prop in oldProps) { if (!(prop in newProps)) element.removeAttribute(prop) } @@ -19,3 +23,84 @@ function patchAttributes(element, oldVirtualNode, newVirtualNode) { if (newValue !== oldProps[prop]) element.setAttribute(prop, newValue) } } + +function patchChildren (parentElement, oldChildren, newChildren) { + let oldStartIndex = 0 + let oldEndIndex = oldChildren.length - 1 + let oldStartChild = oldChildren[0] + let oldEndChild = oldChildren[oldEndIndex] + + let newStartIndex = 0 + let newEndIndex = newChildren.length - 1 + let newStartChild = newChildren[0] + let newEndChild = newChildren[newEndIndex] + + let oldIndicesByKey + + while (oldStartIndex <= oldEndIndex && newStartIndex <= newEndIndex) { + if (!oldStartChild) { + oldStartChild = oldChildren[++oldStartIndex] + } else if (!oldEndChild) { + oldEndChild = oldChildren[--oldEndIndex] + } else if (keysEqual(oldStartChild, newStartChild)) { + patchVirtualNode(oldStartChild, newStartChild) + oldStartChild = oldChildren[++oldStartIndex] + newStartChild = newChildren[++newStartIndex] + } else if (keysEqual(oldEndChild, newEndChild)) { + patchVirtualNode(oldEndChild, newEndChild) + oldEndChild = oldChildren[--oldEndIndex] + newEndChild = newChildren[--newEndIndex] + } else if (keysEqual(oldStartChild, newEndChild)) { + patchVirtualNode(oldStartChild, newEndChild) + parentElement.insertBefore(oldStartChild.element, oldEndChild.element.nextSibling) + oldStartChild = oldChildren[++oldStartIndex] + newEndChild = newChildren[--newEndIndex] + } else if (keysEqual(oldEndChild, newStartChild)) { + patchVirtualNode(oldEndChild, newStartChild) + parentElement.insertBefore(oldEndChild.element, oldStartChild.element); + oldEndChild = oldChildren[--oldEndIndex] + newStartChild = newChildren[++newStartIndex] + } else { + if (!oldIndicesByKey) oldIndicesByKey = mapOldKeysToIndices(oldChildren, oldStartIndex, oldEndIndex) + const oldIndex = oldIndicesByKey[newStartChild.key] + if (oldIndex == null) { + parentElement.insertBefore(render(newStartChild), oldStartChild.element) + newStartChild = newChildren[++newStartIndex] + } else { + const oldChildToMove = oldChildren[oldIndex] + patchVirtualNode(oldChildToMove, newStartChild) + oldChildren[oldIndex] = undefined + parentElement.insertBefore(oldChildToMove.element, oldStartChild.element) + newStartChild = newChildren[++newStartIndex] + } + } + } + + if (oldStartIndex > oldEndIndex) { + const subsequentElement = newChildren[newEndIndex + 1] ? newChildren[newEndIndex + 1].element : null + for (let i = newStartIndex; i <= newEndIndex; i++) { + parentElement.insertBefore(render(newChildren[i]), subsequentElement) + } + } else if (newStartIndex > newEndIndex) { + for (let i = oldStartIndex; i <= oldEndIndex; i++) { + oldChildren[i].element.remove() + } + } +} + +function keysEqual (oldVirtualNode, newVirtualNode) { + return ( + oldVirtualNode.props + && newVirtualNode.props + && oldVirtualNode.props.key === newVirtualNode.props.key + ) +} + +function mapOldKeysToIndices (children, startIndex, endIndex) { + let oldIndicesByKey = {} + for (let i = startIndex; i <= endIndex; i++) { + const key = children[i].key + if (key) oldIndicesByKey[key] = i + } + return oldIndicesByKey; +} diff --git a/src/render.js b/src/render.js index a0a808c..39d0f8d 100644 --- a/src/render.js +++ b/src/render.js @@ -2,8 +2,8 @@ import virtualNodesByElement from './virtual-nodes-by-element' export default function render (virtualNode) { let element - if (typeof virtualNode === 'string') { - element = document.createTextNode(virtualNode) + if (virtualNode.text) { + element = document.createTextNode(virtualNode.text) } else { const {tag, props, children} = virtualNode @@ -16,6 +16,7 @@ export default function render (virtualNode) { if (children) addChildren(element, children) } } + virtualNode.element = element virtualNodesByElement.set(element, virtualNode) return element } @@ -28,10 +29,6 @@ function setAttributes (element, props) { function addChildren (parent, children) { for (let child of children) { - if (Array.isArray(child)) { - addChildren(parent, child) - } else { - parent.appendChild(render(child)) - } + parent.appendChild(render(child)) } } diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 24c070d..376598a 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -9,9 +9,26 @@ import patch from '../../src/patch' describe('patch', () => { describe('attributes', function () { it('can add, remove, and update attributes', function () { - const element = render(
) - patch(element,
) - assert.deepEqual(element, render(
)) + assertValidPatch(
,
) + }) + }) + + describe('keyed children', function () { + it('can append children', function () { + assertValidPatch( +
{keyedSpans('a', 'b')}
, +
{keyedSpans('a', 'b', 'c', 'd')}
+ ) }) }) }) + +function assertValidPatch (oldVirtualNode, newVirtualNode) { + const element = render(oldVirtualNode) + patch(element, newVirtualNode) + assert.deepEqual(element, render(newVirtualNode)) +} + +function keyedSpans (...elements) { + return elements.map(element => {element}) +} From 7f21c802a979039d8f6c33eb89a2722593a40850 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 10:49:51 -0600 Subject: [PATCH 04/41] Complete coverage for patching keyed children --- package.json | 1 + test/unit/patch.test.js | 113 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 111 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 2a2b2b4..5a400e1 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "electron-mocha": "git://github.com/nathansobo/electron-mocha.git#interactive-option", "electron-prebuilt": "^0.30.1", "mocha": "^2.1.0", + "random-seed": "^0.3.0", "standard": "^4.5.4" }, "dependencies": { diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 376598a..6d847f4 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -1,6 +1,7 @@ /** @jsx dom */ import {assert} from 'chai' +import Random from 'random-seed' import dom from '../../src/dom' import render from '../../src/render' @@ -14,21 +15,127 @@ describe('patch', () => { }) describe('keyed children', function () { - it('can append children', function () { + it('can add and remove children at the end', function () { assertValidPatch(
{keyedSpans('a', 'b')}
,
{keyedSpans('a', 'b', 'c', 'd')}
) + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('a', 'b')}
+ ) + }) + + it('can add and remove children at the beginning', function () { + assertValidPatch( +
{keyedSpans('c', 'd')}
, +
{keyedSpans('a', 'b', 'c', 'd')}
+ ) + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('c', 'd')}
+ ) + }) + + it('can add and remove in the middle of existing children', function () { + assertValidPatch( +
{keyedSpans('a', 'd')}
, +
{keyedSpans('a', 'b', 'c', 'd')}
+ ) + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('a', 'd')}
+ ) + }) + + it('can add and remove children at both ends', function () { + assertValidPatch( +
{keyedSpans('c', 'd')}
, +
{keyedSpans('a', 'b', 'c', 'd', 'e', 'f')}
+ ) + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd', 'e', 'f')}
, +
{keyedSpans('c', 'd')}
+ ) + }) + + it('can add children to an empty parent and remove all children', function () { + assertValidPatch( +
, +
{keyedSpans('a', 'b')}
+ ) + assertValidPatch( +
{keyedSpans('a', 'b')}
, +
+ ) + }) + + it('can move children to the right and left', function () { + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('b', 'c', 'a', 'd')}
+ ) + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('a', 'd', 'b', 'c')}
+ ) + }) + + it('can move children to the start and end', function () { + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('a', 'c', 'd', 'b')}
+ ) + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('c', 'a', 'b', 'd')}
+ ) + }) + + it('can swap the first and last child', function () { + assertValidPatch( +
{keyedSpans('a', 'b', 'c', 'd')}
, +
{keyedSpans('d', 'c', 'd', 'a')}
+ ) + }) + + it('can update to randomized reorderings of children', function () { + for (let i = 0; i < 20; i++) { + const seed = Date.now() + const randomGenerator = Random(seed) + assertValidPatch( +
{keyedSpans(...randomLetters(randomGenerator))}
, +
{keyedSpans(...randomLetters(randomGenerator))}
, + seed + ) + } }) }) }) -function assertValidPatch (oldVirtualNode, newVirtualNode) { +function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { const element = render(oldVirtualNode) patch(element, newVirtualNode) - assert.deepEqual(element, render(newVirtualNode)) + const message = seed != null ? `Invalid patch for seed ${seed}` : undefined + assert.deepEqual(element, render(newVirtualNode), message) } function keyedSpans (...elements) { return elements.map(element => {element}) } + +function randomLetters (randomGenerator) { + const letters = [] + const usedLetters = new Set() + const count = randomGenerator(27) + + for (let i = 0; i < count; i++) { + const letter = String.fromCharCode('a'.charCodeAt(0) + randomGenerator(27)) + if (!usedLetters.has(letter)) { + letters.push(letter) + usedLetters.add(letter) + } + } + + return letters +} From 87e1ad5a9755604499a4172f4ab72aa8421bca5b Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 15:14:31 -0600 Subject: [PATCH 05/41] Fix key handling --- src/patch.js | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/patch.js b/src/patch.js index 19dd8e7..2d29f3d 100644 --- a/src/patch.js +++ b/src/patch.js @@ -42,27 +42,28 @@ function patchChildren (parentElement, oldChildren, newChildren) { oldStartChild = oldChildren[++oldStartIndex] } else if (!oldEndChild) { oldEndChild = oldChildren[--oldEndIndex] - } else if (keysEqual(oldStartChild, newStartChild)) { + } else if (isEqual(oldStartChild, newStartChild)) { patchVirtualNode(oldStartChild, newStartChild) oldStartChild = oldChildren[++oldStartIndex] newStartChild = newChildren[++newStartIndex] - } else if (keysEqual(oldEndChild, newEndChild)) { + } else if (isEqual(oldEndChild, newEndChild)) { patchVirtualNode(oldEndChild, newEndChild) oldEndChild = oldChildren[--oldEndIndex] newEndChild = newChildren[--newEndIndex] - } else if (keysEqual(oldStartChild, newEndChild)) { + } else if (isEqual(oldStartChild, newEndChild)) { patchVirtualNode(oldStartChild, newEndChild) parentElement.insertBefore(oldStartChild.element, oldEndChild.element.nextSibling) oldStartChild = oldChildren[++oldStartIndex] newEndChild = newChildren[--newEndIndex] - } else if (keysEqual(oldEndChild, newStartChild)) { + } else if (isEqual(oldEndChild, newStartChild)) { patchVirtualNode(oldEndChild, newStartChild) parentElement.insertBefore(oldEndChild.element, oldStartChild.element); oldEndChild = oldChildren[--oldEndIndex] newStartChild = newChildren[++newStartIndex] } else { if (!oldIndicesByKey) oldIndicesByKey = mapOldKeysToIndices(oldChildren, oldStartIndex, oldEndIndex) - const oldIndex = oldIndicesByKey[newStartChild.key] + const key = getKey(newStartChild) + const oldIndex = key ? oldIndicesByKey[key] : null if (oldIndex == null) { parentElement.insertBefore(render(newStartChild), oldStartChild.element) newStartChild = newChildren[++newStartIndex] @@ -83,23 +84,27 @@ function patchChildren (parentElement, oldChildren, newChildren) { } } else if (newStartIndex > newEndIndex) { for (let i = oldStartIndex; i <= oldEndIndex; i++) { - oldChildren[i].element.remove() + const child = oldChildren[i] + if (child) child.element.remove() } } } -function keysEqual (oldVirtualNode, newVirtualNode) { +function isEqual (oldVirtualNode, newVirtualNode) { return ( - oldVirtualNode.props - && newVirtualNode.props - && oldVirtualNode.props.key === newVirtualNode.props.key + getKey(oldVirtualNode) === getKey(newVirtualNode) + && oldVirtualNode.tag === newVirtualNode.tag ) } +function getKey (virtualNode) { + return virtualNode.props ? virtualNode.props.key : null +} + function mapOldKeysToIndices (children, startIndex, endIndex) { let oldIndicesByKey = {} for (let i = startIndex; i <= endIndex; i++) { - const key = children[i].key + const key = getKey(children[i]) if (key) oldIndicesByKey[key] = i } return oldIndicesByKey; From 4a477c4d08dbfc1539b25a28707df81e993cd355 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 15:15:21 -0600 Subject: [PATCH 06/41] Add tests for patching unkeyed children and implement text node patching --- src/patch.js | 14 +++++++--- test/unit/patch.test.js | 62 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/patch.js b/src/patch.js index 2d29f3d..5be3177 100644 --- a/src/patch.js +++ b/src/patch.js @@ -8,10 +8,16 @@ export default function patch (element, newVirtualNode) { } function patchVirtualNode (oldVirtualNode, newVirtualNode) { - const element = oldVirtualNode.element - patchAttributes(element, oldVirtualNode.props, newVirtualNode.props) - patchChildren(element, oldVirtualNode.children, newVirtualNode.children) - newVirtualNode.element = element + const oldElement = oldVirtualNode.element + + if (newVirtualNode.text) { + oldElement.nodeValue = newVirtualNode.text + } else { + patchChildren(oldElement, oldVirtualNode.children, newVirtualNode.children) + patchAttributes(oldElement, oldVirtualNode.props, newVirtualNode.props) + } + + newVirtualNode.element = oldElement } function patchAttributes(element, oldProps, newProps) { diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 6d847f4..8c06455 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -111,6 +111,64 @@ describe('patch', () => { } }) }) + + describe('unkeyed children', function () { + it('can append elements', function () { + assertValidPatch( +
Hello
, +
HelloWorld
+ ) + assertValidPatch( +
Hello
, +
Hello
World
+ ) + }) + + it('can prepend elements', function () { + assertValidPatch( +
World
, +
HelloWorld
+ ) + assertValidPatch( +
World
, +
Hello
World
+ ) + }) + + it('can change text children', function () { + assertValidPatch( +
HelloWorld
, +
GoodnightMoon
+ ) + }) + + it('can replace an element child with a text child and vice versa', function () { + assertValidPatch( +
HelloWorld
, +
GoodnightWorld
+ ) + assertValidPatch( +
GoodnightWorld
, +
HelloWorld
+ ) + assertValidPatch( +
HelloWorld
, +
GoodnightMoon
+ ) + }) + + it('can update to randomized reorderings of children', function () { + for (let i = 0; i < 20; i++) { + const seed = Date.now() + const randomGenerator = Random(seed) + assertValidPatch( +
{spans(...randomLetters(randomGenerator))}
, +
{spans(...randomLetters(randomGenerator))}
, + seed + ) + } + }) + }) }) function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { @@ -120,6 +178,10 @@ function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { assert.deepEqual(element, render(newVirtualNode), message) } +function spans (...elements) { + return elements.map(element => {element}) +} + function keyedSpans (...elements) { return elements.map(element => {element}) } From e1ff83732cf0af84b6bb957462158444024bfd95 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 15:32:32 -0600 Subject: [PATCH 07/41] Pass old virtual node to patch to eliminate global weak map --- src/patch.js | 19 ++++++------------- src/render.js | 3 --- src/virtual-nodes-by-element.js | 2 -- test/unit/patch.test.js | 2 +- 4 files changed, 7 insertions(+), 19 deletions(-) delete mode 100644 src/virtual-nodes-by-element.js diff --git a/src/patch.js b/src/patch.js index 5be3177..cb778b3 100644 --- a/src/patch.js +++ b/src/patch.js @@ -1,13 +1,6 @@ import render from './render' -import virtualNodesByElement from './virtual-nodes-by-element' -export default function patch (element, newVirtualNode) { - const oldVirtualNode = virtualNodesByElement.get(element) - if (!oldVirtualNode) throw new Error('No virtual node found for element. You can currently only patch elements produced via the render or patch functions.') - patchVirtualNode(oldVirtualNode, newVirtualNode) -} - -function patchVirtualNode (oldVirtualNode, newVirtualNode) { +export default function patch (oldVirtualNode, newVirtualNode) { const oldElement = oldVirtualNode.element if (newVirtualNode.text) { @@ -49,20 +42,20 @@ function patchChildren (parentElement, oldChildren, newChildren) { } else if (!oldEndChild) { oldEndChild = oldChildren[--oldEndIndex] } else if (isEqual(oldStartChild, newStartChild)) { - patchVirtualNode(oldStartChild, newStartChild) + patch(oldStartChild, newStartChild) oldStartChild = oldChildren[++oldStartIndex] newStartChild = newChildren[++newStartIndex] } else if (isEqual(oldEndChild, newEndChild)) { - patchVirtualNode(oldEndChild, newEndChild) + patch(oldEndChild, newEndChild) oldEndChild = oldChildren[--oldEndIndex] newEndChild = newChildren[--newEndIndex] } else if (isEqual(oldStartChild, newEndChild)) { - patchVirtualNode(oldStartChild, newEndChild) + patch(oldStartChild, newEndChild) parentElement.insertBefore(oldStartChild.element, oldEndChild.element.nextSibling) oldStartChild = oldChildren[++oldStartIndex] newEndChild = newChildren[--newEndIndex] } else if (isEqual(oldEndChild, newStartChild)) { - patchVirtualNode(oldEndChild, newStartChild) + patch(oldEndChild, newStartChild) parentElement.insertBefore(oldEndChild.element, oldStartChild.element); oldEndChild = oldChildren[--oldEndIndex] newStartChild = newChildren[++newStartIndex] @@ -75,7 +68,7 @@ function patchChildren (parentElement, oldChildren, newChildren) { newStartChild = newChildren[++newStartIndex] } else { const oldChildToMove = oldChildren[oldIndex] - patchVirtualNode(oldChildToMove, newStartChild) + patch(oldChildToMove, newStartChild) oldChildren[oldIndex] = undefined parentElement.insertBefore(oldChildToMove.element, oldStartChild.element) newStartChild = newChildren[++newStartIndex] diff --git a/src/render.js b/src/render.js index 39d0f8d..3347441 100644 --- a/src/render.js +++ b/src/render.js @@ -1,5 +1,3 @@ -import virtualNodesByElement from './virtual-nodes-by-element' - export default function render (virtualNode) { let element if (virtualNode.text) { @@ -17,7 +15,6 @@ export default function render (virtualNode) { } } virtualNode.element = element - virtualNodesByElement.set(element, virtualNode) return element } diff --git a/src/virtual-nodes-by-element.js b/src/virtual-nodes-by-element.js deleted file mode 100644 index 244dd1b..0000000 --- a/src/virtual-nodes-by-element.js +++ /dev/null @@ -1,2 +0,0 @@ -const virtualNodesByElement = new WeakMap() -export default virtualNodesByElement diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 8c06455..ef11614 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -173,7 +173,7 @@ describe('patch', () => { function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { const element = render(oldVirtualNode) - patch(element, newVirtualNode) + patch(oldVirtualNode, newVirtualNode) const message = seed != null ? `Invalid patch for seed ${seed}` : undefined assert.deepEqual(element, render(newVirtualNode), message) } From e29cbb65449c54470f892e489b04258d33524cb0 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 15:33:23 -0600 Subject: [PATCH 08/41] Compare outerHTML instead of deep-comparing DOM trees --- test/unit/patch.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index ef11614..fa3cf39 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -175,7 +175,7 @@ function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { const element = render(oldVirtualNode) patch(oldVirtualNode, newVirtualNode) const message = seed != null ? `Invalid patch for seed ${seed}` : undefined - assert.deepEqual(element, render(newVirtualNode), message) + assert.equal(element.outerHTML, render(newVirtualNode).outerHTML, message) } function spans (...elements) { From a8c62d85e8fe5bc822fe5710f9bf1f6c936da39a Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 16:02:17 -0600 Subject: [PATCH 09/41] Handle patching with a different root node type --- src/patch.js | 33 +++++++++++++++++++++------------ test/unit/patch.test.js | 10 ++++++++++ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/patch.js b/src/patch.js index cb778b3..263541a 100644 --- a/src/patch.js +++ b/src/patch.js @@ -2,15 +2,24 @@ import render from './render' export default function patch (oldVirtualNode, newVirtualNode) { const oldElement = oldVirtualNode.element - - if (newVirtualNode.text) { - oldElement.nodeValue = newVirtualNode.text + if (virtualNodesAreEqual(oldVirtualNode, newVirtualNode)) { + if (newVirtualNode.text) { + oldElement.nodeValue = newVirtualNode.text + } else { + patchChildren(oldElement, oldVirtualNode.children, newVirtualNode.children) + patchAttributes(oldElement, oldVirtualNode.props, newVirtualNode.props) + } + newVirtualNode.element = oldElement + return oldElement } else { - patchChildren(oldElement, oldVirtualNode.children, newVirtualNode.children) - patchAttributes(oldElement, oldVirtualNode.props, newVirtualNode.props) + const parentNode = oldElement.parentNode + const nextSibling = oldElement.nextSibling + if (parentNode) oldElement.remove() + const newElement = render(newVirtualNode) + if (parentNode) parentNode.insertBefore(newElement, nextSibling) + newVirtualNode.element = newElement + return newElement } - - newVirtualNode.element = oldElement } function patchAttributes(element, oldProps, newProps) { @@ -41,20 +50,20 @@ function patchChildren (parentElement, oldChildren, newChildren) { oldStartChild = oldChildren[++oldStartIndex] } else if (!oldEndChild) { oldEndChild = oldChildren[--oldEndIndex] - } else if (isEqual(oldStartChild, newStartChild)) { + } else if (virtualNodesAreEqual(oldStartChild, newStartChild)) { patch(oldStartChild, newStartChild) oldStartChild = oldChildren[++oldStartIndex] newStartChild = newChildren[++newStartIndex] - } else if (isEqual(oldEndChild, newEndChild)) { + } else if (virtualNodesAreEqual(oldEndChild, newEndChild)) { patch(oldEndChild, newEndChild) oldEndChild = oldChildren[--oldEndIndex] newEndChild = newChildren[--newEndIndex] - } else if (isEqual(oldStartChild, newEndChild)) { + } else if (virtualNodesAreEqual(oldStartChild, newEndChild)) { patch(oldStartChild, newEndChild) parentElement.insertBefore(oldStartChild.element, oldEndChild.element.nextSibling) oldStartChild = oldChildren[++oldStartIndex] newEndChild = newChildren[--newEndIndex] - } else if (isEqual(oldEndChild, newStartChild)) { + } else if (virtualNodesAreEqual(oldEndChild, newStartChild)) { patch(oldEndChild, newStartChild) parentElement.insertBefore(oldEndChild.element, oldStartChild.element); oldEndChild = oldChildren[--oldEndIndex] @@ -89,7 +98,7 @@ function patchChildren (parentElement, oldChildren, newChildren) { } } -function isEqual (oldVirtualNode, newVirtualNode) { +function virtualNodesAreEqual (oldVirtualNode, newVirtualNode) { return ( getKey(oldVirtualNode) === getKey(newVirtualNode) && oldVirtualNode.tag === newVirtualNode.tag diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index fa3cf39..6630bfd 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -169,6 +169,16 @@ describe('patch', () => { } }) }) + + it('can replace a node with a node of a different type', function () { + const parent = render(
) + const oldVirtualNode =
Hello
+ const element = render(oldVirtualNode) + parent.appendChild(element) + const newElement = patch(oldVirtualNode, Goodbye) + assert.equal(newElement.outerHTML, 'Goodbye') + assert.deepEqual(Array.from(parent.children), [newElement]) + }) }) function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { From 1a0338fe028be71ab59fcd1ff9f49160d9c550bc Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 16:18:40 -0600 Subject: [PATCH 10/41] Rename element to domNode in multiple places to be more precise In all these places, the node can be a text node, which isn't technically an element. --- src/patch.js | 24 ++++++++++++------------ src/render.js | 20 ++++++++++---------- test/unit/patch.test.js | 22 +++++++++++----------- test/unit/render.test.js | 14 +++++++------- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/patch.js b/src/patch.js index 263541a..b03eae4 100644 --- a/src/patch.js +++ b/src/patch.js @@ -1,7 +1,7 @@ import render from './render' export default function patch (oldVirtualNode, newVirtualNode) { - const oldElement = oldVirtualNode.element + const oldElement = oldVirtualNode.domNode if (virtualNodesAreEqual(oldVirtualNode, newVirtualNode)) { if (newVirtualNode.text) { oldElement.nodeValue = newVirtualNode.text @@ -9,7 +9,7 @@ export default function patch (oldVirtualNode, newVirtualNode) { patchChildren(oldElement, oldVirtualNode.children, newVirtualNode.children) patchAttributes(oldElement, oldVirtualNode.props, newVirtualNode.props) } - newVirtualNode.element = oldElement + newVirtualNode.domNode = oldElement return oldElement } else { const parentNode = oldElement.parentNode @@ -17,18 +17,18 @@ export default function patch (oldVirtualNode, newVirtualNode) { if (parentNode) oldElement.remove() const newElement = render(newVirtualNode) if (parentNode) parentNode.insertBefore(newElement, nextSibling) - newVirtualNode.element = newElement + newVirtualNode.domNode = newElement return newElement } } -function patchAttributes(element, oldProps, newProps) { +function patchAttributes(domNode, oldProps, newProps) { for (let prop in oldProps) { - if (!(prop in newProps)) element.removeAttribute(prop) + if (!(prop in newProps)) domNode.removeAttribute(prop) } for (let prop in newProps) { const newValue = newProps[prop] - if (newValue !== oldProps[prop]) element.setAttribute(prop, newValue) + if (newValue !== oldProps[prop]) domNode.setAttribute(prop, newValue) } } @@ -60,12 +60,12 @@ function patchChildren (parentElement, oldChildren, newChildren) { newEndChild = newChildren[--newEndIndex] } else if (virtualNodesAreEqual(oldStartChild, newEndChild)) { patch(oldStartChild, newEndChild) - parentElement.insertBefore(oldStartChild.element, oldEndChild.element.nextSibling) + parentElement.insertBefore(oldStartChild.domNode, oldEndChild.domNode.nextSibling) oldStartChild = oldChildren[++oldStartIndex] newEndChild = newChildren[--newEndIndex] } else if (virtualNodesAreEqual(oldEndChild, newStartChild)) { patch(oldEndChild, newStartChild) - parentElement.insertBefore(oldEndChild.element, oldStartChild.element); + parentElement.insertBefore(oldEndChild.domNode, oldStartChild.domNode); oldEndChild = oldChildren[--oldEndIndex] newStartChild = newChildren[++newStartIndex] } else { @@ -73,27 +73,27 @@ function patchChildren (parentElement, oldChildren, newChildren) { const key = getKey(newStartChild) const oldIndex = key ? oldIndicesByKey[key] : null if (oldIndex == null) { - parentElement.insertBefore(render(newStartChild), oldStartChild.element) + parentElement.insertBefore(render(newStartChild), oldStartChild.domNode) newStartChild = newChildren[++newStartIndex] } else { const oldChildToMove = oldChildren[oldIndex] patch(oldChildToMove, newStartChild) oldChildren[oldIndex] = undefined - parentElement.insertBefore(oldChildToMove.element, oldStartChild.element) + parentElement.insertBefore(oldChildToMove.domNode, oldStartChild.domNode) newStartChild = newChildren[++newStartIndex] } } } if (oldStartIndex > oldEndIndex) { - const subsequentElement = newChildren[newEndIndex + 1] ? newChildren[newEndIndex + 1].element : null + const subsequentElement = newChildren[newEndIndex + 1] ? newChildren[newEndIndex + 1].domNode : null for (let i = newStartIndex; i <= newEndIndex; i++) { parentElement.insertBefore(render(newChildren[i]), subsequentElement) } } else if (newStartIndex > newEndIndex) { for (let i = oldStartIndex; i <= oldEndIndex; i++) { const child = oldChildren[i] - if (child) child.element.remove() + if (child) child.domNode.remove() } } } diff --git a/src/render.js b/src/render.js index 3347441..50aea55 100644 --- a/src/render.js +++ b/src/render.js @@ -1,26 +1,26 @@ export default function render (virtualNode) { - let element + let domNode if (virtualNode.text) { - element = document.createTextNode(virtualNode.text) + domNode = document.createTextNode(virtualNode.text) } else { const {tag, props, children} = virtualNode if (typeof tag === 'function') { const component = new tag(props, children) - element = component.element + domNode = component.element } else { - element = document.createElement(tag) - if (props) setAttributes(element, props) - if (children) addChildren(element, children) + domNode = document.createElement(tag) + if (props) setAttributes(domNode, props) + if (children) addChildren(domNode, children) } } - virtualNode.element = element - return element + virtualNode.domNode = domNode + return domNode } -function setAttributes (element, props) { +function setAttributes (domNode, props) { for (let name in props) { - element.setAttribute(name, props[name]) + domNode.setAttribute(name, props[name]) } } diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 6630bfd..f535faf 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -7,7 +7,7 @@ import dom from '../../src/dom' import render from '../../src/render' import patch from '../../src/patch' -describe('patch', () => { +describe('patch (oldVirtualNode, newVirtualNode)', () => { describe('attributes', function () { it('can add, remove, and update attributes', function () { assertValidPatch(
,
) @@ -113,7 +113,7 @@ describe('patch', () => { }) describe('unkeyed children', function () { - it('can append elements', function () { + it('can append nodes', function () { assertValidPatch(
Hello
,
HelloWorld
@@ -124,7 +124,7 @@ describe('patch', () => { ) }) - it('can prepend elements', function () { + it('can prepend nodes', function () { assertValidPatch(
World
,
HelloWorld
@@ -142,7 +142,7 @@ describe('patch', () => { ) }) - it('can replace an element child with a text child and vice versa', function () { + it('can replace an node child with a text child and vice versa', function () { assertValidPatch(
HelloWorld
,
GoodnightWorld
@@ -173,19 +173,19 @@ describe('patch', () => { it('can replace a node with a node of a different type', function () { const parent = render(
) const oldVirtualNode =
Hello
- const element = render(oldVirtualNode) - parent.appendChild(element) - const newElement = patch(oldVirtualNode, Goodbye) - assert.equal(newElement.outerHTML, 'Goodbye') - assert.deepEqual(Array.from(parent.children), [newElement]) + const oldNode = render(oldVirtualNode) + parent.appendChild(oldNode) + const newNode = patch(oldVirtualNode, Goodbye) + assert.equal(newNode.outerHTML, 'Goodbye') + assert.deepEqual(Array.from(parent.children), [newNode]) }) }) function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { - const element = render(oldVirtualNode) + const node = render(oldVirtualNode) patch(oldVirtualNode, newVirtualNode) const message = seed != null ? `Invalid patch for seed ${seed}` : undefined - assert.equal(element.outerHTML, render(newVirtualNode).outerHTML, message) + assert.equal(node.outerHTML, render(newVirtualNode).outerHTML, message) } function spans (...elements) { diff --git a/test/unit/render.test.js b/test/unit/render.test.js index 946d4b9..07f9287 100644 --- a/test/unit/render.test.js +++ b/test/unit/render.test.js @@ -5,9 +5,9 @@ import {assert} from 'chai' import dom from '../../src/dom' import render from '../../src/render' -describe('render', () => { - it('constructs DOM elements from virtual DOM trees', function () { - const element = render( +describe('render (virtualNode)', () => { + it('constructs DOM nodes from virtual DOM trees', function () { + const domNode = render(
Hello World @@ -15,7 +15,7 @@ describe('render', () => {
) - assert.equal(element.outerHTML, ` + assert.equal(domNode.outerHTML, `
Hello World @@ -24,7 +24,7 @@ describe('render', () => { `.replace(/\n\s*/g, '')) }) - it('constructs child components and embeds their elements', function () { + it('constructs child components and embeds whatever DOM node is assigned to the `.element` property on the component', function () { class Component { constructor (props, children) { this.element = render( @@ -35,7 +35,7 @@ describe('render', () => { } } - const element = render( + const domNode = render(
@@ -44,7 +44,7 @@ describe('render', () => {
) - assert.equal(element.outerHTML, ` + assert.equal(domNode.outerHTML, `
From 03e0f002d57c545e016b3eb6d006632a3ce1d376 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sun, 9 Oct 2016 17:59:40 -0600 Subject: [PATCH 11/41] Start on components; call destroy when removing --- src/patch.js | 9 ++++++-- src/render.js | 1 + test/unit/patch.test.js | 47 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/patch.js b/src/patch.js index b03eae4..a447764 100644 --- a/src/patch.js +++ b/src/patch.js @@ -14,7 +14,7 @@ export default function patch (oldVirtualNode, newVirtualNode) { } else { const parentNode = oldElement.parentNode const nextSibling = oldElement.nextSibling - if (parentNode) oldElement.remove() + removeVirtualNode(oldVirtualNode) const newElement = render(newVirtualNode) if (parentNode) parentNode.insertBefore(newElement, nextSibling) newVirtualNode.domNode = newElement @@ -93,11 +93,16 @@ function patchChildren (parentElement, oldChildren, newChildren) { } else if (newStartIndex > newEndIndex) { for (let i = oldStartIndex; i <= oldEndIndex; i++) { const child = oldChildren[i] - if (child) child.domNode.remove() + if (child) removeVirtualNode(child) } } } +function removeVirtualNode (virtualNode) { + if (virtualNode.component) virtualNode.component.destroy() + virtualNode.domNode.remove() +} + function virtualNodesAreEqual (oldVirtualNode, newVirtualNode) { return ( getKey(oldVirtualNode) === getKey(newVirtualNode) diff --git a/src/render.js b/src/render.js index 50aea55..fd61959 100644 --- a/src/render.js +++ b/src/render.js @@ -7,6 +7,7 @@ export default function render (virtualNode) { if (typeof tag === 'function') { const component = new tag(props, children) + virtualNode.component = component domNode = component.element } else { domNode = document.createElement(tag) diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index f535faf..6f3cc12 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -179,6 +179,53 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { assert.equal(newNode.outerHTML, 'Goodbye') assert.deepEqual(Array.from(parent.children), [newNode]) }) + + describe('child components', function () { + it('can insert a component', function () { + class Component { + constructor (props, children) { + this.element = render(
{children}
) + } + } + + assertValidPatch( +
, +
+ +
+ + +
+ ) + }) + + it('can remove a component and call destroy', function () { + const createdInstances = [] + const destroyedInstances = [] + + class Component { + constructor () { + this.element = render(
) + createdInstances.push(this) + } + + destroy () { + destroyedInstances.push(this) + } + } + + assertValidPatch( +
, +
+ ) + + assert.equal(destroyedInstances.length, 1) + assert.deepEqual(destroyedInstances, [createdInstances[0]]) + }) + + it('can update an existing component', function () { + }) + }) }) function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { From b64510bf95fdc5b71776b7f7da5604e7ab81d0f3 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 07:29:44 -0600 Subject: [PATCH 12/41] :art: --- src/patch.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/patch.js b/src/patch.js index a447764..c2c6baf 100644 --- a/src/patch.js +++ b/src/patch.js @@ -1,24 +1,24 @@ import render from './render' export default function patch (oldVirtualNode, newVirtualNode) { - const oldElement = oldVirtualNode.domNode + const oldNode = oldVirtualNode.domNode if (virtualNodesAreEqual(oldVirtualNode, newVirtualNode)) { if (newVirtualNode.text) { - oldElement.nodeValue = newVirtualNode.text + oldNode.nodeValue = newVirtualNode.text } else { - patchChildren(oldElement, oldVirtualNode.children, newVirtualNode.children) - patchAttributes(oldElement, oldVirtualNode.props, newVirtualNode.props) + patchChildren(oldNode, oldVirtualNode.children, newVirtualNode.children) + patchAttributes(oldNode, oldVirtualNode.props, newVirtualNode.props) } - newVirtualNode.domNode = oldElement - return oldElement + newVirtualNode.domNode = oldNode + return oldNode } else { - const parentNode = oldElement.parentNode - const nextSibling = oldElement.nextSibling + const parentNode = oldNode.parentNode + const nextSibling = oldNode.nextSibling removeVirtualNode(oldVirtualNode) - const newElement = render(newVirtualNode) - if (parentNode) parentNode.insertBefore(newElement, nextSibling) - newVirtualNode.domNode = newElement - return newElement + const newNode = render(newVirtualNode) + if (parentNode) parentNode.insertBefore(newNode, nextSibling) + newVirtualNode.domNode = newNode + return newNode } } From 595224b561d69f97e6ea85f036a2eb7e84b676ca Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 10 Oct 2016 07:42:17 -0600 Subject: [PATCH 13/41] :art: --- test/unit/patch.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 6f3cc12..e03fae2 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -142,7 +142,7 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { ) }) - it('can replace an node child with a text child and vice versa', function () { + it('can replace a child with a text child and vice versa', function () { assertValidPatch(
HelloWorld
,
GoodnightWorld
From c1292cd795a6ca1713e1f374cfa6a7d48fc9dcf1 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 15 Oct 2016 11:40:20 -0600 Subject: [PATCH 14/41] Implement refs to DOM nodes --- src/patch.js | 69 ++++++++++++++++++++++++++++------------- src/render.js | 23 ++++++++------ test/unit/patch.test.js | 47 ++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 30 deletions(-) diff --git a/src/patch.js b/src/patch.js index c2c6baf..07a79dd 100644 --- a/src/patch.js +++ b/src/patch.js @@ -1,38 +1,55 @@ import render from './render' -export default function patch (oldVirtualNode, newVirtualNode) { +export default function patch (oldVirtualNode, newVirtualNode, options) { const oldNode = oldVirtualNode.domNode if (virtualNodesAreEqual(oldVirtualNode, newVirtualNode)) { if (newVirtualNode.text) { oldNode.nodeValue = newVirtualNode.text } else { - patchChildren(oldNode, oldVirtualNode.children, newVirtualNode.children) - patchAttributes(oldNode, oldVirtualNode.props, newVirtualNode.props) + updateChildren(oldNode, oldVirtualNode.children, newVirtualNode.children, options) + updateProps(oldNode, oldVirtualNode.props, newVirtualNode.props, options && options.refs) } newVirtualNode.domNode = oldNode return oldNode } else { const parentNode = oldNode.parentNode const nextSibling = oldNode.nextSibling - removeVirtualNode(oldVirtualNode) - const newNode = render(newVirtualNode) + removeVirtualNode(oldVirtualNode, options && options.refs) + const newNode = render(newVirtualNode, options) if (parentNode) parentNode.insertBefore(newNode, nextSibling) newVirtualNode.domNode = newNode return newNode } } -function patchAttributes(domNode, oldProps, newProps) { - for (let prop in oldProps) { - if (!(prop in newProps)) domNode.removeAttribute(prop) +function updateProps(domNode, oldProps, newProps, refs) { + for (const name in oldProps) { + if (!(name in newProps)) { + if (name === 'ref') { + const oldValue = oldProps[name] + if (refs && refs[oldValue] === domNode) delete refs[oldValue] + } else { + domNode.removeAttribute(name) + } + } } - for (let prop in newProps) { - const newValue = newProps[prop] - if (newValue !== oldProps[prop]) domNode.setAttribute(prop, newValue) + for (let name in newProps) { + const oldValue = oldProps[name] + const newValue = newProps[name] + if (newValue !== oldValue) { + if (name === 'ref') { + if (refs) { + if (refs[oldValue] === domNode) delete refs[oldValue] + refs[newValue] = domNode + } + } else { + domNode.setAttribute(name, newValue) + } + } } } -function patchChildren (parentElement, oldChildren, newChildren) { +function updateChildren (parentElement, oldChildren, newChildren, options) { let oldStartIndex = 0 let oldEndIndex = oldChildren.length - 1 let oldStartChild = oldChildren[0] @@ -51,20 +68,20 @@ function patchChildren (parentElement, oldChildren, newChildren) { } else if (!oldEndChild) { oldEndChild = oldChildren[--oldEndIndex] } else if (virtualNodesAreEqual(oldStartChild, newStartChild)) { - patch(oldStartChild, newStartChild) + patch(oldStartChild, newStartChild, options) oldStartChild = oldChildren[++oldStartIndex] newStartChild = newChildren[++newStartIndex] } else if (virtualNodesAreEqual(oldEndChild, newEndChild)) { - patch(oldEndChild, newEndChild) + patch(oldEndChild, newEndChild, options) oldEndChild = oldChildren[--oldEndIndex] newEndChild = newChildren[--newEndIndex] } else if (virtualNodesAreEqual(oldStartChild, newEndChild)) { - patch(oldStartChild, newEndChild) + patch(oldStartChild, newEndChild, options) parentElement.insertBefore(oldStartChild.domNode, oldEndChild.domNode.nextSibling) oldStartChild = oldChildren[++oldStartIndex] newEndChild = newChildren[--newEndIndex] } else if (virtualNodesAreEqual(oldEndChild, newStartChild)) { - patch(oldEndChild, newStartChild) + patch(oldEndChild, newStartChild, options) parentElement.insertBefore(oldEndChild.domNode, oldStartChild.domNode); oldEndChild = oldChildren[--oldEndIndex] newStartChild = newChildren[++newStartIndex] @@ -73,11 +90,11 @@ function patchChildren (parentElement, oldChildren, newChildren) { const key = getKey(newStartChild) const oldIndex = key ? oldIndicesByKey[key] : null if (oldIndex == null) { - parentElement.insertBefore(render(newStartChild), oldStartChild.domNode) + parentElement.insertBefore(render(newStartChild, options), oldStartChild.domNode) newStartChild = newChildren[++newStartIndex] } else { const oldChildToMove = oldChildren[oldIndex] - patch(oldChildToMove, newStartChild) + patch(oldChildToMove, newStartChild, options) oldChildren[oldIndex] = undefined parentElement.insertBefore(oldChildToMove.domNode, oldStartChild.domNode) newStartChild = newChildren[++newStartIndex] @@ -88,21 +105,31 @@ function patchChildren (parentElement, oldChildren, newChildren) { if (oldStartIndex > oldEndIndex) { const subsequentElement = newChildren[newEndIndex + 1] ? newChildren[newEndIndex + 1].domNode : null for (let i = newStartIndex; i <= newEndIndex; i++) { - parentElement.insertBefore(render(newChildren[i]), subsequentElement) + parentElement.insertBefore(render(newChildren[i], options), subsequentElement) } } else if (newStartIndex > newEndIndex) { for (let i = oldStartIndex; i <= oldEndIndex; i++) { const child = oldChildren[i] - if (child) removeVirtualNode(child) + if (child) removeVirtualNode(child, options && options.refs) } } } -function removeVirtualNode (virtualNode) { +function removeVirtualNode (virtualNode, refs) { + if (refs) removeRefs(virtualNode, refs) if (virtualNode.component) virtualNode.component.destroy() virtualNode.domNode.remove() } +function removeRefs (virtualNode, refs) { + const {domNode, props, children} = virtualNode + const refName = props.ref + if (refName && refs[refName] === domNode) delete refs[refName] + for (const child of children) { + removeRefs(child, refs) + } +} + function virtualNodesAreEqual (oldVirtualNode, newVirtualNode) { return ( getKey(oldVirtualNode) === getKey(newVirtualNode) diff --git a/src/render.js b/src/render.js index fd61959..27f4851 100644 --- a/src/render.js +++ b/src/render.js @@ -1,4 +1,4 @@ -export default function render (virtualNode) { +export default function render (virtualNode, options) { let domNode if (virtualNode.text) { domNode = document.createTextNode(virtualNode.text) @@ -11,22 +11,27 @@ export default function render (virtualNode) { domNode = component.element } else { domNode = document.createElement(tag) - if (props) setAttributes(domNode, props) - if (children) addChildren(domNode, children) + if (children) addChildren(domNode, children, options) + if (props) addProps(domNode, props, options && options.refs) } } virtualNode.domNode = domNode return domNode } -function setAttributes (domNode, props) { - for (let name in props) { - domNode.setAttribute(name, props[name]) +function addChildren (parent, children, options) { + for (let child of children) { + parent.appendChild(render(child, options)) } } -function addChildren (parent, children) { - for (let child of children) { - parent.appendChild(render(child)) +function addProps (domNode, props, refs) { + for (const name in props) { + const value = props[name] + if (name === 'ref') { + if (refs) refs[value] = domNode + } else { + domNode.setAttribute(name, value) + } } } diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index e03fae2..e6c8d3a 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -180,6 +180,53 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { assert.deepEqual(Array.from(parent.children), [newNode]) }) + describe('ref properties', function () { + it('maintains references to child elements based on their `ref` property', function () { + const refs = {} + + const virtualNode1 = ( +
+
+
+
+
+
+
+ ) + let element1 = render(virtualNode1, {refs}) + + assert.equal(refs.a, element1) + assert.equal(refs.b, element1.querySelector('.b')) + assert.equal(refs.c, element1.querySelector('.c')) + assert.equal(refs.d, element1.querySelector('.d')) + assert.equal(refs.e, element1.querySelector('.e')) + + const virtualNode2 = ( +
+
+ +

+

+ ) + patch(virtualNode1, virtualNode2, {refs}) + + assert(!refs.hasOwnProperty('a')) + assert(!refs.hasOwnProperty('b')) + assert(!refs.hasOwnProperty('c')) + assert(!refs.hasOwnProperty('d')) + assert.equal(refs.e, element1.querySelector('.b')) + assert.equal(refs.f, element1.querySelector('.e')) + assert.equal(refs.g, element1.querySelector('.g')) + + const virtualNode3 = + const element2 = patch(virtualNode2, virtualNode3, {refs}) + assert(!refs.hasOwnProperty('e')) + assert(!refs.hasOwnProperty('f')) + assert(!refs.hasOwnProperty('g')) + assert.equal(refs.h, element2) + }) + }) + describe('child components', function () { it('can insert a component', function () { class Component { From 42aa16d636a616fc843fb234a666da91016e2121 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 15 Oct 2016 12:32:00 -0600 Subject: [PATCH 15/41] Call update and destroy on child components --- src/patch.js | 27 ++++++++---- src/render.js | 3 ++ test/unit/patch.test.js | 92 +++++++++++++++++++++++++++-------------- 3 files changed, 83 insertions(+), 39 deletions(-) diff --git a/src/patch.js b/src/patch.js index 07a79dd..5c70dff 100644 --- a/src/patch.js +++ b/src/patch.js @@ -6,8 +6,13 @@ export default function patch (oldVirtualNode, newVirtualNode, options) { if (newVirtualNode.text) { oldNode.nodeValue = newVirtualNode.text } else { - updateChildren(oldNode, oldVirtualNode.children, newVirtualNode.children, options) - updateProps(oldNode, oldVirtualNode.props, newVirtualNode.props, options && options.refs) + if (typeof newVirtualNode.tag === 'function') { + newVirtualNode.component = oldVirtualNode.component + newVirtualNode.component.update(newVirtualNode.props, newVirtualNode.children) + } else { + updateChildren(oldNode, oldVirtualNode.children, newVirtualNode.children, options) + updateProps(oldNode, oldVirtualNode.props, newVirtualNode.props, options && options.refs) + } } newVirtualNode.domNode = oldNode return oldNode @@ -115,16 +120,20 @@ function updateChildren (parentElement, oldChildren, newChildren, options) { } } -function removeVirtualNode (virtualNode, refs) { - if (refs) removeRefs(virtualNode, refs) - if (virtualNode.component) virtualNode.component.destroy() - virtualNode.domNode.remove() +function removeVirtualNode (virtualNode, refs, removeDOMNode = true) { + const {domNode, props, children, component} = virtualNode + if (component) { + component.destroy() + } else if (children) { + for (const child of children) { + removeVirtualNode(child, refs, false) + } + } + if (refs && props && props.ref && refs[props.ref] === domNode) delete refs[props.ref] + if (removeDOMNode) domNode.remove() } function removeRefs (virtualNode, refs) { - const {domNode, props, children} = virtualNode - const refName = props.ref - if (refName && refs[refName] === domNode) delete refs[refName] for (const child of children) { removeRefs(child, refs) } diff --git a/src/render.js b/src/render.js index 27f4851..5a873d2 100644 --- a/src/render.js +++ b/src/render.js @@ -9,6 +9,9 @@ export default function render (virtualNode, options) { const component = new tag(props, children) virtualNode.component = component domNode = component.element + if (options && options.refs && props.ref) { + options.refs[props.ref] = component + } } else { domNode = document.createElement(tag) if (children) addChildren(domNode, children, options) diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index e6c8d3a..64e9c94 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -228,49 +228,81 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { }) describe('child components', function () { - it('can insert a component', function () { + it('can insert, update, and remove components', function () { class Component { constructor (props, children) { - this.element = render(
{children}
) + this.props = props + this.children = children + this.updateCount = 0 + this.destroyCount = 0 + this.virtualNode = this.render() + this.element = render(this.virtualNode) + } + + update (props, children) { + this.props = props + this.children = children + this.updateCount++ + const oldVirtualNode = this.virtualNode + this.virtualNode = this.render() + patch(oldVirtualNode, this.virtualNode) + } + + destroy () { + this.destroyCount++ + } + + render () { + return
{this.children}
} } - assertValidPatch( -
, + const refs = {} + const virtualNode1 =
+ const element = render(virtualNode1, {refs}) + const virtualNode2 = (
- +
) - }) - - it('can remove a component and call destroy', function () { - const createdInstances = [] - const destroyedInstances = [] - - class Component { - constructor () { - this.element = render(
) - createdInstances.push(this) - } - - destroy () { - destroyedInstances.push(this) - } - } + patch(virtualNode1, virtualNode2, {refs}) + const component = refs.component + assert.equal(element.firstChild, component.element) + assert.equal(element.outerHTML, render( +
+
+
+ +
+
+ ).outerHTML) - assertValidPatch( -
, -
+ const virtualNode3 = ( +
+ +

+ +

) - - assert.equal(destroyedInstances.length, 1) - assert.deepEqual(destroyedInstances, [createdInstances[0]]) - }) - - it('can update an existing component', function () { + patch(virtualNode2, virtualNode3, {refs}) + assert.equal(component.updateCount, 1) + assert.equal(element.outerHTML, render( +
+
+

+

+
+ ).outerHTML) + + global.debug = true + const virtualNode4 =
+ patch(virtualNode3, virtualNode4, {refs}) + assert.equal(component.updateCount, 1) + assert.equal(component.destroyCount, 1) + assert.equal(element.outerHTML, render(
).outerHTML) }) }) }) From ca98d73b4f22b731ccd61843acf727fe5f040815 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 15 Oct 2016 12:42:13 -0600 Subject: [PATCH 16/41] Handle situations where virtual node props are null --- src/patch.js | 8 ++++---- test/unit/patch.test.js | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/patch.js b/src/patch.js index 5c70dff..18f1dfa 100644 --- a/src/patch.js +++ b/src/patch.js @@ -29,7 +29,7 @@ export default function patch (oldVirtualNode, newVirtualNode, options) { function updateProps(domNode, oldProps, newProps, refs) { for (const name in oldProps) { - if (!(name in newProps)) { + if (!(newProps && name in newProps)) { if (name === 'ref') { const oldValue = oldProps[name] if (refs && refs[oldValue] === domNode) delete refs[oldValue] @@ -39,12 +39,12 @@ function updateProps(domNode, oldProps, newProps, refs) { } } for (let name in newProps) { - const oldValue = oldProps[name] + const oldValue = oldProps && oldProps[name] const newValue = newProps[name] if (newValue !== oldValue) { if (name === 'ref') { if (refs) { - if (refs[oldValue] === domNode) delete refs[oldValue] + if (oldValue && refs[oldValue] === domNode) delete refs[oldValue] refs[newValue] = domNode } } else { @@ -147,7 +147,7 @@ function virtualNodesAreEqual (oldVirtualNode, newVirtualNode) { } function getKey (virtualNode) { - return virtualNode.props ? virtualNode.props.key : null + return virtualNode.props ? virtualNode.props.key : undefined } function mapOldKeysToIndices (children, startIndex, endIndex) { diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 64e9c94..311d490 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -12,6 +12,11 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { it('can add, remove, and update attributes', function () { assertValidPatch(
,
) }) + + it('can update from no attributes to some attributes and vice versa', function () { + assertValidPatch(
,
) + assertValidPatch(
,
) + }) }) describe('keyed children', function () { From 75b6e0b4aa319cb0ff4b7bc6c05aecc26f03c527 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 15 Oct 2016 18:03:44 -0600 Subject: [PATCH 17/41] Bind event handlers via `on` prop --- src/patch.js | 30 ++----------------------- src/render.js | 15 +++---------- src/update-props.js | 50 +++++++++++++++++++++++++++++++++++++++++ test/unit/patch.test.js | 39 ++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 40 deletions(-) create mode 100644 src/update-props.js diff --git a/src/patch.js b/src/patch.js index 18f1dfa..55a024b 100644 --- a/src/patch.js +++ b/src/patch.js @@ -1,4 +1,5 @@ import render from './render' +import updateProps from './update-props' export default function patch (oldVirtualNode, newVirtualNode, options) { const oldNode = oldVirtualNode.domNode @@ -11,7 +12,7 @@ export default function patch (oldVirtualNode, newVirtualNode, options) { newVirtualNode.component.update(newVirtualNode.props, newVirtualNode.children) } else { updateChildren(oldNode, oldVirtualNode.children, newVirtualNode.children, options) - updateProps(oldNode, oldVirtualNode.props, newVirtualNode.props, options && options.refs) + updateProps(oldNode, oldVirtualNode.props, newVirtualNode.props, options) } } newVirtualNode.domNode = oldNode @@ -27,33 +28,6 @@ export default function patch (oldVirtualNode, newVirtualNode, options) { } } -function updateProps(domNode, oldProps, newProps, refs) { - for (const name in oldProps) { - if (!(newProps && name in newProps)) { - if (name === 'ref') { - const oldValue = oldProps[name] - if (refs && refs[oldValue] === domNode) delete refs[oldValue] - } else { - domNode.removeAttribute(name) - } - } - } - for (let name in newProps) { - const oldValue = oldProps && oldProps[name] - const newValue = newProps[name] - if (newValue !== oldValue) { - if (name === 'ref') { - if (refs) { - if (oldValue && refs[oldValue] === domNode) delete refs[oldValue] - refs[newValue] = domNode - } - } else { - domNode.setAttribute(name, newValue) - } - } - } -} - function updateChildren (parentElement, oldChildren, newChildren, options) { let oldStartIndex = 0 let oldEndIndex = oldChildren.length - 1 diff --git a/src/render.js b/src/render.js index 5a873d2..2d5aa92 100644 --- a/src/render.js +++ b/src/render.js @@ -1,3 +1,5 @@ +import updateProps from './update-props' + export default function render (virtualNode, options) { let domNode if (virtualNode.text) { @@ -15,7 +17,7 @@ export default function render (virtualNode, options) { } else { domNode = document.createElement(tag) if (children) addChildren(domNode, children, options) - if (props) addProps(domNode, props, options && options.refs) + if (props) updateProps(domNode, null, props, options) } } virtualNode.domNode = domNode @@ -27,14 +29,3 @@ function addChildren (parent, children, options) { parent.appendChild(render(child, options)) } } - -function addProps (domNode, props, refs) { - for (const name in props) { - const value = props[name] - if (name === 'ref') { - if (refs) refs[value] = domNode - } else { - domNode.setAttribute(name, value) - } - } -} diff --git a/src/update-props.js b/src/update-props.js new file mode 100644 index 0000000..53d06f2 --- /dev/null +++ b/src/update-props.js @@ -0,0 +1,50 @@ +const RESERVED_PROPS = new Set(['on', 'ref']) + +export default function updateProps(domNode, oldProps, newProps, options) { + const refs = options && options.refs + updateAttributes(domNode, oldProps, newProps) + if (refs) updateRef(domNode, oldProps && oldProps.ref, newProps && newProps.ref, refs) + updateEventListeners(domNode, oldProps && oldProps.on, newProps && newProps.on) +} + +function updateAttributes (domNode, oldProps, newProps) { + for (const name in oldProps) { + if (RESERVED_PROPS.has(name)) continue + if (!newProps || !(name in newProps)) { + domNode.removeAttribute(name) + } + } + for (let name in newProps) { + if (RESERVED_PROPS.has(name)) continue + const oldValue = oldProps && oldProps[name] + const newValue = newProps[name] + if (newValue !== oldValue) { + domNode.setAttribute(name, newValue) + } + } +} + +function updateRef (domNode, oldRefName, newRefName, refs) { + if (newRefName !== oldRefName) { + if (oldRefName && refs[oldRefName] === domNode) delete refs[oldRefName] + if (newRefName) refs[newRefName] = domNode + } +} + +function updateEventListeners (domNode, oldListeners, newListeners) { + for (const name in oldListeners) { + if (!(newListeners && name in newListeners)) { + domNode.removeEventListener(name, oldListeners[name]) + } + } + + for (const name in newListeners) { + const oldListener = oldListeners && oldListeners[name] + const newListener = newListeners[name] + + if (newListener !== oldListener) { + if (oldListener) domNode.removeEventListener(name, oldListener) + domNode.addEventListener(name, newListener) + } + } +} diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 311d490..70243ec 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -232,6 +232,45 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { }) }) + describe('event handlers', function () { + it('registers event handlers from the `on` property', function () { + let listenerCalls = [] + function recordEvent (event) { + listenerCalls.push({context: this, event}) + } + + const virtualNode1 =
+ const element = render(virtualNode1) + + element.dispatchEvent(new CustomEvent('a')) + element.dispatchEvent(new CustomEvent('b')) + assert.equal(listenerCalls.length, 2) + assert.equal(listenerCalls[0].context, element) + assert.equal(listenerCalls[0].event.type, 'a') + assert.equal(listenerCalls[1].context, element) + assert.equal(listenerCalls[1].event.type, 'b') + + listenerCalls = [] + const virtualNode2 =
+ patch(virtualNode1, virtualNode2) + + element.dispatchEvent(new CustomEvent('a')) + element.dispatchEvent(new CustomEvent('b')) + element.dispatchEvent(new CustomEvent('c')) + assert.equal(listenerCalls.length, 2) + assert.equal(listenerCalls[0].context, element) + assert.equal(listenerCalls[0].event.type, 'b') + assert.equal(listenerCalls[1].context, element) + assert.equal(listenerCalls[1].event.type, 'c') + }) + }) + describe('child components', function () { it('can insert, update, and remove components', function () { class Component { From 1ec44e2774bd47639562f161f4774d58cb1a7865 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 15 Oct 2016 18:33:02 -0600 Subject: [PATCH 18/41] Add listenerContext option to bind event listeners in a specific context --- src/patch.js | 2 +- src/render.js | 2 +- src/update-props.js | 46 ++++++++++++++++++++++++++++++++++------- test/unit/patch.test.js | 36 +++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/patch.js b/src/patch.js index 55a024b..b9c513a 100644 --- a/src/patch.js +++ b/src/patch.js @@ -12,7 +12,7 @@ export default function patch (oldVirtualNode, newVirtualNode, options) { newVirtualNode.component.update(newVirtualNode.props, newVirtualNode.children) } else { updateChildren(oldNode, oldVirtualNode.children, newVirtualNode.children, options) - updateProps(oldNode, oldVirtualNode.props, newVirtualNode.props, options) + updateProps(oldNode, oldVirtualNode, newVirtualNode, options) } } newVirtualNode.domNode = oldNode diff --git a/src/render.js b/src/render.js index 2d5aa92..f184eb4 100644 --- a/src/render.js +++ b/src/render.js @@ -17,7 +17,7 @@ export default function render (virtualNode, options) { } else { domNode = document.createElement(tag) if (children) addChildren(domNode, children, options) - if (props) updateProps(domNode, null, props, options) + if (props) updateProps(domNode, null, virtualNode, options) } } virtualNode.domNode = domNode diff --git a/src/update-props.js b/src/update-props.js index 53d06f2..39232de 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -1,10 +1,17 @@ const RESERVED_PROPS = new Set(['on', 'ref']) -export default function updateProps(domNode, oldProps, newProps, options) { - const refs = options && options.refs +export default function updateProps(domNode, oldVirtualNode, newVirtualNode, options) { + const oldProps = oldVirtualNode && oldVirtualNode.props + const newProps = newVirtualNode.props + + let refs, listenerContext + if (options) { + refs = options.refs + listenerContext = options.listenerContext + } updateAttributes(domNode, oldProps, newProps) if (refs) updateRef(domNode, oldProps && oldProps.ref, newProps && newProps.ref, refs) - updateEventListeners(domNode, oldProps && oldProps.on, newProps && newProps.on) + updateEventListeners(domNode, oldVirtualNode, newVirtualNode, listenerContext) } function updateAttributes (domNode, oldProps, newProps) { @@ -31,10 +38,19 @@ function updateRef (domNode, oldRefName, newRefName, refs) { } } -function updateEventListeners (domNode, oldListeners, newListeners) { +function updateEventListeners (domNode, oldVirtualNode, newVirtualNode, listenerContext) { + const oldListeners = oldVirtualNode && oldVirtualNode.props && oldVirtualNode.props.on + const newListeners = newVirtualNode.props && newVirtualNode.props.on + for (const name in oldListeners) { if (!(newListeners && name in newListeners)) { - domNode.removeEventListener(name, oldListeners[name]) + let listenerToRemove + if (oldVirtualNode && oldVirtualNode.boundListeners && oldVirtualNode.boundListeners[name]) { + listenerToRemove = oldVirtualNode.boundListeners[name] + } else { + listenerToRemove =oldListeners[name] + } + domNode.removeEventListener(name, listenerToRemove) } } @@ -43,8 +59,24 @@ function updateEventListeners (domNode, oldListeners, newListeners) { const newListener = newListeners[name] if (newListener !== oldListener) { - if (oldListener) domNode.removeEventListener(name, oldListener) - domNode.addEventListener(name, newListener) + if (oldListener) { + let listenerToRemove + if (oldVirtualNode && oldVirtualNode.boundListeners && oldVirtualNode.boundListeners[name]) { + listenerToRemove = oldVirtualNode.boundListeners[name] + } else { + listenerToRemove = oldListener + } + domNode.removeEventListener(name, listenerToRemove) + } + let listenerToAdd + if (listenerContext) { + listenerToAdd = newListener.bind(listenerContext) + if (!newVirtualNode.boundListeners) newVirtualNode.boundListeners = {} + newVirtualNode.boundListeners[name] = listenerToAdd + } else { + listenerToAdd = newListener + } + domNode.addEventListener(name, listenerToAdd) } } } diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 70243ec..c8ea4e7 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -253,13 +253,13 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { assert.equal(listenerCalls[1].context, element) assert.equal(listenerCalls[1].event.type, 'b') - listenerCalls = [] const virtualNode2 =
patch(virtualNode1, virtualNode2) + listenerCalls = [] element.dispatchEvent(new CustomEvent('a')) element.dispatchEvent(new CustomEvent('b')) element.dispatchEvent(new CustomEvent('c')) @@ -268,6 +268,40 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { assert.equal(listenerCalls[0].event.type, 'b') assert.equal(listenerCalls[1].context, element) assert.equal(listenerCalls[1].event.type, 'c') + + const virtualNode3 =
+ patch(virtualNode2, virtualNode3) + listenerCalls = [] + element.dispatchEvent(new CustomEvent('a')) + element.dispatchEvent(new CustomEvent('b')) + element.dispatchEvent(new CustomEvent('c')) + assert.equal(listenerCalls.length, 0) + }) + + it('binds event listeners with the specified `listenerContext` value, if provided', function () { + const listenerContext = {} + let listenerCalls = [] + function recordEvent (event) { + listenerCalls.push({context: this, event}) + } + + const virtualNode1 =
+ const element = render(virtualNode1, {listenerContext}) + + element.dispatchEvent(new CustomEvent('a')) + assert.equal(listenerCalls.length, 1) + assert.equal(listenerCalls[0].context, listenerContext) + assert.equal(listenerCalls[0].event.type, 'a') + + const virtualNode2 =
+ patch(virtualNode1, virtualNode2, {listenerContext}) + + listenerCalls = [] + element.dispatchEvent(new CustomEvent('a')) + element.dispatchEvent(new CustomEvent('b')) + assert.equal(listenerCalls.length, 1) + assert.equal(listenerCalls[0].context, listenerContext) + assert.equal(listenerCalls[0].event.type, 'b') }) }) From 716d5326702723320cfe58f2f5d130f750d5aa35 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 15 Oct 2016 19:35:50 -0600 Subject: [PATCH 19/41] :art: --- src/update-props.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/update-props.js b/src/update-props.js index 39232de..e2eddb1 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -42,41 +42,41 @@ function updateEventListeners (domNode, oldVirtualNode, newVirtualNode, listener const oldListeners = oldVirtualNode && oldVirtualNode.props && oldVirtualNode.props.on const newListeners = newVirtualNode.props && newVirtualNode.props.on - for (const name in oldListeners) { - if (!(newListeners && name in newListeners)) { + for (const eventName in oldListeners) { + if (!(newListeners && eventName in newListeners)) { let listenerToRemove - if (oldVirtualNode && oldVirtualNode.boundListeners && oldVirtualNode.boundListeners[name]) { - listenerToRemove = oldVirtualNode.boundListeners[name] + if (oldVirtualNode && oldVirtualNode.boundListeners && oldVirtualNode.boundListeners[eventName]) { + listenerToRemove = oldVirtualNode.boundListeners[eventName] } else { - listenerToRemove =oldListeners[name] + listenerToRemove =oldListeners[eventName] } - domNode.removeEventListener(name, listenerToRemove) + domNode.removeEventListener(eventName, listenerToRemove) } } - for (const name in newListeners) { - const oldListener = oldListeners && oldListeners[name] - const newListener = newListeners[name] + for (const eventName in newListeners) { + const oldListener = oldListeners && oldListeners[eventName] + const newListener = newListeners[eventName] if (newListener !== oldListener) { if (oldListener) { let listenerToRemove - if (oldVirtualNode && oldVirtualNode.boundListeners && oldVirtualNode.boundListeners[name]) { - listenerToRemove = oldVirtualNode.boundListeners[name] + if (oldVirtualNode && oldVirtualNode.boundListeners && oldVirtualNode.boundListeners[eventName]) { + listenerToRemove = oldVirtualNode.boundListeners[eventName] } else { listenerToRemove = oldListener } - domNode.removeEventListener(name, listenerToRemove) + domNode.removeEventListener(eventName, listenerToRemove) } let listenerToAdd if (listenerContext) { listenerToAdd = newListener.bind(listenerContext) if (!newVirtualNode.boundListeners) newVirtualNode.boundListeners = {} - newVirtualNode.boundListeners[name] = listenerToAdd + newVirtualNode.boundListeners[eventName] = listenerToAdd } else { listenerToAdd = newListener } - domNode.addEventListener(name, listenerToAdd) + domNode.addEventListener(eventName, listenerToAdd) } } } From 82c783bbbe2026d042c3e1c50f6c1bdd2f529458 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 15 Oct 2016 19:55:29 -0600 Subject: [PATCH 20/41] Support standard events via camel-cased props Support the same set of events that React supports. --- src/dom.js | 13 ++++++- src/event-listener-props.js | 70 +++++++++++++++++++++++++++++++++++++ src/update-props.js | 8 +++-- test/unit/patch.test.js | 14 ++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 src/event-listener-props.js diff --git a/src/dom.js b/src/dom.js index 92fc8b9..468c7cb 100644 --- a/src/dom.js +++ b/src/dom.js @@ -1,5 +1,6 @@ -export default function dom (tag, props, ...children) { +import EVENT_LISTENER_PROPS from './event-listener-props' +export default function dom (tag, props, ...children) { for (let i = 0; i < children.length;) { const child = children[i] if (Array.isArray(child)) { @@ -12,5 +13,15 @@ export default function dom (tag, props, ...children) { } } + if (props) { + for (const propName in props) { + const eventName = EVENT_LISTENER_PROPS[propName] + if (eventName) { + if (!props.on) props.on = {} + props.on[eventName] = props[propName] + } + } + } + return {tag, props, children} } diff --git a/src/event-listener-props.js b/src/event-listener-props.js new file mode 100644 index 0000000..d96da5c --- /dev/null +++ b/src/event-listener-props.js @@ -0,0 +1,70 @@ +export default { + onCopy: 'copy', + onCut: 'cut', + onPaste: 'paste', + onCompositionEnd: 'compositionend', + onCompositionStart: 'compositionstart', + onCompositionUpdate: 'compositionupdate', + onKeyDown: 'keydown', + onKeyPress: 'keypress', + onKeyUp: 'keyup', + onFocus: 'focus', + onBlur: 'blur', + onChange: 'change', + onInput: 'input', + onSubmit: 'submit', + onClick: 'click', + onContextMenu: 'contextmenu', + onDoubleClick: 'doubleclick', + onDrag: 'drag', + onDragEnd: 'dragend', + onDragEnter: 'dragenter', + onDragExit: 'dragexit', + onDragLeave: 'dragleave', + onDragOver: 'dragover', + onDragStart: 'dragstart', + onDrop: 'drop', + onMouseDown: 'mousedown', + onMouseEnter: 'mousenter', + onMouseLeave: 'mouseleave', + onMouseMove: 'mousemove', + onMouseOut: 'mouseout', + onMouseOver: 'mouseover', + onMouseUp: 'mouseup', + onSelect: 'select', + onTouchCancel: 'touchcancel', + onTouchEnd: 'touchend', + onTouchMove: 'touchmove', + onTouchStart: 'touchstart', + onScroll: 'scroll', + onWheel: 'wheel', + onAbort: 'abort', + onCanPlay: 'canplay', + onCanPlayThrough: 'canplaythrough', + onDurationChange: 'durationchange', + onEmptied: 'emptied', + onEncrypted: 'encrypted', + onEnded: 'ended', + onError: 'error', + onLoadedData: 'loadeddata', + onLoadedMetadata: 'loadedmetadat', + onLoadStart: 'loadstart', + onPause: 'pause', + onPlay: 'play', + onPlaying: 'playing', + onProgress: 'progress', + onRateChange: 'ratechange', + onSeeked: 'seeked', + onSeeking: 'seeking', + onStalled: 'stalled', + onSuspend: 'suspend', + onTimeUpdate: 'timeupdate', + onVolumeChange: 'volumechange', + onWaiting: 'waiting', + onLoad: 'load', + onError: 'error', + onAnimationStart: 'animationstart', + onAnimationEnd: 'animationend', + onAnimationIteration: 'animationiteration', + onTransitionEnd: 'transitionend' +} diff --git a/src/update-props.js b/src/update-props.js index e2eddb1..cfb12ef 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -1,4 +1,4 @@ -const RESERVED_PROPS = new Set(['on', 'ref']) +import EVENT_LISTENER_PROPS from './event-listener-props' export default function updateProps(domNode, oldVirtualNode, newVirtualNode, options) { const oldProps = oldVirtualNode && oldVirtualNode.props @@ -16,13 +16,15 @@ export default function updateProps(domNode, oldVirtualNode, newVirtualNode, opt function updateAttributes (domNode, oldProps, newProps) { for (const name in oldProps) { - if (RESERVED_PROPS.has(name)) continue + if (name === 'ref' || name === 'on') continue + if (name in EVENT_LISTENER_PROPS) continue if (!newProps || !(name in newProps)) { domNode.removeAttribute(name) } } for (let name in newProps) { - if (RESERVED_PROPS.has(name)) continue + if (name === 'ref' || name === 'on') continue + if (name in EVENT_LISTENER_PROPS) continue const oldValue = oldProps && oldProps[name] const newValue = newProps[name] if (newValue !== oldValue) { diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index c8ea4e7..05efc3a 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -303,6 +303,20 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { assert.equal(listenerCalls[0].context, listenerContext) assert.equal(listenerCalls[0].event.type, 'b') }) + + it('allows standard event listeners to be specified as props like onClick or onMouseDown', function () { + let listenerCalls = [] + function recordEvent (event) { + listenerCalls.push(event) + } + const element = render(
) + + element.dispatchEvent(new MouseEvent('click')) + element.dispatchEvent(new MouseEvent('mousedown')) + assert.equal(listenerCalls.length, 2) + assert.equal(listenerCalls[0].type, 'click') + assert.equal(listenerCalls[1].type, 'mousedown') + }) }) describe('child components', function () { From 3f58b1e7dcdf47ca96227dc67901029395eff7b0 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 10 Nov 2016 21:45:25 -0700 Subject: [PATCH 21/41] Convert component helpers to use custom virtual DOM implementation --- TASKS.md | 6 +++ src/component-helpers.js | 55 +++++++++++----------- src/component-widget.js | 99 ---------------------------------------- src/patch.js | 47 +++++++++++++++---- src/ref-hook.js | 29 ------------ src/refs-stack.js | 6 --- src/render.js | 13 ++++-- test/unit/dom.test.js | 3 -- test/unit/patch.test.js | 43 +++++++++++++++++ test/unit/render.test.js | 13 ++++++ 10 files changed, 136 insertions(+), 178 deletions(-) create mode 100644 TASKS.md delete mode 100644 src/component-widget.js delete mode 100644 src/ref-hook.js delete mode 100644 src/refs-stack.js diff --git a/TASKS.md b/TASKS.md new file mode 100644 index 0000000..52c711d --- /dev/null +++ b/TASKS.md @@ -0,0 +1,6 @@ +* Consolidate old tests with new tests... what can be simpler? +* Boolean attributes +* Props that shouldn't be converted to attributes +* Support for objects as the `style` prop +* SVG +* Benchmark diff --git a/src/component-helpers.js b/src/component-helpers.js index d74461e..f12315a 100644 --- a/src/component-helpers.js +++ b/src/component-helpers.js @@ -1,23 +1,20 @@ -import createElement from 'virtual-dom/create-element' -import diff from 'virtual-dom/diff' -import patch from 'virtual-dom/patch' - -import refsStack from './refs-stack' +import render from './render' +import patch from './patch' import {getScheduler} from './scheduler-assignment' const componentsWithPendingUpdates = new WeakSet let syncUpdatesInProgressCounter = 0 let syncDestructionsInProgressCounter = 0 -function isValidVirtualElement (virtualElement) { - return virtualElement != null && virtualElement !== false +function isValidVirtualNode (virtualNode) { + return virtualNode != null && virtualNode !== false } // This function associates a component object with a DOM element by calling // the components `render` method, assigning an `.element` property on the // object and also returning the element. // -// It also assigns a `virtualElement` property based on the return value of the +// It also assigns a `virtualNode` property based on the return value of the // `render` method. This will be used later by `performElementUpdate` to diff // the new results of `render` with the previous results when updating the // component's element. @@ -32,16 +29,17 @@ export function initialize(component) { throw new Error('Etch components must implement `update(props, children)`.') } - let virtualElement = component.render() - if (!isValidVirtualElement(virtualElement)) { + let virtualNode = component.render() + if (!isValidVirtualNode(virtualNode)) { let namePart = component.constructor && component.constructor.name ? ' in ' + component.constructor.name : '' - throw new Error('invalid falsy value ' + virtualElement + ' returned from render()' + namePart) + throw new Error('invalid falsy value ' + virtualNode + ' returned from render()' + namePart) } + component.refs = {} - component.virtualElement = virtualElement - refsStack.push(component.refs) - component.element = createElement(component.virtualElement) - refsStack.pop() + component.virtualNode = virtualNode + component.element = render(component.virtualNode, { + refs: component.refs, listenerContext: component + }) } // This function receives a component that has already been associated with an @@ -79,7 +77,7 @@ export function update (component, replaceNode=true) { } // Synchronsly updates the DOM element associated with a component object. . -// This method assumes the presence of `.element` and `.virtualElement` +// This method assumes the presence of `.element` and `.virtualNode` // properties on the component, which are assigned in the `initialize` // function. // @@ -98,19 +96,20 @@ export function update (component, replaceNode=true) { // between invocations, because we want to preserve a one-to-one relationship // between component objects and DOM elements for simplicity. export function updateSync (component, replaceNode=true) { - let newVirtualElement = component.render() - if (!isValidVirtualElement(newVirtualElement)) { + let newVirtualNode = component.render() + if (!isValidVirtualNode(newVirtualNode)) { let namePart = component.constructor && component.constructor.name ? ' in ' + component.constructor.name : '' - throw new Error('invalid falsy value ' + newVirtualElement + ' returned from render()' + namePart) + throw new Error('invalid falsy value ' + newVirtualNode + ' returned from render()' + namePart) } syncUpdatesInProgressCounter++ - let oldVirtualElement = component.virtualElement + let oldVirtualNode = component.virtualNode let oldDomNode = component.element - refsStack.push(component.refs) - let newDomNode = patch(component.element, diff(oldVirtualElement, newVirtualElement)) - refsStack.pop() - component.virtualElement = newVirtualElement + let newDomNode = patch(oldVirtualNode, newVirtualNode, { + refs: component.refs, + listenerContext: component + }) + component.virtualNode = newVirtualNode if (newDomNode !== oldDomNode && !replaceNode) { throw new Error('The root node type changed on update, but the update was performed with the replaceNode option set to false') } else { @@ -161,16 +160,14 @@ export function destroy (component, removeNode=true) { // the element if we are not a nested call. export function destroySync (component, removeNode=true) { syncDestructionsInProgressCounter++ - destroyChildComponents(component.virtualElement) + destroyChildComponents(component.virtualNode) if (syncDestructionsInProgressCounter === 1 && removeNode) component.element.remove() syncDestructionsInProgressCounter-- } function destroyChildComponents(virtualNode) { - if (virtualNode.type === 'Widget') { - if (virtualNode.component && typeof virtualNode.component.destroy === 'function') { - virtualNode.component.destroy() - } + if (virtualNode.component && typeof virtualNode.component.destroy === 'function') { + virtualNode.component.destroy() } else if (virtualNode.children) { virtualNode.children.forEach(destroyChildComponents) } diff --git a/src/component-widget.js b/src/component-widget.js deleted file mode 100644 index a0857d6..0000000 --- a/src/component-widget.js +++ /dev/null @@ -1,99 +0,0 @@ -import refsStack from './refs-stack' - -// Instances of this class interface with an [extension mechanism](https://github.com/Matt-Esch/virtual-dom/blob/master/docs/widget.md) -// in the `virtual-dom` library that allows us to manually manage specific DOM -// nodes within a `virtual-dom` tree. It is designed to wrap a *component*, -// which is just a plain JavaScript object that implements a simple interface. -export default class ComponentWidget { - constructor (componentConstructor, properties, children) { - this.type = 'Widget' - this.componentConstructor = componentConstructor - this.properties = properties - this.children = children - - if (this.properties && this.properties.key) { - this.key = this.properties.key - } - } - - // The `virtual-dom` library expects this method to return a DOM node. It - // calls this method when creating a DOM tree for the first time or when - // updating a DOM tree via patch. We create an instance of the component based - // on the constructor we saved when the widget was created, deal with the - // `ref` property if it is present, then return the component's associated DOM - // element. - init () { - this.component = new this.componentConstructor(this.properties, this.children) - if (this.properties && this.properties.ref && refsStack.length > 0) { - refsStack[refsStack.length - 1][this.properties.ref] = this.component - } - return this.component.element - } - - // The `virtual-dom` library calls this method when applying updates to the - // DOM by `diff`ing two virtual trees and then `patch`ing an element. - // - // If a widget with the same `componentConstructor` occurs at the same - // location of the tree, we attempt to update the underlying component by - // calling the `update` method with the component's new properties and - // children. If the component doesn't have an `update` method or if the - // widgets have *different* component constructors, we destroy the old - // component and build a new one in its place. - update (oldWidget, oldElement) { - let oldRef, newRef - - if (oldWidget.properties && oldWidget.properties.ref) { - oldRef = oldWidget.properties.ref - } - - if (this.properties && this.properties.ref) { - newRef = this.properties.ref - } - - if (this.componentConstructor === oldWidget.componentConstructor) { - this.component = oldWidget.component - - // If the ref properties have changed, delete the old reference and create - // a new reference to this widget's component. - if (newRef !== oldRef && refsStack.length > 0) { - delete refsStack[refsStack.length - 1][oldRef] - refsStack[refsStack.length - 1][newRef] = this.component - } - - this.component.update(this.properties, this.children) - } else { - // If `ref` properties are defined, delete the reference to the old - // component. Only do this if the reference name changed because otherwise - // the new reference (created in the call to `this.init`) will overwrite - // the old one anyway. - if (refsStack.length > 0) { - if (newRef !== oldRef) { - delete refsStack[refsStack.length - 1][oldRef] - } - } - - // Clean up the old component - oldWidget.destroy() - // Build a new component instance by calling `init`. - this.init() - } - - // Return the element so virtual-dom will replace the element if it changed - return this.component.element - } - - destroy () { - // Clean up the reference to this component if it is not now referencing a - // different component. - if (this.properties && this.properties.ref && refsStack.length > 0) { - const refs = refsStack[refsStack.length - 1] - if (refs[this.properties.ref] === this.component) { - delete refs[this.properties.ref] - } - } - - if (typeof this.component.destroy === 'function') { - this.component.destroy() - } - } -} diff --git a/src/patch.js b/src/patch.js index b9c513a..8a28cea 100644 --- a/src/patch.js +++ b/src/patch.js @@ -4,19 +4,24 @@ import updateProps from './update-props' export default function patch (oldVirtualNode, newVirtualNode, options) { const oldNode = oldVirtualNode.domNode if (virtualNodesAreEqual(oldVirtualNode, newVirtualNode)) { + let newNode if (newVirtualNode.text) { oldNode.nodeValue = newVirtualNode.text + newNode = oldNode } else { if (typeof newVirtualNode.tag === 'function') { - newVirtualNode.component = oldVirtualNode.component - newVirtualNode.component.update(newVirtualNode.props, newVirtualNode.children) + newNode = updateComponent(oldVirtualNode, newVirtualNode, options) } else { updateChildren(oldNode, oldVirtualNode.children, newVirtualNode.children, options) updateProps(oldNode, oldVirtualNode, newVirtualNode, options) + newNode = oldNode } } newVirtualNode.domNode = oldNode - return oldNode + if (newNode !== oldNode && oldNode.parentNode) { + oldNode.parentNode.replaceChild(newNode, oldNode) + } + return newNode } else { const parentNode = oldNode.parentNode const nextSibling = oldNode.nextSibling @@ -28,6 +33,27 @@ export default function patch (oldVirtualNode, newVirtualNode, options) { } } +function updateComponent (oldVirtualNode, newVirtualNode, options) { + const {component, props: oldProps} = oldVirtualNode + let {props: newProps, children: newChildren} = newVirtualNode + newVirtualNode.component = component + if (options && options.refs) { + const refs = options.refs + const oldRefName = oldProps && oldProps.ref + const newRefName = newProps && newProps.ref + if (newRefName !== oldRefName) { + if (oldRefName && refs[oldRefName] === component) delete refs[oldRefName] + if (newRefName) refs[newRefName] = component + } + if (newRefName) { + newProps = {...newProps} + delete newProps.ref + } + } + component.update(newProps || {}, newChildren) + return component.element +} + function updateChildren (parentElement, oldChildren, newChildren, options) { let oldStartIndex = 0 let oldEndIndex = oldChildren.length - 1 @@ -96,14 +122,19 @@ function updateChildren (parentElement, oldChildren, newChildren, options) { function removeVirtualNode (virtualNode, refs, removeDOMNode = true) { const {domNode, props, children, component} = virtualNode + const ref = props && props.ref if (component) { - component.destroy() - } else if (children) { - for (const child of children) { - removeVirtualNode(child, refs, false) + if (refs && ref && refs[ref] === component) delete refs[ref] + if (typeof component.destroy === 'function') component.destroy() + } else { + if (refs && ref && refs[ref] === domNode) delete refs[ref] + if (children) { + for (const child of children) { + removeVirtualNode(child, refs, false) + } } } - if (refs && props && props.ref && refs[props.ref] === domNode) delete refs[props.ref] + if (removeDOMNode) domNode.remove() } diff --git a/src/ref-hook.js b/src/ref-hook.js deleted file mode 100644 index 9ad1049..0000000 --- a/src/ref-hook.js +++ /dev/null @@ -1,29 +0,0 @@ -import refsStack from './refs-stack' - -// Instances of this class interface with an [extension mechanism](https://github.com/Matt-Esch/virtual-dom/blob/master/docs/hooks.md) -// in the `virtual-dom` library called "hooks". When virtual nodes have -// properties that reference hook instances, `virtual-dom` will call `hook` -// and `unhook` on this object with the underlying DOM node when it is created -// and destroyed. -// -// We maintain a global stack of simple `Object` instances in which to store -// references. When creating or updating a component, we push its associated -// `refs` object to the top of the stack. This allows the hooks to assign -// references into the component whose element we are currently creating or -// updating. -export default class RefHook { - constructor (refName) { - this.refName = refName - } - - hook (node) { - refsStack[refsStack.length - 1][this.refName] = node - } - - unhook (node) { - const currentRefs = refsStack[refsStack.length - 1] - if (refsStack.length > 0 && currentRefs[this.refName] === node) { - delete currentRefs[this.refName] - } - } -} diff --git a/src/refs-stack.js b/src/refs-stack.js deleted file mode 100644 index cda59a1..0000000 --- a/src/refs-stack.js +++ /dev/null @@ -1,6 +0,0 @@ -// This stack is shared by `initialize`, `performElementUpdate`, -// `ComponentWidget`, and `RefHook`. The top of the stack will always contain -// the `refs` object of the component that is currently being created or -// updated, enabling widgets and hooks to create references to DOM nodes -// that are associated with `ref` properties. -export default [] diff --git a/src/render.js b/src/render.js index f184eb4..2c95435 100644 --- a/src/render.js +++ b/src/render.js @@ -5,14 +5,19 @@ export default function render (virtualNode, options) { if (virtualNode.text) { domNode = document.createTextNode(virtualNode.text) } else { - const {tag, props, children} = virtualNode + const {tag, children} = virtualNode + let {props} = virtualNode if (typeof tag === 'function') { - const component = new tag(props, children) + let ref + if (props && props.ref) { + ({ref, ...props} = props) + } + const component = new tag(props || {}, children) virtualNode.component = component domNode = component.element - if (options && options.refs && props.ref) { - options.refs[props.ref] = component + if (options && options.refs && ref) { + options.refs[ref] = component } } else { domNode = document.createElement(tag) diff --git a/test/unit/dom.test.js b/test/unit/dom.test.js index 956b81e..468ac46 100644 --- a/test/unit/dom.test.js +++ b/test/unit/dom.test.js @@ -324,7 +324,6 @@ describe('etch.dom', () => { etch.initialize(parentComponent) expect(parentComponent.refs.child instanceof ChildComponentA).to.be.true - expect(parentComponent.refs.child.properties.ref).to.equal('child') expect(parentComponent.refs.child.refs.self.textContent).to.equal('A') parentComponent.refName = 'kid' @@ -332,7 +331,6 @@ describe('etch.dom', () => { expect(parentComponent.refs.child).to.be.undefined expect(parentComponent.refs.kid instanceof ChildComponentA).to.be.true - expect(parentComponent.refs.kid.properties.ref).to.equal('kid') expect(parentComponent.refs.kid.refs.self.textContent).to.equal('A') parentComponent.refName = 'child' @@ -342,7 +340,6 @@ describe('etch.dom', () => { expect(parentComponent.refs.kid).to.be.undefined expect(parentComponent.refs.child instanceof ChildComponentB).to.be.true - expect(parentComponent.refs.child.properties.ref).to.equal('child') expect(parentComponent.refs.child.refs.self.textContent).to.equal('B') parentComponent.renderB = false diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 05efc3a..526be69 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -230,6 +230,49 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { assert(!refs.hasOwnProperty('g')) assert.equal(refs.h, element2) }) + + it('maintains references to child component instances based on their `ref` property', function () { + class ChildComponentA { + constructor (props) { + assert(!props.ref, 'Ref property not passed through to component constructor') + this.element = document.createElement('div') + } + + update (props) { + assert(!props.ref, 'Ref property not passed through to component update') + } + } + + class ChildComponentB { + constructor (props) { + assert(!props.ref, 'Ref property not passed through to component constructor') + this.element = document.createElement('div') + } + + update (props) { + assert(!props.ref, 'Ref property not passed through to component update') + } + } + + const refs = {} + const virtualNode1 =
+ render(virtualNode1, {refs}) + assert(refs.child instanceof ChildComponentA) + + const virtualNode2 =
+ patch(virtualNode1, virtualNode2, {refs}) + assert(!refs.child, 'Old ref was deleted') + assert(refs.kid instanceof ChildComponentA) + + const virtualNode3 =
+ patch(virtualNode2, virtualNode3, {refs}) + assert(!refs.kid, 'Old ref was deleted') + assert(refs.child instanceof ChildComponentB) + + const virtualNode4 =
+ patch(virtualNode3, virtualNode4, {refs}) + assert(refs.child instanceof ChildComponentA) + }) }) describe('event handlers', function () { diff --git a/test/unit/render.test.js b/test/unit/render.test.js index 07f9287..5d755cb 100644 --- a/test/unit/render.test.js +++ b/test/unit/render.test.js @@ -53,4 +53,17 @@ describe('render (virtualNode)', () => {
`.replace(/\n\s*/g, '')) }) + + it('passes an empty props object to child components by default', function () { + class Component { + constructor (props, children) { + this.element = document.createElement('div') + assert.deepEqual(props, {}) + assert.deepEqual(children, []) + } + } + + const domNode = render(
) + assert.equal(domNode.outerHTML, `
`) + }) }) From 54a52e1f5c646a5cffecfdd5b55d5a14b36089f3 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 20 Feb 2017 12:30:02 -0700 Subject: [PATCH 22/41] Replace devtool w/ electron-mocha --- package.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/package.json b/package.json index 5a400e1..48572c8 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "main": "dist/index.js", "scripts": { "test": "npm run standard && npm run mocha", - "mocha": "devtool --quit --console node_modules/.bin/_mocha --colors --recursive ./test/helpers/register-babel test", + "mocha": "electron-mocha --renderer --interactive --recursive ./test/helpers/register-babel test", "tdd": "npm run standard && node_modules/.bin/electron-mocha --renderer --interactive --recursive ./test/helpers/register-babel test", "prepublish": "npm run standard && npm run clean && npm run build", "standard": "node_modules/.bin/standard --recursive src test && echo Linting passed", @@ -29,9 +29,7 @@ "babel": "^5.0.0", "babel-eslint": "^4.0.5", "chai": "^2.0.0", - "devtool": "^1.9.1", "electron-mocha": "git://github.com/nathansobo/electron-mocha.git#interactive-option", - "electron-prebuilt": "^0.30.1", "mocha": "^2.1.0", "random-seed": "^0.3.0", "standard": "^4.5.4" From 3ec802612af783a80edc162fd419730a6eebace0 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 21 Feb 2017 19:55:38 +0100 Subject: [PATCH 23/41] Assign properties to DOM elements instead of attributes Signed-off-by: Nathan Sobo --- src/dom.js | 26 +++++++++++++++++++++++ src/update-props.js | 51 ++++++++++++++++++++++++++++++++------------- 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/dom.js b/src/dom.js index 468c7cb..4c0d268 100644 --- a/src/dom.js +++ b/src/dom.js @@ -21,7 +21,33 @@ export default function dom (tag, props, ...children) { props.on[eventName] = props[propName] } } + + if (props.class) { + props.className = props.class + } } return {tag, props, children} } + +const TAG_NAMES = [ + 'a', 'abbr', 'address', 'article', 'aside', 'audio', 'b', 'bdi', 'bdo', + 'blockquote', 'body', 'button', 'canvas', 'caption', 'cite', 'code', + 'colgroup', 'datalist', 'dd', 'del', 'details', 'dfn', 'dialog', 'div', 'dl', + 'dt', 'em', 'fieldset', 'figcaption', 'figure', 'footer', 'form', 'h1', 'h2', + 'h3', 'h4', 'h5', 'h6', 'head', 'header', 'html', 'i', 'iframe', 'ins', 'kbd', + 'label', 'legend', 'li', 'main', 'map', 'mark', 'menu', 'meter', 'nav', + 'noscript', 'object', 'ol', 'optgroup', 'option', 'output', 'p', 'pre', + 'progress', 'q', 'rp', 'rt', 'ruby', 's', 'samp', 'script', 'section', + 'select', 'small', 'span', 'strong', 'style', 'sub', 'summary', 'sup', 'svg', + 'table', 'tbody', 'td', 'textarea', 'tfoot', 'th', 'thead', 'time', 'title', + 'tr', 'u', 'ul', 'var', 'video', 'area', 'base', 'br', 'col', 'command', + 'embed', 'hr', 'img', 'input', 'keygen', 'link', 'meta', 'param', 'source', + 'track', 'wbr' +] + +for (const tagName of TAG_NAMES) { + dom[tagName] = (props, ...children) => { + return dom(tagName, props, ...children) + } +} diff --git a/src/update-props.js b/src/update-props.js index cfb12ef..f771869 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -1,6 +1,6 @@ import EVENT_LISTENER_PROPS from './event-listener-props' -export default function updateProps(domNode, oldVirtualNode, newVirtualNode, options) { +export default function (domNode, oldVirtualNode, newVirtualNode, options) { const oldProps = oldVirtualNode && oldVirtualNode.props const newProps = newVirtualNode.props @@ -9,26 +9,47 @@ export default function updateProps(domNode, oldVirtualNode, newVirtualNode, opt refs = options.refs listenerContext = options.listenerContext } - updateAttributes(domNode, oldProps, newProps) + updateProps(domNode, oldProps, newProps) if (refs) updateRef(domNode, oldProps && oldProps.ref, newProps && newProps.ref, refs) updateEventListeners(domNode, oldVirtualNode, newVirtualNode, listenerContext) } -function updateAttributes (domNode, oldProps, newProps) { - for (const name in oldProps) { - if (name === 'ref' || name === 'on') continue - if (name in EVENT_LISTENER_PROPS) continue - if (!newProps || !(name in newProps)) { - domNode.removeAttribute(name) +function updateProps (domNode, oldProps, newProps) { + if (oldProps) { + const oldPropsNames = Object.keys(oldProps) + for (let i = 0; i < oldPropsNames.length; i++) { + const name = oldPropsNames[i] + if (name === 'ref' || name === 'on') continue + if (name in EVENT_LISTENER_PROPS) continue + if (!newProps || !(name in newProps)) { + if (name === 'dataset') { + updateProps(domNode.dataset, oldProps ? oldProps.dataset : null, null) + } else if (name === 'style') { + updateProps(domNode.style, oldProps ? oldProps.style : null, null) + } else { + delete domNode[name] + } + } } } - for (let name in newProps) { - if (name === 'ref' || name === 'on') continue - if (name in EVENT_LISTENER_PROPS) continue - const oldValue = oldProps && oldProps[name] - const newValue = newProps[name] - if (newValue !== oldValue) { - domNode.setAttribute(name, newValue) + + if (newProps) { + const newPropsNames = Object.keys(newProps) + for (let i = 0; i < newPropsNames.length; i++) { + const name = newPropsNames[i] + if (name === 'ref' || name === 'on') continue + if (name in EVENT_LISTENER_PROPS) continue + const oldValue = oldProps && oldProps[name] + const newValue = newProps[name] + if (name === 'dataset') { + updateProps(domNode.dataset, oldProps ? oldProps.dataset : null, newProps.dataset) + } else if (name === 'style') { + updateProps(domNode.style, oldProps ? oldProps.style : null, newProps.style) + } else { + if (newValue !== oldValue) { + domNode[name] = newValue + } + } } } } From 97112fc591d8b381e246981dfe078511de8862f0 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 21 Feb 2017 20:08:40 +0100 Subject: [PATCH 24/41] Handle text children that are empty Signed-off-by: Nathan Sobo --- src/patch.js | 2 +- src/render.js | 2 +- test/unit/patch.test.js | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/patch.js b/src/patch.js index 8a28cea..86bf431 100644 --- a/src/patch.js +++ b/src/patch.js @@ -5,7 +5,7 @@ export default function patch (oldVirtualNode, newVirtualNode, options) { const oldNode = oldVirtualNode.domNode if (virtualNodesAreEqual(oldVirtualNode, newVirtualNode)) { let newNode - if (newVirtualNode.text) { + if (newVirtualNode.text != null) { oldNode.nodeValue = newVirtualNode.text newNode = oldNode } else { diff --git a/src/render.js b/src/render.js index 2c95435..2d3ecbb 100644 --- a/src/render.js +++ b/src/render.js @@ -2,7 +2,7 @@ import updateProps from './update-props' export default function render (virtualNode, options) { let domNode - if (virtualNode.text) { + if (virtualNode.text != null) { domNode = document.createTextNode(virtualNode.text) } else { const {tag, children} = virtualNode diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 526be69..5354262 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -147,6 +147,17 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { ) }) + it('can handle text children that are empty', function () { + assertValidPatch( +
Hello
, +
{''}
+ ) + assertValidPatch( +
{''}
, +
Hello
+ ) + }) + it('can replace a child with a text child and vice versa', function () { assertValidPatch(
HelloWorld
, From 3444b6c838254ced13bff5b7a3a029005f3d3954 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 21 Feb 2017 20:09:42 +0100 Subject: [PATCH 25/41] Don't use `for...of` and `for...in` loops in hot code paths Signed-off-by: Nathan Sobo --- src/patch.js | 8 ++++---- src/render.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/patch.js b/src/patch.js index 86bf431..9951c52 100644 --- a/src/patch.js +++ b/src/patch.js @@ -129,8 +129,8 @@ function removeVirtualNode (virtualNode, refs, removeDOMNode = true) { } else { if (refs && ref && refs[ref] === domNode) delete refs[ref] if (children) { - for (const child of children) { - removeVirtualNode(child, refs, false) + for (let i = 0; i < children.length; i++) { + removeVirtualNode(children[i], refs, false) } } } @@ -139,8 +139,8 @@ function removeVirtualNode (virtualNode, refs, removeDOMNode = true) { } function removeRefs (virtualNode, refs) { - for (const child of children) { - removeRefs(child, refs) + for (let i = 0; i < children.length; i++) { + removeRefs(children[i], refs) } } diff --git a/src/render.js b/src/render.js index 2d3ecbb..a765eff 100644 --- a/src/render.js +++ b/src/render.js @@ -30,7 +30,7 @@ export default function render (virtualNode, options) { } function addChildren (parent, children, options) { - for (let child of children) { - parent.appendChild(render(child, options)) + for (let i = 0; i < children.length; i++) { + parent.appendChild(render(children[i], options)) } } From 50b4a6922ec6db9794e6219acbfca634888e9281 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Feb 2017 09:01:35 +0100 Subject: [PATCH 26/41] Add support for SVG elements --- src/render.js | 5 + src/svg-attribute-translations.js | 146 +++++++++++++++--------------- src/update-props.js | 22 +++-- test/unit/patch.test.js | 20 ++++ 4 files changed, 113 insertions(+), 80 deletions(-) diff --git a/src/render.js b/src/render.js index a765eff..be3e205 100644 --- a/src/render.js +++ b/src/render.js @@ -1,4 +1,5 @@ import updateProps from './update-props' +import SVG_TAGS from './svg-tags' export default function render (virtualNode, options) { let domNode @@ -19,6 +20,10 @@ export default function render (virtualNode, options) { if (options && options.refs && ref) { options.refs[ref] = component } + } else if (SVG_TAGS.has(tag)) { + domNode = document.createElementNS("http://www.w3.org/2000/svg", tag); + if (children) addChildren(domNode, children, options) + if (props) updateProps(domNode, null, virtualNode, options) } else { domNode = document.createElement(tag) if (children) addChildren(domNode, children, options) diff --git a/src/svg-attribute-translations.js b/src/svg-attribute-translations.js index d46f3f2..49c1878 100644 --- a/src/svg-attribute-translations.js +++ b/src/svg-attribute-translations.js @@ -1,74 +1,74 @@ // Based on https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute -export default { - accentHeight: 'accent-height', - alignmentBaseline: 'alignment-baseline', - arabicForm: 'arabic-form', - baselineShift: 'baseline-shift', - capHeight: 'cap-height', - className: 'class', - clipPath: 'clip-path', - clipRule: 'clip-rule', - colorInterpolation: 'color-interpolation', - colorInterpolationFilters: 'color-interpolation-filters', - colorProfile: 'color-profile', - colorRendering: 'color-rendering', - dominantBaseline: 'dominant-baseline', - enableBackground: 'enable-background', - fillOpacity: 'fill-opacity', - fillRule: 'fill-rule', - floodColor: 'flood-color', - floodOpacity: 'flood-opacity', - fontFamily: 'font-family', - fontSize: 'font-size', - fontSizeAdjust: 'font-size-adjust', - fontStretch: 'font-stretch', - fontStyle: 'font-style', - fontVariant: 'font-variant', - fontWeight: 'font-weight', - glyphName: 'glyph-name', - glyphOrientationHorizontal: 'glyph-orientation-horizontal', - glyphOrientationVertical: 'glyph-orientation-vertical', - horizAdvX: 'horiz-adv-x', - horizOriginX: 'horiz-origin-x', - letterSpacing: 'letter-spacing', - lightingColor: 'lighting-color', - markerEnd: 'marker-end', - markerMid: 'marker-mid', - markerStart: 'marker-start', - overlinePosition: 'overline-position', - overlineThickness: 'overline-thickness', - panose1: 'panose-1', - paintOrder: 'paint-order', - pointerEvents: 'pointer-events', - renderingIntent: 'rendering-intent', - shapeRendering: 'shape-rendering', - stopColor: 'stop-color', - stopOpacity: 'stop-opacity', - strikethroughPosition: 'strikethrough-position', - strikethroughThickness: 'strikethrough-thickness', - strokeDasharray: 'stroke-dasharray', - strokeDashoffset: 'stroke-dashoffset', - strokeLinecap: 'stroke-linecap', - strokeLinejoin: 'stroke-linejoin', - strokeMiterlimit: 'stroke-miterlimit', - strokeOpacity: 'stroke-opacity', - strokeWidth: 'stroke-width', - textAnchor: 'text-anchor', - textDecoration: 'text-decoration', - textRendering: 'text-rendering', - underlinePosition: 'underline-position', - underlineThickness: 'underline-thickness', - unicodeBidi: 'unicode-bidi', - unicodeRange: 'unicode-range', - unitsPerEm: 'units-per-em', - vAlphabetic: 'v-alphabetic', - vHanging: 'v-hanging', - vIdeographic: 'v-ideographic', - vMathematical: 'v-mathematical', - vertAdvY: 'vert-adv-y', - vertOriginX: 'vert-origin-x', - vertOriginY: 'vert-origin-y', - wordSpacing: 'word-spacing', - writingMode: 'writing-mode', - xHeight: 'x-height', -} +export default new Map([ + ['accentHeight', 'accent-height'], + ['alignmentBaseline', 'alignment-baseline'], + ['arabicForm', 'arabic-form'], + ['baselineShift', 'baseline-shift'], + ['capHeight', 'cap-height'], + ['className', 'class'], + ['clipPath', 'clip-path'], + ['clipRule', 'clip-rule'], + ['colorInterpolation', 'color-interpolation'], + ['colorInterpolationFilters', 'color-interpolation-filters'], + ['colorProfile', 'color-profile'], + ['colorRendering', 'color-rendering'], + ['dominantBaseline', 'dominant-baseline'], + ['enableBackground', 'enable-background'], + ['fillOpacity', 'fill-opacity'], + ['fillRule', 'fill-rule'], + ['floodColor', 'flood-color'], + ['floodOpacity', 'flood-opacity'], + ['fontFamily', 'font-family'], + ['fontSize', 'font-size'], + ['fontSizeAdjust', 'font-size-adjust'], + ['fontStretch', 'font-stretch'], + ['fontStyle', 'font-style'], + ['fontVariant', 'font-variant'], + ['fontWeight', 'font-weight'], + ['glyphName', 'glyph-name'], + ['glyphOrientationHorizontal', 'glyph-orientation-horizontal'], + ['glyphOrientationVertical', 'glyph-orientation-vertical'], + ['horizAdvX', 'horiz-adv-x'], + ['horizOriginX', 'horiz-origin-x'], + ['letterSpacing', 'letter-spacing'], + ['lightingColor', 'lighting-color'], + ['markerEnd', 'marker-end'], + ['markerMid', 'marker-mid'], + ['markerStart', 'marker-start'], + ['overlinePosition', 'overline-position'], + ['overlineThickness', 'overline-thickness'], + ['panose1', 'panose-1'], + ['paintOrder', 'paint-order'], + ['pointerEvents', 'pointer-events'], + ['renderingIntent', 'rendering-intent'], + ['shapeRendering', 'shape-rendering'], + ['stopColor', 'stop-color'], + ['stopOpacity', 'stop-opacity'], + ['strikethroughPosition', 'strikethrough-position'], + ['strikethroughThickness', 'strikethrough-thickness'], + ['strokeDasharray', 'stroke-dasharray'], + ['strokeDashoffset', 'stroke-dashoffset'], + ['strokeLinecap', 'stroke-linecap'], + ['strokeLinejoin', 'stroke-linejoin'], + ['strokeMiterlimit', 'stroke-miterlimit'], + ['strokeOpacity', 'stroke-opacity'], + ['strokeWidth', 'stroke-width'], + ['textAnchor', 'text-anchor'], + ['textDecoration', 'text-decoration'], + ['textRendering', 'text-rendering'], + ['underlinePosition', 'underline-position'], + ['underlineThickness', 'underline-thickness'], + ['unicodeBidi', 'unicode-bidi'], + ['unicodeRange', 'unicode-range'], + ['unitsPerEm', 'units-per-em'], + ['vAlphabetic', 'v-alphabetic'], + ['vHanging', 'v-hanging'], + ['vIdeographic', 'v-ideographic'], + ['vMathematical', 'v-mathematical'], + ['vertAdvY', 'vert-adv-y'], + ['vertOriginX', 'vert-origin-x'], + ['vertOriginY', 'vert-origin-y'], + ['wordSpacing', 'word-spacing'], + ['writingMode', 'writing-mode'], + ['xHeight', 'x-height'], +]) diff --git a/src/update-props.js b/src/update-props.js index f771869..de0cf41 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -1,4 +1,6 @@ import EVENT_LISTENER_PROPS from './event-listener-props' +import SVG_TAGS from './svg-tags' +import SVG_ATTRIBUTE_TRANSLATIONS from './svg-attribute-translations' export default function (domNode, oldVirtualNode, newVirtualNode, options) { const oldProps = oldVirtualNode && oldVirtualNode.props @@ -9,12 +11,12 @@ export default function (domNode, oldVirtualNode, newVirtualNode, options) { refs = options.refs listenerContext = options.listenerContext } - updateProps(domNode, oldProps, newProps) + updateProps(domNode, oldVirtualNode, oldProps, newVirtualNode, newProps) if (refs) updateRef(domNode, oldProps && oldProps.ref, newProps && newProps.ref, refs) updateEventListeners(domNode, oldVirtualNode, newVirtualNode, listenerContext) } -function updateProps (domNode, oldProps, newProps) { +function updateProps (domNode, oldVirtualNode, oldProps, newVirtualNode, newProps) { if (oldProps) { const oldPropsNames = Object.keys(oldProps) for (let i = 0; i < oldPropsNames.length; i++) { @@ -23,9 +25,11 @@ function updateProps (domNode, oldProps, newProps) { if (name in EVENT_LISTENER_PROPS) continue if (!newProps || !(name in newProps)) { if (name === 'dataset') { - updateProps(domNode.dataset, oldProps ? oldProps.dataset : null, null) + updateProps(domNode.dataset, null, oldProps && oldProps.dataset, null, null) } else if (name === 'style') { - updateProps(domNode.style, oldProps ? oldProps.style : null, null) + updateProps(domNode.style, null, oldProps && oldProps.style, null, null) + } else if (oldVirtualNode && SVG_TAGS.has(oldVirtualNode.tag)) { + domNode.removeAttribute(SVG_ATTRIBUTE_TRANSLATIONS.get(name) || name) } else { delete domNode[name] } @@ -42,12 +46,16 @@ function updateProps (domNode, oldProps, newProps) { const oldValue = oldProps && oldProps[name] const newValue = newProps[name] if (name === 'dataset') { - updateProps(domNode.dataset, oldProps ? oldProps.dataset : null, newProps.dataset) + updateProps(domNode.dataset, null, oldProps && oldProps.dataset, null, newProps.dataset) } else if (name === 'style') { - updateProps(domNode.style, oldProps ? oldProps.style : null, newProps.style) + updateProps(domNode.style, null, oldProps && oldProps.style, null, newProps.style) } else { if (newValue !== oldValue) { - domNode[name] = newValue + if (newVirtualNode && SVG_TAGS.has(newVirtualNode.tag)) { + domNode.setAttribute(SVG_ATTRIBUTE_TRANSLATIONS.get(name) || name, newValue) + } else { + domNode[name] = newValue + } } } } diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 5354262..f109bf9 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -451,6 +451,26 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { assert.equal(element.outerHTML, render(
).outerHTML) }) }) + + describe('svg elements', function () { + it('can insert, delete, update and move nodes', function () { + assertValidPatch( + + Hello, world + + + , + + Goodbye, moon + + + + + + + ) + }) + }) }) function assertValidPatch (oldVirtualNode, newVirtualNode, seed) { From ca8d2c72fa2980ded130581324d81c4635005f11 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Feb 2017 09:12:26 +0100 Subject: [PATCH 27/41] Support assigning `innerHTML` to svg tags --- src/update-props.js | 4 ++-- test/unit/dom.test.js | 12 ++++++++++++ test/unit/patch.test.js | 8 +++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/update-props.js b/src/update-props.js index de0cf41..3acdd8c 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -28,7 +28,7 @@ function updateProps (domNode, oldVirtualNode, oldProps, newVirtualNode, newProp updateProps(domNode.dataset, null, oldProps && oldProps.dataset, null, null) } else if (name === 'style') { updateProps(domNode.style, null, oldProps && oldProps.style, null, null) - } else if (oldVirtualNode && SVG_TAGS.has(oldVirtualNode.tag)) { + } else if (name !== 'innerHTML' && oldVirtualNode && SVG_TAGS.has(oldVirtualNode.tag)) { domNode.removeAttribute(SVG_ATTRIBUTE_TRANSLATIONS.get(name) || name) } else { delete domNode[name] @@ -51,7 +51,7 @@ function updateProps (domNode, oldVirtualNode, oldProps, newVirtualNode, newProp updateProps(domNode.style, null, oldProps && oldProps.style, null, newProps.style) } else { if (newValue !== oldValue) { - if (newVirtualNode && SVG_TAGS.has(newVirtualNode.tag)) { + if (name !== 'innerHTML' && newVirtualNode && SVG_TAGS.has(newVirtualNode.tag)) { domNode.setAttribute(SVG_ATTRIBUTE_TRANSLATIONS.get(name) || name, newValue) } else { domNode[name] = newValue diff --git a/test/unit/dom.test.js b/test/unit/dom.test.js index 468ac46..2a004fb 100644 --- a/test/unit/dom.test.js +++ b/test/unit/dom.test.js @@ -40,6 +40,18 @@ describe('etch.dom', () => { expect(component.element.outerHTML).to.equal('') }) + it('supports assigning innerHTML to SVG tags', function () { + let component = { + render () { + return + }, + update () {} + } + + etch.initialize(component) + expect(component.element.outerHTML).to.equal('') + }) + describe('when a component constructor is used as a tag name', () => { describe('on initial render', () => { it('constructs the component with the specified properties and children, then appends its element to the DOM', () => { diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index f109bf9..c464aa8 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -443,7 +443,6 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => {
).outerHTML) - global.debug = true const virtualNode4 =
patch(virtualNode3, virtualNode4, {refs}) assert.equal(component.updateCount, 1) @@ -470,6 +469,13 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { ) }) + + it('can update the innerHTML property', function () { + assertValidPatch( + , + + ) + }) }) }) From de67ebd68dcf6bdea2927a7d1f902b0c825be30f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Feb 2017 09:29:55 +0100 Subject: [PATCH 28/41] Remove babel constructs from production code --- README.md | 10 +++++----- package.json | 8 +++----- src/component-helpers.js | 22 ++++++++++++++-------- src/default-scheduler.js | 2 +- src/dom.js | 6 ++++-- src/event-listener-props.js | 2 +- src/index.js | 10 ++++------ src/patch.js | 10 ++++++---- src/render.js | 12 ++++++++---- src/scheduler-assignment.js | 6 +++--- src/svg-attribute-translations.js | 2 +- src/svg-tags.js | 2 +- src/update-props.js | 8 ++++---- test/helpers/setup.js | 2 +- test/unit/default-scheduler.test.js | 2 +- test/unit/destroy-sync.test.js | 2 +- test/unit/destroy.test.js | 2 +- test/unit/dom.test.js | 2 +- test/unit/initialize.test.js | 2 +- test/unit/patch.test.js | 10 +++++----- test/unit/render.test.js | 6 +++--- test/unit/svg.test.js | 2 +- test/unit/update-sync.test.js | 2 +- test/unit/update.test.js | 2 +- 24 files changed, 72 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index 7867a07..261279a 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Etch components are ordinary JavaScript objects that conform to a minimal interf ```js /** @jsx etch.dom */ -import etch from 'etch' +const etch = require('etch') class MyComponent { // Required: Define an ordinary constructor to initialize your component. @@ -74,7 +74,7 @@ This function is typically called at the end of your component's constructor: ```js /** @jsx etch.dom */ -import etch from 'etch' +const etch = require('etch') class MyComponent { constructor (properties) { @@ -102,7 +102,7 @@ This function takes a component that is already associated with an `.element` pr ```js /** @jsx etch.dom */ -import etch from 'etch' +const etch = require('etch') class MyComponent { constructor (properties) { @@ -181,7 +181,7 @@ Components can be nested within other components by referencing a child componen ```js /** @jsx etch.dom */ -import etch from 'etch' +const etch = require('etch') class ChildComponent { constructor () { @@ -212,7 +212,7 @@ A constructor function can always take the place of a tag name in any Etch JSX e ```js /** @jsx etch.dom */ -import etch from 'etch' +const etch = require('etch') class ChildComponent { constructor (properties, children) { diff --git a/package.json b/package.json index 48572c8..63b1971 100644 --- a/package.json +++ b/package.json @@ -2,15 +2,13 @@ "name": "etch", "version": "0.7.2", "description": "Perform virtual DOM updates based on changes to a data model.", - "main": "dist/index.js", + "main": "src/index.js", "scripts": { "test": "npm run standard && npm run mocha", "mocha": "electron-mocha --renderer --interactive --recursive ./test/helpers/register-babel test", "tdd": "npm run standard && node_modules/.bin/electron-mocha --renderer --interactive --recursive ./test/helpers/register-babel test", - "prepublish": "npm run standard && npm run clean && npm run build", - "standard": "node_modules/.bin/standard --recursive src test && echo Linting passed", - "build": "node_modules/.bin/babel src --out-dir dist", - "clean": "rm -rf dist" + "prepublish": "npm run standard", + "standard": "node_modules/.bin/standard --recursive src test && echo Linting passed" }, "repository": { "type": "git", diff --git a/src/component-helpers.js b/src/component-helpers.js index f12315a..c84495f 100644 --- a/src/component-helpers.js +++ b/src/component-helpers.js @@ -1,6 +1,6 @@ -import render from './render' -import patch from './patch' -import {getScheduler} from './scheduler-assignment' +const render = require('./render') +const patch = require('./patch') +const {getScheduler} = require('./scheduler-assignment') const componentsWithPendingUpdates = new WeakSet let syncUpdatesInProgressCounter = 0 @@ -24,7 +24,7 @@ function isValidVirtualNode (virtualNode) { // nodes of the `virtual-dom` tree. Before calling into `virtual-dom` to create // the DOM tree, it pushes this `refs` object to a shared stack so it can be // accessed by hooks during the creation of individual elements. -export function initialize(component) { +function initialize(component) { if (typeof component.update !== 'function') { throw new Error('Etch components must implement `update(props, children)`.') } @@ -57,7 +57,7 @@ export function initialize(component) { // // Returns a promise that will resolve when the requested update has been // completed. -export function update (component, replaceNode=true) { +function update (component, replaceNode=true) { if (syncUpdatesInProgressCounter > 0) { updateSync(component, replaceNode) return Promise.resolve() @@ -95,7 +95,7 @@ export function update (component, replaceNode=true) { // For now, etch does not allow the root tag of the `render` method to change // between invocations, because we want to preserve a one-to-one relationship // between component objects and DOM elements for simplicity. -export function updateSync (component, replaceNode=true) { +function updateSync (component, replaceNode=true) { let newVirtualNode = component.render() if (!isValidVirtualNode(newVirtualNode)) { let namePart = component.constructor && component.constructor.name ? ' in ' + component.constructor.name : '' @@ -141,7 +141,7 @@ export function updateSync (component, replaceNode=true) { // If called as the result of destroying a component higher in the DOM, the // element is not removed to avoid redundant DOM manipulation. Returns a promise // that resolves when the destruction is completed. -export function destroy (component, removeNode=true) { +function destroy (component, removeNode=true) { if (syncUpdatesInProgressCounter > 0 || syncDestructionsInProgressCounter > 0) { destroySync(component, removeNode) return Promise.resolve() @@ -158,7 +158,7 @@ export function destroy (component, removeNode=true) { // // Note that we track whether `destroy` calls are in progress and only remove // the element if we are not a nested call. -export function destroySync (component, removeNode=true) { +function destroySync (component, removeNode=true) { syncDestructionsInProgressCounter++ destroyChildComponents(component.virtualNode) if (syncDestructionsInProgressCounter === 1 && removeNode) component.element.remove() @@ -172,3 +172,9 @@ function destroyChildComponents(virtualNode) { virtualNode.children.forEach(destroyChildComponents) } } + +module.exports = { + initialize, + update, updateSync, + destroy, destroySync +} diff --git a/src/default-scheduler.js b/src/default-scheduler.js index 4c20a8a..f4cc8a7 100644 --- a/src/default-scheduler.js +++ b/src/default-scheduler.js @@ -2,7 +2,7 @@ // this class will be used to schedule updates to the document. The // `updateDocument` method accepts functions to be run at some point in the // future, then runs them on the next animation frame. -export default class DefaultScheduler { +module.exports = class DefaultScheduler { constructor () { this.updateRequests = [] this.readRequests = [] diff --git a/src/dom.js b/src/dom.js index 4c0d268..28c1098 100644 --- a/src/dom.js +++ b/src/dom.js @@ -1,6 +1,6 @@ -import EVENT_LISTENER_PROPS from './event-listener-props' +const EVENT_LISTENER_PROPS = require('./event-listener-props') -export default function dom (tag, props, ...children) { +function dom (tag, props, ...children) { for (let i = 0; i < children.length;) { const child = children[i] if (Array.isArray(child)) { @@ -51,3 +51,5 @@ for (const tagName of TAG_NAMES) { return dom(tagName, props, ...children) } } + +module.exports = dom diff --git a/src/event-listener-props.js b/src/event-listener-props.js index d96da5c..71d1e94 100644 --- a/src/event-listener-props.js +++ b/src/event-listener-props.js @@ -1,4 +1,4 @@ -export default { +module.exports = { onCopy: 'copy', onCut: 'cut', onPaste: 'paste', diff --git a/src/index.js b/src/index.js index 0946ccb..1346615 100644 --- a/src/index.js +++ b/src/index.js @@ -1,11 +1,9 @@ -import dom from './dom' -import {initialize, update, updateSync, destroy, destroySync, onUpdate} from './component-helpers' -import {setScheduler, getScheduler} from './scheduler-assignment' +const dom = require('./dom') +const {initialize, update, updateSync, destroy, destroySync, onUpdate} = require('./component-helpers') +const {setScheduler, getScheduler} = require('./scheduler-assignment') -let etch = { +module.exports = { dom, initialize, update, updateSync, destroy, destroySync, onUpdate, setScheduler, getScheduler } - -export default etch diff --git a/src/patch.js b/src/patch.js index 9951c52..28a63b0 100644 --- a/src/patch.js +++ b/src/patch.js @@ -1,7 +1,7 @@ -import render from './render' -import updateProps from './update-props' +const render = require('./render') +const updateProps = require('./update-props') -export default function patch (oldVirtualNode, newVirtualNode, options) { +function patch (oldVirtualNode, newVirtualNode, options) { const oldNode = oldVirtualNode.domNode if (virtualNodesAreEqual(oldVirtualNode, newVirtualNode)) { let newNode @@ -46,7 +46,7 @@ function updateComponent (oldVirtualNode, newVirtualNode, options) { if (newRefName) refs[newRefName] = component } if (newRefName) { - newProps = {...newProps} + newProps = Object.assign({}, newProps) delete newProps.ref } } @@ -163,3 +163,5 @@ function mapOldKeysToIndices (children, startIndex, endIndex) { } return oldIndicesByKey; } + +module.exports = patch diff --git a/src/render.js b/src/render.js index be3e205..13a13ed 100644 --- a/src/render.js +++ b/src/render.js @@ -1,7 +1,7 @@ -import updateProps from './update-props' -import SVG_TAGS from './svg-tags' +const updateProps = require('./update-props') +const SVG_TAGS = require('./svg-tags') -export default function render (virtualNode, options) { +function render (virtualNode, options) { let domNode if (virtualNode.text != null) { domNode = document.createTextNode(virtualNode.text) @@ -12,7 +12,9 @@ export default function render (virtualNode, options) { if (typeof tag === 'function') { let ref if (props && props.ref) { - ({ref, ...props} = props) + ref = props.ref + props = Object.assign({}, props) + delete props['ref'] } const component = new tag(props || {}, children) virtualNode.component = component @@ -39,3 +41,5 @@ function addChildren (parent, children, options) { parent.appendChild(render(children[i], options)) } } + +module.exports = render diff --git a/src/scheduler-assignment.js b/src/scheduler-assignment.js index c6034bc..ac3b13a 100644 --- a/src/scheduler-assignment.js +++ b/src/scheduler-assignment.js @@ -22,15 +22,15 @@ // associated functions repeatedly. Again, they should be scheduled in such a // way so as to avoid synchronous reflows. -import DefaultScheduler from './default-scheduler' +const DefaultScheduler = require('./default-scheduler') let scheduler = null -export function setScheduler (customScheduler) { +module.exports.setScheduler = function setScheduler (customScheduler) { scheduler = customScheduler } -export function getScheduler () { +module.exports.getScheduler = function getScheduler () { if (!scheduler) { scheduler = new DefaultScheduler() } diff --git a/src/svg-attribute-translations.js b/src/svg-attribute-translations.js index 49c1878..c38b4b4 100644 --- a/src/svg-attribute-translations.js +++ b/src/svg-attribute-translations.js @@ -1,5 +1,5 @@ // Based on https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute -export default new Map([ +module.exports = new Map([ ['accentHeight', 'accent-height'], ['alignmentBaseline', 'alignment-baseline'], ['arabicForm', 'arabic-form'], diff --git a/src/svg-tags.js b/src/svg-tags.js index 8c23138..653ea19 100644 --- a/src/svg-tags.js +++ b/src/svg-tags.js @@ -1,5 +1,5 @@ // taken from https://github.com/facebook/react/blob/67f8524e88abbf1ac0fd86d38a0477d11fbc7b3e/src/isomorphic/classic/element/ReactDOMFactories.js#L153 -export default new Set([ +module.exports = new Set([ 'circle', 'clipPath', 'defs', diff --git a/src/update-props.js b/src/update-props.js index 3acdd8c..fabed6c 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -1,8 +1,8 @@ -import EVENT_LISTENER_PROPS from './event-listener-props' -import SVG_TAGS from './svg-tags' -import SVG_ATTRIBUTE_TRANSLATIONS from './svg-attribute-translations' +const EVENT_LISTENER_PROPS = require('./event-listener-props') +const SVG_TAGS = require('./svg-tags') +const SVG_ATTRIBUTE_TRANSLATIONS = require('./svg-attribute-translations') -export default function (domNode, oldVirtualNode, newVirtualNode, options) { +module.exports = function (domNode, oldVirtualNode, newVirtualNode, options) { const oldProps = oldVirtualNode && oldVirtualNode.props const newProps = newVirtualNode.props diff --git a/test/helpers/setup.js b/test/helpers/setup.js index 4ea0650..da2d686 100644 --- a/test/helpers/setup.js +++ b/test/helpers/setup.js @@ -1,6 +1,6 @@ /* global beforeEach */ -import chai from 'chai' +const chai = require('chai') global.expect = chai.expect beforeEach(function () { diff --git a/test/unit/default-scheduler.test.js b/test/unit/default-scheduler.test.js index 09e028b..ca22d0d 100644 --- a/test/unit/default-scheduler.test.js +++ b/test/unit/default-scheduler.test.js @@ -1,4 +1,4 @@ -import DefaultScheduler from '../../src/default-scheduler' +const DefaultScheduler = require('../../src/default-scheduler') describe('DefaultScheduler', () => { let scheduler diff --git a/test/unit/destroy-sync.test.js b/test/unit/destroy-sync.test.js index 407a8a7..fc52baa 100644 --- a/test/unit/destroy-sync.test.js +++ b/test/unit/destroy-sync.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -import etch from '../../src/index' +const etch = require('../../src/index') describe('etch.destroySync(component)', () => { it('synchronously removes the component\'s element from the document and calls `destroy` on child components', () => { diff --git a/test/unit/destroy.test.js b/test/unit/destroy.test.js index d70c504..d7b2895 100644 --- a/test/unit/destroy.test.js +++ b/test/unit/destroy.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -import etch from '../../src/index' +const etch = require('../../src/index') describe('etch.destroy(component)', () => { it('removes the component\'s element from the document and calls `destroy` on child components', async () => { diff --git a/test/unit/dom.test.js b/test/unit/dom.test.js index 2a004fb..3954868 100644 --- a/test/unit/dom.test.js +++ b/test/unit/dom.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -import etch from '../../src/index' +const etch = require('../../src/index') describe('etch.dom', () => { it('defaults properties to an empty object', () => { diff --git a/test/unit/initialize.test.js b/test/unit/initialize.test.js index 0a3d4db..58e4299 100644 --- a/test/unit/initialize.test.js +++ b/test/unit/initialize.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -import etch from '../../src/index' +const etch = require('../../src/index') describe('etch.initialize(component)', () => { it('returns an element with content based on the render method of the given component', () => { diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index c464aa8..c972821 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -1,11 +1,11 @@ /** @jsx dom */ -import {assert} from 'chai' -import Random from 'random-seed' +const {assert} = require('chai') +const Random = require('random-seed') -import dom from '../../src/dom' -import render from '../../src/render' -import patch from '../../src/patch' +const dom = require('../../src/dom') +const render = require('../../src/render') +const patch = require('../../src/patch') describe('patch (oldVirtualNode, newVirtualNode)', () => { describe('attributes', function () { diff --git a/test/unit/render.test.js b/test/unit/render.test.js index 5d755cb..bcb24a8 100644 --- a/test/unit/render.test.js +++ b/test/unit/render.test.js @@ -1,9 +1,9 @@ /** @jsx dom */ -import {assert} from 'chai' +const {assert} = require('chai') -import dom from '../../src/dom' -import render from '../../src/render' +const dom = require('../../src/dom') +const render = require('../../src/render') describe('render (virtualNode)', () => { it('constructs DOM nodes from virtual DOM trees', function () { diff --git a/test/unit/svg.test.js b/test/unit/svg.test.js index 76021a6..30547ff 100644 --- a/test/unit/svg.test.js +++ b/test/unit/svg.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -import etch from '../../src/index' +const etch = require('../../src/index') describe('svg support', () => { it('sets the correct tag thingies', () => { diff --git a/test/unit/update-sync.test.js b/test/unit/update-sync.test.js index 97fa750..7922bdc 100644 --- a/test/unit/update-sync.test.js +++ b/test/unit/update-sync.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -import etch from '../../src/index' +const etch = require('../../src/index') describe('etch.updateSync(component)', () => { it('performs an update of the component\'s element and any resulting child updates synchronously', () => { diff --git a/test/unit/update.test.js b/test/unit/update.test.js index 684d778..1172de4 100644 --- a/test/unit/update.test.js +++ b/test/unit/update.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -import etch from '../../src/index' +const etch = require('../../src/index') describe('etch.update(component)', () => { it('schedules an update of the element associated with the component', async () => { From 47997a5351d837e154ae1e11b011dd783c36d642 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Feb 2017 09:51:59 +0100 Subject: [PATCH 29/41] Create a text node for children that are numbers --- src/dom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dom.js b/src/dom.js index 28c1098..accd558 100644 --- a/src/dom.js +++ b/src/dom.js @@ -5,7 +5,7 @@ function dom (tag, props, ...children) { const child = children[i] if (Array.isArray(child)) { children.splice(i, 1, ...child) - } else if (typeof child === 'string') { + } else if (typeof child === 'string' || typeof child === 'number') { children.splice(i, 1, {text: child}) i++ } else { From c248122b6481f13b9aeaca2c1928f1398317d0a3 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Feb 2017 10:13:43 +0100 Subject: [PATCH 30/41] Add functions to create SVG tags to `etch.dom` --- src/dom.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/dom.js b/src/dom.js index accd558..4dab8ab 100644 --- a/src/dom.js +++ b/src/dom.js @@ -1,4 +1,5 @@ const EVENT_LISTENER_PROPS = require('./event-listener-props') +const SVG_TAGS = require('./svg-tags') function dom (tag, props, ...children) { for (let i = 0; i < children.length;) { @@ -30,7 +31,7 @@ function dom (tag, props, ...children) { return {tag, props, children} } -const TAG_NAMES = [ +const HTML_TAGS = [ 'a', 'abbr', 'address', 'article', 'aside', 'audio', 'b', 'bdi', 'bdo', 'blockquote', 'body', 'button', 'canvas', 'caption', 'cite', 'code', 'colgroup', 'datalist', 'dd', 'del', 'details', 'dfn', 'dialog', 'div', 'dl', @@ -39,17 +40,24 @@ const TAG_NAMES = [ 'label', 'legend', 'li', 'main', 'map', 'mark', 'menu', 'meter', 'nav', 'noscript', 'object', 'ol', 'optgroup', 'option', 'output', 'p', 'pre', 'progress', 'q', 'rp', 'rt', 'ruby', 's', 'samp', 'script', 'section', - 'select', 'small', 'span', 'strong', 'style', 'sub', 'summary', 'sup', 'svg', + 'select', 'small', 'span', 'strong', 'style', 'sub', 'summary', 'sup', 'table', 'tbody', 'td', 'textarea', 'tfoot', 'th', 'thead', 'time', 'title', 'tr', 'u', 'ul', 'var', 'video', 'area', 'base', 'br', 'col', 'command', 'embed', 'hr', 'img', 'input', 'keygen', 'link', 'meta', 'param', 'source', 'track', 'wbr' ] -for (const tagName of TAG_NAMES) { +for (const tagName of HTML_TAGS) { dom[tagName] = (props, ...children) => { return dom(tagName, props, ...children) } } +for (const tagName of SVG_TAGS) { + dom[tagName] = (props, ...children) => { + return dom(tagName, props, ...children) + } +} + + module.exports = dom From 0b6ebad48350a5fd3aebbd6420bbec3d31919295 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 22 Feb 2017 14:16:14 +0100 Subject: [PATCH 31/41] Fix updating the `style` property on a DOM node --- src/update-props.js | 2 ++ test/unit/patch.test.js | 28 +++++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/update-props.js b/src/update-props.js index fabed6c..922fd6c 100644 --- a/src/update-props.js +++ b/src/update-props.js @@ -31,6 +31,8 @@ function updateProps (domNode, oldVirtualNode, oldProps, newVirtualNode, newProp } else if (name !== 'innerHTML' && oldVirtualNode && SVG_TAGS.has(oldVirtualNode.tag)) { domNode.removeAttribute(SVG_ATTRIBUTE_TRANSLATIONS.get(name) || name) } else { + // Null out property for objects that don't support deletion (e.g. style). + domNode[name] = null delete domNode[name] } } diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index c972821..9025cad 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -8,15 +8,37 @@ const render = require('../../src/render') const patch = require('../../src/patch') describe('patch (oldVirtualNode, newVirtualNode)', () => { - describe('attributes', function () { - it('can add, remove, and update attributes', function () { + describe('properties', function () { + it('can add, remove, and update properties', function () { assertValidPatch(
,
) }) - it('can update from no attributes to some attributes and vice versa', function () { + it('can update from no properties to some properties and vice versa', function () { assertValidPatch(
,
) assertValidPatch(
,
) }) + + it('correctly updates the `dataset` property', function () { + assertValidPatch( +
, +
, + ) + assertValidPatch( +
, +
+ ) + }) + + it('correctly updates the `style` property', function () { + assertValidPatch( +
, +
, + ) + assertValidPatch( +
, +
+ ) + }) }) describe('keyed children', function () { From 6128e3238ad532795e6dd8ee9e325cf5729ded34 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 22 Feb 2017 20:15:03 -0700 Subject: [PATCH 32/41] Export render --- src/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 1346615..eea4891 100644 --- a/src/index.js +++ b/src/index.js @@ -1,9 +1,10 @@ const dom = require('./dom') +const render = require('./render') const {initialize, update, updateSync, destroy, destroySync, onUpdate} = require('./component-helpers') const {setScheduler, getScheduler} = require('./scheduler-assignment') module.exports = { - dom, + dom, render, initialize, update, updateSync, destroy, destroySync, onUpdate, setScheduler, getScheduler } From 65da030acd33cc99e657c2a34f0752c9e80f3bb7 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 22 Feb 2017 20:15:10 -0700 Subject: [PATCH 33/41] =?UTF-8?q?Don=E2=80=99t=20run=20tests=20interactive?= =?UTF-8?q?ly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 63b1971..392d5e6 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "main": "src/index.js", "scripts": { "test": "npm run standard && npm run mocha", - "mocha": "electron-mocha --renderer --interactive --recursive ./test/helpers/register-babel test", + "mocha": "electron-mocha --renderer --recursive ./test/helpers/register-babel test", "tdd": "npm run standard && node_modules/.bin/electron-mocha --renderer --interactive --recursive ./test/helpers/register-babel test", "prepublish": "npm run standard", "standard": "node_modules/.bin/standard --recursive src test && echo Linting passed" From 2831c103a1d02ff3bc0cd35833138e9bbb8fe306 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Feb 2017 08:51:43 +0100 Subject: [PATCH 34/41] Don't pass the component key as a prop Refs: https://github.com/atom/etch/pull/37 --- src/patch.js | 19 +++++++++++++++---- src/render.js | 11 ++++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/patch.js b/src/patch.js index 28a63b0..03dcc2d 100644 --- a/src/patch.js +++ b/src/patch.js @@ -18,6 +18,7 @@ function patch (oldVirtualNode, newVirtualNode, options) { } } newVirtualNode.domNode = oldNode + newVirtualNode.key = oldVirtualNode.key if (newNode !== oldNode && oldNode.parentNode) { oldNode.parentNode.replaceChild(newNode, oldNode) } @@ -50,6 +51,12 @@ function updateComponent (oldVirtualNode, newVirtualNode, options) { delete newProps.ref } } + + if (newProps && newProps.key) { + newProps = Object.assign({}, newProps) + delete newProps.key + } + component.update(newProps || {}, newChildren) return component.element } @@ -92,7 +99,7 @@ function updateChildren (parentElement, oldChildren, newChildren, options) { newStartChild = newChildren[++newStartIndex] } else { if (!oldIndicesByKey) oldIndicesByKey = mapOldKeysToIndices(oldChildren, oldStartIndex, oldEndIndex) - const key = getKey(newStartChild) + const key = getNewVirtualNodeKey(newStartChild) const oldIndex = key ? oldIndicesByKey[key] : null if (oldIndex == null) { parentElement.insertBefore(render(newStartChild, options), oldStartChild.domNode) @@ -146,19 +153,23 @@ function removeRefs (virtualNode, refs) { function virtualNodesAreEqual (oldVirtualNode, newVirtualNode) { return ( - getKey(oldVirtualNode) === getKey(newVirtualNode) + getOldVirtualNodeKey(oldVirtualNode) === getNewVirtualNodeKey(newVirtualNode) && oldVirtualNode.tag === newVirtualNode.tag ) } -function getKey (virtualNode) { +function getOldVirtualNodeKey (virtualNode) { + return virtualNode.key +} + +function getNewVirtualNodeKey (virtualNode) { return virtualNode.props ? virtualNode.props.key : undefined } function mapOldKeysToIndices (children, startIndex, endIndex) { let oldIndicesByKey = {} for (let i = startIndex; i <= endIndex; i++) { - const key = getKey(children[i]) + const key = getOldVirtualNodeKey(children[i]) if (key) oldIndicesByKey[key] = i } return oldIndicesByKey; diff --git a/src/render.js b/src/render.js index 13a13ed..ec0482e 100644 --- a/src/render.js +++ b/src/render.js @@ -10,14 +10,23 @@ function render (virtualNode, options) { let {props} = virtualNode if (typeof tag === 'function') { - let ref + let ref, key + if (props && props.ref) { ref = props.ref props = Object.assign({}, props) delete props['ref'] } + + if (props && props.key) { + key = props.key + props = Object.assign({}, props) + delete props['key'] + } + const component = new tag(props || {}, children) virtualNode.component = component + virtualNode.key = key domNode = component.element if (options && options.refs && ref) { options.refs[ref] = component From 8a96251f90db2db0e4ef7300bba4133162869bbc Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Feb 2017 08:56:19 +0100 Subject: [PATCH 35/41] Use a newer electron version and the newest electron-mocha ...which supports --interactive --- package.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 991fe69..267d985 100644 --- a/package.json +++ b/package.json @@ -27,10 +27,11 @@ "babel": "^5.8.33", "babel-eslint": "^4.1.6", "chai": "^3.5.0", - "electron-mocha": "git://github.com/nathansobo/electron-mocha.git#interactive-option", - "electron-prebuilt": "^0.37.8", + "electron": "1.3.13", + "electron-mocha": "3.3.0", "estraverse-fb": "^1.3.1", "mocha": "^3.1.2", + "random-seed": "^0.3.0", "standard": "^8.5.0" }, "standard": { From 19475eb24de5a6e088a906c14ae4efb6ef8fae54 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Feb 2017 09:06:03 +0100 Subject: [PATCH 36/41] Replace src with lib --- .npmignore | 1 - README.md | 6 +++--- {src => lib}/component-helpers.js | 0 {src => lib}/default-scheduler.js | 0 {src => lib}/dom.js | 0 {src => lib}/event-listener-props.js | 0 {src => lib}/index.js | 4 ++-- {src => lib}/patch.js | 0 {src => lib}/render.js | 0 {src => lib}/scheduler-assignment.js | 0 {src => lib}/svg-attribute-translations.js | 0 {src => lib}/svg-tags.js | 0 {src => lib}/update-props.js | 0 package.json | 4 ++-- test/unit/default-scheduler.test.js | 2 +- test/unit/destroy-sync.test.js | 2 +- test/unit/destroy.test.js | 2 +- test/unit/dom.test.js | 2 +- test/unit/initialize.test.js | 2 +- test/unit/patch.test.js | 6 +++--- test/unit/render.test.js | 4 ++-- test/unit/svg.test.js | 2 +- test/unit/update-sync.test.js | 2 +- test/unit/update.test.js | 2 +- 24 files changed, 20 insertions(+), 21 deletions(-) rename {src => lib}/component-helpers.js (100%) rename {src => lib}/default-scheduler.js (100%) rename {src => lib}/dom.js (100%) rename {src => lib}/event-listener-props.js (100%) rename {src => lib}/index.js (54%) rename {src => lib}/patch.js (100%) rename {src => lib}/render.js (100%) rename {src => lib}/scheduler-assignment.js (100%) rename {src => lib}/svg-attribute-translations.js (100%) rename {src => lib}/svg-tags.js (100%) rename {src => lib}/update-props.js (100%) diff --git a/.npmignore b/.npmignore index 121ef01..9daeafb 100644 --- a/.npmignore +++ b/.npmignore @@ -1,2 +1 @@ test -src diff --git a/README.md b/README.md index 5ab2a67..7930e36 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ class MyComponent { async destroy () { // call etch.destroy to remove the element and destroy child components await etch.destroy(this) - // then perform custom teardown logic here... + // then perform custom teardown logic here... } } ``` @@ -365,6 +365,6 @@ Compared to efficiently updating the DOM declaratively (the primary focus of thi Etch aims to stay small and focused. If you have a feature idea, consider implementing it as a library that either wraps Etch or, even better, that can be used in concert with it. If it's impossible to implement your feature outside of Etch, we can discuss adding a hook that makes your feature possible. [babel]: https://babeljs.io/ -[scheduler-assignment]: https://github.com/nathansobo/etch/blob/master/src/scheduler-assignment.js -[default-scheduler]: https://github.com/nathansobo/etch/blob/master/src/default-scheduler.js +[scheduler-assignment]: https://github.com/nathansobo/etch/blob/master/lib/scheduler-assignment.js +[default-scheduler]: https://github.com/nathansobo/etch/blob/master/lib/default-scheduler.js [dom-listener]: https://github.com/atom/dom-listener diff --git a/src/component-helpers.js b/lib/component-helpers.js similarity index 100% rename from src/component-helpers.js rename to lib/component-helpers.js diff --git a/src/default-scheduler.js b/lib/default-scheduler.js similarity index 100% rename from src/default-scheduler.js rename to lib/default-scheduler.js diff --git a/src/dom.js b/lib/dom.js similarity index 100% rename from src/dom.js rename to lib/dom.js diff --git a/src/event-listener-props.js b/lib/event-listener-props.js similarity index 100% rename from src/event-listener-props.js rename to lib/event-listener-props.js diff --git a/src/index.js b/lib/index.js similarity index 54% rename from src/index.js rename to lib/index.js index eea4891..34e268e 100644 --- a/src/index.js +++ b/lib/index.js @@ -1,10 +1,10 @@ const dom = require('./dom') const render = require('./render') -const {initialize, update, updateSync, destroy, destroySync, onUpdate} = require('./component-helpers') +const {initialize, update, updateSync, destroy, destroySync} = require('./component-helpers') const {setScheduler, getScheduler} = require('./scheduler-assignment') module.exports = { dom, render, - initialize, update, updateSync, destroy, destroySync, onUpdate, + initialize, update, updateSync, destroy, destroySync, setScheduler, getScheduler } diff --git a/src/patch.js b/lib/patch.js similarity index 100% rename from src/patch.js rename to lib/patch.js diff --git a/src/render.js b/lib/render.js similarity index 100% rename from src/render.js rename to lib/render.js diff --git a/src/scheduler-assignment.js b/lib/scheduler-assignment.js similarity index 100% rename from src/scheduler-assignment.js rename to lib/scheduler-assignment.js diff --git a/src/svg-attribute-translations.js b/lib/svg-attribute-translations.js similarity index 100% rename from src/svg-attribute-translations.js rename to lib/svg-attribute-translations.js diff --git a/src/svg-tags.js b/lib/svg-tags.js similarity index 100% rename from src/svg-tags.js rename to lib/svg-tags.js diff --git a/src/update-props.js b/lib/update-props.js similarity index 100% rename from src/update-props.js rename to lib/update-props.js diff --git a/package.json b/package.json index 267d985..38d893d 100644 --- a/package.json +++ b/package.json @@ -2,13 +2,13 @@ "name": "etch", "version": "0.8.0", "description": "Perform virtual DOM updates based on changes to a data model.", - "main": "src/index.js", + "main": "lib/index.js", "scripts": { "test": "npm run standard && npm run mocha", "mocha": "electron-mocha --renderer --recursive ./test/helpers/register-babel test", "tdd": "npm run standard && node_modules/.bin/electron-mocha --renderer --interactive --recursive ./test/helpers/register-babel test", "prepublish": "npm run standard", - "standard": "node_modules/.bin/standard --recursive src test && echo Linting passed" + "standard": "node_modules/.bin/standard --recursive lib test && echo Linting passed" }, "repository": { "type": "git", diff --git a/test/unit/default-scheduler.test.js b/test/unit/default-scheduler.test.js index ca22d0d..bf1053f 100644 --- a/test/unit/default-scheduler.test.js +++ b/test/unit/default-scheduler.test.js @@ -1,4 +1,4 @@ -const DefaultScheduler = require('../../src/default-scheduler') +const DefaultScheduler = require('../../lib/default-scheduler') describe('DefaultScheduler', () => { let scheduler diff --git a/test/unit/destroy-sync.test.js b/test/unit/destroy-sync.test.js index fc52baa..0a1e870 100644 --- a/test/unit/destroy-sync.test.js +++ b/test/unit/destroy-sync.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -const etch = require('../../src/index') +const etch = require('../../lib/index') describe('etch.destroySync(component)', () => { it('synchronously removes the component\'s element from the document and calls `destroy` on child components', () => { diff --git a/test/unit/destroy.test.js b/test/unit/destroy.test.js index d7b2895..e7d8209 100644 --- a/test/unit/destroy.test.js +++ b/test/unit/destroy.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -const etch = require('../../src/index') +const etch = require('../../lib/index') describe('etch.destroy(component)', () => { it('removes the component\'s element from the document and calls `destroy` on child components', async () => { diff --git a/test/unit/dom.test.js b/test/unit/dom.test.js index 8e601c5..3037a48 100644 --- a/test/unit/dom.test.js +++ b/test/unit/dom.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -const etch = require('../../src/index') +const etch = require('../../lib/index') describe('etch.dom', () => { it('defaults properties to an empty object', () => { diff --git a/test/unit/initialize.test.js b/test/unit/initialize.test.js index 59edcc5..09bfd18 100644 --- a/test/unit/initialize.test.js +++ b/test/unit/initialize.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -const etch = require('../../src/index') +const etch = require('../../lib/index') describe('etch.initialize(component)', () => { it('returns an element with content based on the render method of the given component', () => { diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 9025cad..2bcda2e 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -3,9 +3,9 @@ const {assert} = require('chai') const Random = require('random-seed') -const dom = require('../../src/dom') -const render = require('../../src/render') -const patch = require('../../src/patch') +const dom = require('../../lib/dom') +const render = require('../../lib/render') +const patch = require('../../lib/patch') describe('patch (oldVirtualNode, newVirtualNode)', () => { describe('properties', function () { diff --git a/test/unit/render.test.js b/test/unit/render.test.js index bcb24a8..1bebab8 100644 --- a/test/unit/render.test.js +++ b/test/unit/render.test.js @@ -2,8 +2,8 @@ const {assert} = require('chai') -const dom = require('../../src/dom') -const render = require('../../src/render') +const dom = require('../../lib/dom') +const render = require('../../lib/render') describe('render (virtualNode)', () => { it('constructs DOM nodes from virtual DOM trees', function () { diff --git a/test/unit/svg.test.js b/test/unit/svg.test.js index 30547ff..dbbfb4e 100644 --- a/test/unit/svg.test.js +++ b/test/unit/svg.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -const etch = require('../../src/index') +const etch = require('../../lib/index') describe('svg support', () => { it('sets the correct tag thingies', () => { diff --git a/test/unit/update-sync.test.js b/test/unit/update-sync.test.js index e48a1f8..d9c3419 100644 --- a/test/unit/update-sync.test.js +++ b/test/unit/update-sync.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -const etch = require('../../src/index') +const etch = require('../../lib/index') describe('etch.updateSync(component)', () => { it('performs an update of the component\'s element and any resulting child updates synchronously', () => { diff --git a/test/unit/update.test.js b/test/unit/update.test.js index 348ce75..f204d67 100644 --- a/test/unit/update.test.js +++ b/test/unit/update.test.js @@ -1,6 +1,6 @@ /** @jsx etch.dom */ -const etch = require('../../src/index') +const etch = require('../../lib/index') describe('etch.update(component)', () => { it('schedules an update of the element associated with the component', async () => { From 827f311ac2233f1e782d13ae192823d452a9402d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Feb 2017 09:39:16 +0100 Subject: [PATCH 37/41] Update README to describe the new event handling facility --- README.md | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7930e36..d001a5c 100644 --- a/README.md +++ b/README.md @@ -356,9 +356,31 @@ Read comments in the [scheduler assignment][scheduler-assignment] and [default s ### Handling Events -This library doesn't currently prescribe or support a specific approach to binding event handlers. We are considering an API that integrates inline handlers directly into JSX expressions, but we're not convinced the utility warrants the added surface area. +Etch supports listening to arbitrary events on a DOM node via the `on` property by specifying a hash of `eventName: handlerFunction` pairs: -Compared to efficiently updating the DOM declaratively (the primary focus of this library), binding events is a pretty simple problem. You might try [dom-listener][dom-listener] if you're looking for a library that you could combine with Etch to deal with event binding. +```js +class ComponentWithEvents { + constructor () { + etch.initialize(this) + } + + render () { + return
+ } + + didClick (event) { + console.log(event) // ==> MouseEvent {...} + console.log(this) // ==> ComponentWithEvents {...} + } + + didFocus (event) { + console.log(event) // ==> FocusEvent {...} + console.log(this) // ==> ComponentWithEvents {...} + } +} +``` + +As you can see, the function is automatically bound to the component which registered the event handler. Other than improving readability, this removes the need for rebinding the handler functions in the constructor or, worse, creating a new closure every time the component is updated. ### Feature Requests From 9761d7fc104b16444ced72f935681beea0e61b27 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 23 Feb 2017 07:08:20 -0700 Subject: [PATCH 38/41] Small README tweaks --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d001a5c..0e21732 100644 --- a/README.md +++ b/README.md @@ -356,7 +356,7 @@ Read comments in the [scheduler assignment][scheduler-assignment] and [default s ### Handling Events -Etch supports listening to arbitrary events on a DOM node via the `on` property by specifying a hash of `eventName: handlerFunction` pairs: +Etch supports listening to arbitrary events on DOM nodes via the special `on` property, which can be used to assign a hash of `eventName: listenerFunction` pairs: ```js class ComponentWithEvents { @@ -380,7 +380,7 @@ class ComponentWithEvents { } ``` -As you can see, the function is automatically bound to the component which registered the event handler. Other than improving readability, this removes the need for rebinding the handler functions in the constructor or, worse, creating a new closure every time the component is updated. +As you can see, the listener function's `this` value is automatically bound to the parent component. You should rely on this auto-binding facility rather than using arrow functions or `Function.bind` to avoid complexity and extraneous closure allocations. ### Feature Requests From bff2ffc1d31136c88780778388a30cc42920e57e Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 23 Feb 2017 07:29:11 -0700 Subject: [PATCH 39/41] Revert "Don't pass the component key as a prop" This reverts commit 2831c103a1d02ff3bc0cd35833138e9bbb8fe306. --- lib/patch.js | 19 ++++--------------- lib/render.js | 11 +---------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/lib/patch.js b/lib/patch.js index 03dcc2d..28a63b0 100644 --- a/lib/patch.js +++ b/lib/patch.js @@ -18,7 +18,6 @@ function patch (oldVirtualNode, newVirtualNode, options) { } } newVirtualNode.domNode = oldNode - newVirtualNode.key = oldVirtualNode.key if (newNode !== oldNode && oldNode.parentNode) { oldNode.parentNode.replaceChild(newNode, oldNode) } @@ -51,12 +50,6 @@ function updateComponent (oldVirtualNode, newVirtualNode, options) { delete newProps.ref } } - - if (newProps && newProps.key) { - newProps = Object.assign({}, newProps) - delete newProps.key - } - component.update(newProps || {}, newChildren) return component.element } @@ -99,7 +92,7 @@ function updateChildren (parentElement, oldChildren, newChildren, options) { newStartChild = newChildren[++newStartIndex] } else { if (!oldIndicesByKey) oldIndicesByKey = mapOldKeysToIndices(oldChildren, oldStartIndex, oldEndIndex) - const key = getNewVirtualNodeKey(newStartChild) + const key = getKey(newStartChild) const oldIndex = key ? oldIndicesByKey[key] : null if (oldIndex == null) { parentElement.insertBefore(render(newStartChild, options), oldStartChild.domNode) @@ -153,23 +146,19 @@ function removeRefs (virtualNode, refs) { function virtualNodesAreEqual (oldVirtualNode, newVirtualNode) { return ( - getOldVirtualNodeKey(oldVirtualNode) === getNewVirtualNodeKey(newVirtualNode) + getKey(oldVirtualNode) === getKey(newVirtualNode) && oldVirtualNode.tag === newVirtualNode.tag ) } -function getOldVirtualNodeKey (virtualNode) { - return virtualNode.key -} - -function getNewVirtualNodeKey (virtualNode) { +function getKey (virtualNode) { return virtualNode.props ? virtualNode.props.key : undefined } function mapOldKeysToIndices (children, startIndex, endIndex) { let oldIndicesByKey = {} for (let i = startIndex; i <= endIndex; i++) { - const key = getOldVirtualNodeKey(children[i]) + const key = getKey(children[i]) if (key) oldIndicesByKey[key] = i } return oldIndicesByKey; diff --git a/lib/render.js b/lib/render.js index ec0482e..13a13ed 100644 --- a/lib/render.js +++ b/lib/render.js @@ -10,23 +10,14 @@ function render (virtualNode, options) { let {props} = virtualNode if (typeof tag === 'function') { - let ref, key - + let ref if (props && props.ref) { ref = props.ref props = Object.assign({}, props) delete props['ref'] } - - if (props && props.key) { - key = props.key - props = Object.assign({}, props) - delete props['key'] - } - const component = new tag(props || {}, children) virtualNode.component = component - virtualNode.key = key domNode = component.element if (options && options.refs && ref) { options.refs[ref] = component From dc91dee711b441c87c1e79cc43a903805bb69eb1 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 24 Feb 2017 16:19:07 +0100 Subject: [PATCH 40/41] Assign an empty string instead of null to clear out DOM properties --- lib/update-props.js | 7 +++++-- test/unit/patch.test.js | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/update-props.js b/lib/update-props.js index 922fd6c..4a65e51 100644 --- a/lib/update-props.js +++ b/lib/update-props.js @@ -1,6 +1,7 @@ const EVENT_LISTENER_PROPS = require('./event-listener-props') const SVG_TAGS = require('./svg-tags') const SVG_ATTRIBUTE_TRANSLATIONS = require('./svg-attribute-translations') +const EMPTY = '' module.exports = function (domNode, oldVirtualNode, newVirtualNode, options) { const oldProps = oldVirtualNode && oldVirtualNode.props @@ -31,8 +32,10 @@ function updateProps (domNode, oldVirtualNode, oldProps, newVirtualNode, newProp } else if (name !== 'innerHTML' && oldVirtualNode && SVG_TAGS.has(oldVirtualNode.tag)) { domNode.removeAttribute(SVG_ATTRIBUTE_TRANSLATIONS.get(name) || name) } else { - // Null out property for objects that don't support deletion (e.g. style). - domNode[name] = null + // Clear property for objects that don't support deletion (e.g. style + // or className). If we used null instead of an empty string, the DOM + // could sometimes stringify the value and mistakenly assign 'null'. + domNode[name] = EMPTY delete domNode[name] } } diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 2bcda2e..6ef221d 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -39,6 +39,24 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => {
) }) + + it('correctly updates the `className` property', function () { + assertValidPatch( +
, +
, + ) + assertValidPatch( +
, +
+ ) + + const oldVirtualNode =
+ const oldNode = render(oldVirtualNode) + const newVirtualNode =
+ const newNode = render(newVirtualNode) + patch(oldNode, newNode) + assert(!newNode.className) + }) }) describe('keyed children', function () { From accde4d8d29db91e626b7d46df620a79239b4ab8 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 24 Feb 2017 16:23:45 +0100 Subject: [PATCH 41/41] Pass ref and key to child components to avoid copying objects --- lib/patch.js | 4 ---- lib/render.js | 2 -- test/unit/dom.test.js | 41 ++++------------------------------------- test/unit/patch.test.js | 10 ++-------- 4 files changed, 6 insertions(+), 51 deletions(-) diff --git a/lib/patch.js b/lib/patch.js index 28a63b0..a40d132 100644 --- a/lib/patch.js +++ b/lib/patch.js @@ -45,10 +45,6 @@ function updateComponent (oldVirtualNode, newVirtualNode, options) { if (oldRefName && refs[oldRefName] === component) delete refs[oldRefName] if (newRefName) refs[newRefName] = component } - if (newRefName) { - newProps = Object.assign({}, newProps) - delete newProps.ref - } } component.update(newProps || {}, newChildren) return component.element diff --git a/lib/render.js b/lib/render.js index 13a13ed..938f1f7 100644 --- a/lib/render.js +++ b/lib/render.js @@ -13,8 +13,6 @@ function render (virtualNode, options) { let ref if (props && props.ref) { ref = props.ref - props = Object.assign({}, props) - delete props['ref'] } const component = new tag(props || {}, children) virtualNode.component = component diff --git a/test/unit/dom.test.js b/test/unit/dom.test.js index 3037a48..8c03a67 100644 --- a/test/unit/dom.test.js +++ b/test/unit/dom.test.js @@ -287,7 +287,7 @@ describe('etch.dom', () => { }) describe('when the child component constructor tag has a ref property', () => { - it('creates a reference to the child component object on the parent component and does not pass the ref as a prop to the child component', async () => { + it('creates a reference to the child component object on the parent component', async () => { class ChildComponentA { constructor (properties) { this.properties = properties @@ -336,7 +336,7 @@ describe('etch.dom', () => { etch.initialize(parentComponent) expect(parentComponent.refs.child instanceof ChildComponentA).to.be.true - expect(parentComponent.refs.child.properties.ref).to.be.undefined + expect(parentComponent.refs.child.properties.ref).to.equal('child') expect(parentComponent.refs.child.refs.self.textContent).to.equal('A') parentComponent.refName = 'kid' @@ -344,7 +344,7 @@ describe('etch.dom', () => { expect(parentComponent.refs.child).to.be.undefined expect(parentComponent.refs.kid instanceof ChildComponentA).to.be.true - expect(parentComponent.refs.kid.properties.ref).to.be.undefined + expect(parentComponent.refs.kid.properties.ref).to.equal('kid') expect(parentComponent.refs.kid.refs.self.textContent).to.equal('A') parentComponent.refName = 'child' @@ -354,7 +354,7 @@ describe('etch.dom', () => { expect(parentComponent.refs.kid).to.be.undefined expect(parentComponent.refs.child instanceof ChildComponentB).to.be.true - expect(parentComponent.refs.child.properties.ref).to.be.undefined + expect(parentComponent.refs.child.properties.ref).to.equal('child') expect(parentComponent.refs.child.refs.self.textContent).to.equal('B') parentComponent.renderB = false @@ -417,38 +417,5 @@ describe('etch.dom', () => { expect(parentComponent.refs.child.constructor).to.equal(ChildComponentB) }) }) - - describe('when the child component constructor tag has a key property', () => { - it('does not pass the key to the child component', async function () { - class ChildComponentA { - constructor (properties) { - this.properties = properties - etch.initialize(this) - } - - render () { - return
A
- } - - update (properties) { - this.properties = properties - } - } - - let parentComponent = { - render () { - return
- }, - - update () {} - } - - etch.initialize(parentComponent) - - expect(parentComponent.refs.child.properties.key).to.be.undefined - await etch.update(parentComponent) - expect(parentComponent.refs.child.properties.key).to.be.undefined - }) - }) }) }) diff --git a/test/unit/patch.test.js b/test/unit/patch.test.js index 6ef221d..9940ed7 100644 --- a/test/unit/patch.test.js +++ b/test/unit/patch.test.js @@ -285,24 +285,18 @@ describe('patch (oldVirtualNode, newVirtualNode)', () => { it('maintains references to child component instances based on their `ref` property', function () { class ChildComponentA { constructor (props) { - assert(!props.ref, 'Ref property not passed through to component constructor') this.element = document.createElement('div') } - update (props) { - assert(!props.ref, 'Ref property not passed through to component update') - } + update (props) {} } class ChildComponentB { constructor (props) { - assert(!props.ref, 'Ref property not passed through to component constructor') this.element = document.createElement('div') } - update (props) { - assert(!props.ref, 'Ref property not passed through to component update') - } + update (props) {} } const refs = {}