From 10c3b41a1e4a24d4203871df41d8214b52625468 Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Tue, 17 May 2016 15:20:13 +0600 Subject: [PATCH 01/10] [reuse markup] A way to handle pre-rendered HTML (including the option to do checksum comparison) This reverts commit b6317116b90bd570d6117a37edbd7b08532f568c. Add tests & doc for string-eq-based self-healing (i.e. autoFix) Add string-eq-based self-healing fix basic example to adapt new createApp behavoir add test for pre-render HTML event handling Modify setAttribute to accept an option parameter Reconstruct createElement so the side effects are now accessible Implement event handler registration for pre rendered HTML Rebase fix --- docs/api/create-app.md | 9 +++-- docs/tips/pre-rendered.md | 48 ++++++++++++++++++++++ examples/basic/index.js | 5 ++- package.json | 1 + src/app/index.js | 30 +++++++++++--- src/dom/create.js | 84 +++++++++++++++++++++++++++++++-------- src/dom/index.js | 6 ++- src/dom/setAttribute.js | 19 +++++---- src/dom/update.js | 4 +- test/app/index.js | 83 ++++++++++++++++++++++++++++++++------ 10 files changed, 235 insertions(+), 54 deletions(-) create mode 100644 docs/tips/pre-rendered.md diff --git a/docs/api/create-app.md b/docs/api/create-app.md index de449cb..db65be8 100644 --- a/docs/api/create-app.md +++ b/docs/api/create-app.md @@ -34,7 +34,10 @@ render() ### Notes -The container DOM element should: -* **Not be the document.body**. You'll probably run into problems with other libraries. They'll often add elements to the `document.body` which can confuse the diff algorithm. -* **Be empty**. All elements inside of the container will be removed when a virtual element is rendered into it. The renderer needs to have complete control of all of the elements within the container. +* You should **avoid using document.body as the container element**. You'll probably run into problems with other libraries. They'll often add elements to the `document.body` which can confuse the diff algorithm. + +* When the container element is not empty, **deku would assume that the HTML elements inside the container are the pre-rendered elements**. Read [this page](/deku/docs/tips/pre-rendered.md) to learn more about working with pre-rendered elements. + + +. All elements inside of the container will be removed when a virtual element is rendered into it. The renderer needs to have complete control of all of the elements within the container. diff --git a/docs/tips/pre-rendered.md b/docs/tips/pre-rendered.md new file mode 100644 index 0000000..687e1ee --- /dev/null +++ b/docs/tips/pre-rendered.md @@ -0,0 +1,48 @@ +# Pre-rendered HTML Elements + +When the browser requests the HTML file, some of the elements may have been pre-rendered on the server-side. This can be done using deku's [`string.render`](/deku/docs/api/string.html). + + +```html +

pre-rendered text

+``` + +On the client side, if we just create a render function as usual, the first call to the render function would not do anything. This is because deku would assume that the container's pre-rendered content is properly rendered. + +```js +var render = createApp(document.getElementById("container")) +render(

pre-rendered text

) //do nothing +``` + +This means if the virtualDOM describes a different HTML element, deku is not going to fix it for you. + +```js +var render = createApp(document.getElementById("container")) +render(

Meow!

) //do nothing +``` + +Therefore, to be 100% safe, one may want to use the `autoFix` attribute: + +```html +

pre-rendered text

+``` + +In this case, on the first call to the render function, deku would destroy and recreate the elements inside the container, after a quick string-comparison of the pre-rendered and to-be-rendered versions: + +```js +var render = createApp(document.getElementById("container")) +render(

Meow!

) //re-rendered the HTML due to difference in HTML strings +``` + +Alternatively, one can also choose to do a [checksum comparison](https://en.wikipedia.org/wiki/Checksum) between the HTML strings instead. This can be done by getting the server to compute the [Adler32](https://en.wikipedia.org/wiki/Adler-32) checksum value of the string and add it to the container element's `checksum` attribute. + +```html +

pre-rendered text

+``` + +and similar to the using `autoFix` attribute, deku would fix the incorrect pre-rendered HTML on the first call to the render function. + +```js +var render = createApp(document.getElementById("container")) +render(

Meow!

) //re-rendered the HTML due to difference in adler32 checksums +``` diff --git a/examples/basic/index.js b/examples/basic/index.js index 93b16e6..3b4ce20 100644 --- a/examples/basic/index.js +++ b/examples/basic/index.js @@ -1,12 +1,15 @@ import {createApp} from '../../src' import {view, update, init} from './app' +var div = document.createElement("div"); +document.body.appendChild(div); + /** * Create a DOM renderer for vnodes. It accepts a dispatch function as * a second parameter to handle all actions from the UI. */ -let render = createApp(document.body) +let render = createApp(div) /** * Update the UI with the latest state. diff --git a/package.json b/package.json index cf6d88f..284c50b 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "@f/reduce-array": "^0.2.1", "@f/set-attribute": "1.0.1", "@f/to-array": "1.1.1", + "adler-32": "^0.3.0", "dift": "0.1.12", "index-of": "0.2.0", "setify": "1.0.3", diff --git a/src/app/index.js b/src/app/index.js index 33544f3..435da5d 100644 --- a/src/app/index.js +++ b/src/app/index.js @@ -2,6 +2,7 @@ import * as dom from '../dom' import {diffNode} from '../diff' import empty from '@f/empty-element' import noop from '@f/noop' +import {str as adler32} from 'adler-32' /** * Create a DOM renderer using a container element. Everything will be rendered @@ -15,10 +16,6 @@ export function createApp (container, handler = noop, options = {}) { let rootId = options.id || '0' let dispatch = effect => effect && handler(effect) - if (container) { - empty(container) - } - let update = (newVnode, context) => { let changes = diffNode(oldVnode, newVnode, rootId) node = changes.reduce(dom.updateElement(dispatch, context), node) @@ -27,8 +24,29 @@ export function createApp (container, handler = noop, options = {}) { } let create = (vnode, context) => { - node = dom.createElement(vnode, rootId, dispatch, context) - if (container) container.appendChild(node) + if (container){ + if(container.childNodes.length === 0){ + node = dom.createElement(vnode, rootId, dispatch, context) + container.appendChild(node) + }else{ + let {DOMnode, attachEvents} = dom.createElementThenEvents(vnode, rootId, dispatch, context) + let isRenderedCorrectly = true + if(container.attributes.checksum){ + isRenderedCorrectly = container.attributes.checksum === adler32(DOMnode.outerHTML) + }else if(container.attributes.autoFix){ + isRenderedCorrectly = container.innerHTML === DOMnode.outerHTML + } + if(isRenderedCorrectly){ + node = attachEvents(container.firstChild) + }else{ + node = DOMnode + container.innerHTML = '' + container.appendChild(node) + } + } + }else{ + node = dom.createElement(vnode, rootId, dispatch, context) + } oldVnode = vnode return node } diff --git a/src/dom/create.js b/src/dom/create.js index 9c1ef92..8bdb627 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -7,20 +7,62 @@ import isNumber from '@f/is-number' import isNull from '@f/is-null' const cache = {} +export default function createElement(vnode, path, dispatch, context) { + let DOM = createWithSideEffects(vnode, path, dispatch, context) + runSideEffects(DOM.sideEffects) + return DOM.element +} + +export function createElementThenEvents(vnode, path, dispatch, context) { + let DOM = createWithSideEffects(vnode, path, dispatch, context) + runSideEffects(DOM.sideEffects, {noEventListeners: true, }) + return { + DOMnode: DOM.element, + attachEvents: function(PreRenderedElement){ + runSideEffects(DOM.sideEffects, {onlyEventListeners: true}, PreRenderedElement) + } + } +} + +/** +/* Recursively traverse a tree of side effects created by `createWithSideEffects`, and run each side effect. + * Passing a third parameter DOMElement and it will traverse the DOMElement as well. + * + * A tree of side effects either takes in the value `null` or is a JSON object in the form + * {ofParent : P, ofChildren : C} + * where P is either an empty array or an array of functions + * and C is either an empty array or an array of trees of side effects + * + */ + +function runSideEffects(sideEffects, option, DOMElement){ + if(sideEffects){ + sideEffects.ofParent.map(function(sideEffect){ sideEffect(option, DOMElement) }) + sideEffects.ofChildren.map(function(child,index){ + if(DOMElement){ + runSideEffects(child, option, DOMElement.childNodes[index]) + }else{ + runSideEffects(child, option) + } + }) + } +} + /** - * Create a real DOM element from a virtual element, recursively looping down. + * Create a DOM element with a tree of side effects from a virtual element by recursion. * When it finds custom elements it will render them, cache them, and keep going, * so they are treated like any other native element. + * */ -export function createElement (vnode, path, dispatch, context) { +export function createWithSideEffects (vnode, path, dispatch, context) { switch (vnode.type) { case 'text': - return createTextNode(vnode.nodeValue) + return {element: createTextNode(vnode.nodeValue), sideEffects: null} case 'empty': - return getCachedElement('noscript') + return {element: getCachedElement('noscript'), sideEffects: null} case 'thunk': - return createThunk(vnode, path, dispatch, context) + return {element: createThunk(vnode, path, dispatch, context), sideEffects: null} case 'native': return createHTMLElement(vnode, path, dispatch, context) } @@ -36,8 +78,8 @@ function getCachedElement (type) { function createTextNode (text) { let value = isString(text) || isNumber(text) - ? text - : '' + ? text + : '' return document.createTextNode(value) } @@ -63,19 +105,27 @@ function createThunk (vnode, path, dispatch, context) { } function createHTMLElement (vnode, path, dispatch, context) { - let { tagName, attributes, children } = vnode - let DOMElement = getCachedElement(tagName) + let DOMElement = getCachedElement(vnode.tagName) + let sideEffects = {ofParent: [], ofChildren: []} - for (let name in attributes) { - setAttribute(DOMElement, name, attributes[name]) - } + let attributes = Object.keys(vnode.attributes) + sideEffects.ofParent = attributes.map(function(name){ + return function(option, element = DOMElement){ + setAttribute(element, name, vnode.attributes[name], null, option) + } + }) - children.forEach((node, index) => { + vnode.children.forEach((node, index) => { if (isNull(node) || isUndefined(node)) return - let childPath = createPath(path, node.key || index) - let child = createElement(node, childPath, dispatch, context) - DOMElement.appendChild(child) + let DOM = createWithSideEffects( + node, + createPath(path, node.key || index), + dispatch, + context + ) + sideEffects.ofChildren.push(DOM.sideEffects) + DOMElement.appendChild(DOM.element) }) - return DOMElement + return {element: DOMElement, sideEffects: sideEffects} } diff --git a/src/dom/index.js b/src/dom/index.js index fd92b81..25b5400 100644 --- a/src/dom/index.js +++ b/src/dom/index.js @@ -1,7 +1,9 @@ -import {createElement} from './create' -import {updateElement} from './update' +import createElement from './create' +import {createElementThenEvents} from './create' +import updateElement from './update' export { createElement, + createElementThenEvents, updateElement } diff --git a/src/dom/setAttribute.js b/src/dom/setAttribute.js index dfd38a4..7d09829 100644 --- a/src/dom/setAttribute.js +++ b/src/dom/setAttribute.js @@ -28,18 +28,17 @@ export function removeAttribute (DOMElement, name, previousValue) { } } -export function setAttribute (DOMElement, name, value, previousValue) { - let eventType = events[name] - if (value === previousValue) { - return - } - if (eventType) { - if (isFunction(previousValue)) { - DOMElement.removeEventListener(eventType, previousValue) +export function setAttribute (DOMElement, name, value, previousValue, option = {}) { + if (value === previousValue) return + if(!option.noEventListeners){ + let eventType = events[name] + if (eventType) { + if (isFunction(previousValue)) DOMElement.removeEventListener(eventType, previousValue) + DOMElement.addEventListener(eventType, value) + return } - DOMElement.addEventListener(eventType, value) - return } + if(option.onlyEventListeners) return if (!isValidAttribute(value)) { removeAttribute(DOMElement, name, previousValue) return diff --git a/src/dom/update.js b/src/dom/update.js index f3912e9..bd52930 100644 --- a/src/dom/update.js +++ b/src/dom/update.js @@ -2,7 +2,7 @@ import {setAttribute, removeAttribute} from './setAttribute' import {isThunk, createPath} from '../element' import {Actions, diffNode} from '../diff' import reduceArray from '@f/reduce-array' -import {createElement} from './create' +import createElement from './create' import toArray from '@f/to-array' import forEach from '@f/foreach' import noop from '@f/noop' @@ -11,7 +11,7 @@ import noop from '@f/noop' * Modify a DOM element given an array of actions. */ -export function updateElement (dispatch, context) { +export default function updateElement (dispatch, context) { return (DOMElement, action) => { Actions.case({ sameNode: noop, diff --git a/test/app/index.js b/test/app/index.js index 5774ccb..4af2389 100644 --- a/test/app/index.js +++ b/test/app/index.js @@ -3,6 +3,8 @@ import test from 'tape' import {createApp} from '../../src/app' import {create as h} from '../../src/element' +import {str as adler32} from 'adler-32' +import trigger from 'trigger-event' test('rendering elements', t => { let el = document.createElement('div') @@ -66,19 +68,6 @@ test('moving elements using keys', t => { t.end() }) -test('emptying the container', t => { - let el = document.createElement('div') - el.innerHTML = '
' - let render = createApp(el) - render() - t.equal( - el.innerHTML, - '', - 'container emptied' - ) - t.end() -}) - test('context should be passed down all elements', t => { let Form = { render ({ props, context }) { @@ -283,3 +272,71 @@ test('rendering and updating null', t => { t.end() }) + +test('rendering in a container with pre-rendered HTML', t => { + let el = document.createElement('div') + + el.innerHTML = '
' + let render = createApp(el) + render(
) + t.equal( + el.innerHTML, + '
', + 'no string-comparison occurs (nothing should happen)' + ) + + el.attributes.autoFix = ' ' + el.innerHTML = '
Meow
' + render = createApp(el) + render(
Thrr
) + t.equal( + el.innerHTML, + '
Thrr
', + 'destory and re-rendered due to string inequivalence' + ) + + el.innerHTML = '
Cat
' + render(
Neko
) + t.equal( + el.innerHTML, + '
Cat
', + 'nothing should happen because this is not the first call to render' + ) + + el.innerHTML = 'whatever' + el.attributes.checksum = adler32("

pre-rendered text

") + render = createApp(el) + render(

pre-rendered text

) + t.equal( + el.innerHTML, + 'whatever', + 'nothing should happen because checksums are the same' + ) + + el.innerHTML = '
Nyan!
' + el.attributes.checksum = adler32(el.innerHTML) + render = createApp(el) + render(

Nyan!

) + t.equal( + el.innerHTML, + '

Nyan!

', + 'destory and re-rendered due to checksum inequivalence' + ) + t.end() +}) + +test('rendering in a container with pre-rendered HTML and click events', t => { + t.plan(12) + let el = document.createElement('div') + el.innerHTML = '
' + let render = createApp(el) + let a = function(){t.assert("clicked")} + let b = function(){t.assert("clicked"); t.assert("clicked")} + render(
) + let arr = el.querySelectorAll('button, span') + for (var item of arr) { + trigger(item, 'click') + trigger(item, 'click') + } + t.end() +}) From 866f945748e9b59c82f874cb25b752814e9b5e9e Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Wed, 18 May 2016 17:24:02 +0600 Subject: [PATCH 02/10] [reuse markup] Add path-based markup reuse Add failing rerender test Add path-based virtualdom restoration on client-side libs cleanup --- package.json | 2 - src/app/index.js | 22 +++-------- src/dom/create.js | 76 ++++++++++++++++++++++++++++++-------- src/dom/setAttribute.js | 5 ++- src/dom/update.js | 2 +- src/string/renderString.js | 22 +++++++---- test/app/index.js | 75 +++++++++++++++++++++---------------- test/app/thunk.js | 9 +++-- 8 files changed, 134 insertions(+), 79 deletions(-) diff --git a/package.json b/package.json index 284c50b..94cb756 100644 --- a/package.json +++ b/package.json @@ -84,9 +84,7 @@ "@f/reduce-array": "^0.2.1", "@f/set-attribute": "1.0.1", "@f/to-array": "1.1.1", - "adler-32": "^0.3.0", "dift": "0.1.12", - "index-of": "0.2.0", "setify": "1.0.3", "union-type": "0.1.6" }, diff --git a/src/app/index.js b/src/app/index.js index 435da5d..f4c7043 100644 --- a/src/app/index.js +++ b/src/app/index.js @@ -2,7 +2,6 @@ import * as dom from '../dom' import {diffNode} from '../diff' import empty from '@f/empty-element' import noop from '@f/noop' -import {str as adler32} from 'adler-32' /** * Create a DOM renderer using a container element. Everything will be rendered @@ -25,26 +24,15 @@ export function createApp (container, handler = noop, options = {}) { let create = (vnode, context) => { if (container){ - if(container.childNodes.length === 0){ + if (container.childNodes.length === 0) { node = dom.createElement(vnode, rootId, dispatch, context) container.appendChild(node) - }else{ + } else { let {DOMnode, attachEvents} = dom.createElementThenEvents(vnode, rootId, dispatch, context) - let isRenderedCorrectly = true - if(container.attributes.checksum){ - isRenderedCorrectly = container.attributes.checksum === adler32(DOMnode.outerHTML) - }else if(container.attributes.autoFix){ - isRenderedCorrectly = container.innerHTML === DOMnode.outerHTML - } - if(isRenderedCorrectly){ - node = attachEvents(container.firstChild) - }else{ - node = DOMnode - container.innerHTML = '' - container.appendChild(node) - } + node = DOMnode + attachEvents(container.firstChild) } - }else{ + } else { node = dom.createElement(vnode, rootId, dispatch, context) } oldVnode = vnode diff --git a/src/dom/create.js b/src/dom/create.js index 8bdb627..84c1b89 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -60,7 +60,9 @@ export function createWithSideEffects (vnode, path, dispatch, context) { case 'text': return {element: createTextNode(vnode.nodeValue), sideEffects: null} case 'empty': - return {element: getCachedElement('noscript'), sideEffects: null} + const el = getCachedElement('noscript') + el.attributes['__created'] = true + return {element: el, sideEffects: null} case 'thunk': return {element: createThunk(vnode, path, dispatch, context), sideEffects: null} case 'native': @@ -94,8 +96,8 @@ function createThunk (vnode, path, dispatch, context) { context } let output = vnode.fn(model) - let childPath = createPath(path, output.key || '0') - let DOMElement = createElement(output, childPath, dispatch, context) + // let childPath = createPath(path, output.key || '0') + let DOMElement = createElement(output, path, dispatch, context) if (onCreate) dispatch(onCreate(model)) vnode.state = { vnode: output, @@ -105,27 +107,71 @@ function createThunk (vnode, path, dispatch, context) { } function createHTMLElement (vnode, path, dispatch, context) { - let DOMElement = getCachedElement(vnode.tagName) + let DOMElement; let sideEffects = {ofParent: [], ofChildren: []} - let attributes = Object.keys(vnode.attributes) - sideEffects.ofParent = attributes.map(function(name){ - return function(option, element = DOMElement){ - setAttribute(element, name, vnode.attributes[name], null, option) - } - }) + DOMElement = document.getElementById('_id' + path) + if (!DOMElement) { + // no such element on client -> create + DOMElement = getCachedElement(vnode.tagName) + DOMElement.attributes['__created'] = true; // Parent will append this node when this flag is set + } else if (DOMElement.tagName.toLowerCase() != vnode.tagName.toLowerCase()) { + // found element, but with wrong type -> recreate + let newDOMElement = getCachedElement(vnode.tagName) + + Array.prototype.forEach.call(DOMElement.childNodes, function(nodeChild){ + newDOMElement.appendChild(nodeChild); + }); + DOMElement.parentNode.replaceChild(newDOMElement, DOMElement); + DOMElement = newDOMElement; + } + + let textIterator = document.createNodeIterator(DOMElement, NodeFilter.SHOW_TEXT); vnode.children.forEach((node, index) => { if (isNull(node) || isUndefined(node)) return + + if (node.type == 'text') { + let textnode = textIterator.nextNode(); + + if (!textnode || textnode.nodeValue != node.nodeValue) { + let DOM = createWithSideEffects( + node, + createPath(path, node.key || index), + dispatch, + context + ) + sideEffects.ofChildren.push(DOM.sideEffects) + + if (!textnode) { + DOMElement.appendChild(DOM.element); + } else { // node values not equal + DOMElement.replaceChild(DOM.element, textnode) + } + } + + return + } + let DOM = createWithSideEffects( - node, - createPath(path, node.key || index), - dispatch, - context + node, + createPath(path, node.key || index), + dispatch, + context ) sideEffects.ofChildren.push(DOM.sideEffects) - DOMElement.appendChild(DOM.element) + if (DOM.element.attributes && DOM.element.attributes['__created']) { + // newly created html node: add it + DOMElement.appendChild(DOM.element) + } }) + let attributes = Object.keys(vnode.attributes) + sideEffects.ofParent = attributes.map(function(name){ + return function(option, element = DOMElement) { + setAttribute(element, name, vnode.attributes[name], null, option) + } + }) + return {element: DOMElement, sideEffects: sideEffects} } diff --git a/src/dom/setAttribute.js b/src/dom/setAttribute.js index 7d09829..4b15182 100644 --- a/src/dom/setAttribute.js +++ b/src/dom/setAttribute.js @@ -1,7 +1,6 @@ import setNativeAttribute from '@f/set-attribute' import isValidAttribute from '@f/is-valid-attr' import isFunction from '@f/is-function' -import indexOf from 'index-of' import setValue from 'setify' import events from './events' @@ -55,7 +54,9 @@ export function setAttribute (DOMElement, name, value, previousValue, option = { // Fix for IE/Safari where select is not correctly selected on change if (DOMElement.tagName === 'OPTION' && DOMElement.parentNode) { let select = DOMElement.parentNode - select.selectedIndex = indexOf(select.options, DOMElement) + if (select.options.indexOf) { + select.selectedIndex = select.options.indexOf(DOMElement) + } } break case 'value': diff --git a/src/dom/update.js b/src/dom/update.js index bd52930..49880a0 100644 --- a/src/dom/update.js +++ b/src/dom/update.js @@ -88,7 +88,7 @@ function updateThunk (DOMElement, prev, next, path, dispatch, context) { context } let nextNode = next.fn(model) - let changes = diffNode(prevNode, nextNode, createPath(path, '0')) + let changes = diffNode(prevNode, nextNode, path) DOMElement = reduceArray(updateElement(dispatch, context), DOMElement, changes) if (onUpdate) dispatch(onUpdate(model)) next.state = { diff --git a/src/string/renderString.js b/src/string/renderString.js index 684ea89..f7c4625 100644 --- a/src/string/renderString.js +++ b/src/string/renderString.js @@ -22,16 +22,16 @@ function attributesToString (attributes) { * object that will be given to all components. */ -export function renderString (vnode, context, path = '0') { +export function renderString (vnode, context, path = '0', opts = {}) { switch (vnode.type) { case 'text': return renderTextNode(vnode) case 'empty': return renderEmptyNode() case 'thunk': - return renderThunk(vnode, path, context) + return renderThunk(vnode, path, context, opts) case 'native': - return renderHTML(vnode, path, context) + return renderHTML(vnode, path, context, opts) } } @@ -43,21 +43,29 @@ function renderEmptyNode () { return '' } -function renderThunk (vnode, path, context) { +function renderThunk (vnode, path, context, opts) { let { props, children } = vnode let output = vnode.fn({ children, props, path, context }) - return renderString(output, context, path) + return renderString(output, context, path, opts) } -function renderHTML (vnode, path, context) { +function renderHTML (vnode, path, context, opts) { let {attributes, tagName, children} = vnode let innerHTML = attributes.innerHTML + if (opts.appendUids) { + attributes.id = '_id' + path; // for client-side restoration + } let str = '<' + tagName + attributesToString(attributes) + '>' if (innerHTML) { str += innerHTML } else { - str += children.map((child, i) => renderString(child, context, path + '.' + (isNull(child.key) ? i : child.key))).join('') + str += children.map((child, i) => renderString( + child, + context, + path + '.' + (isNull(child.key) ? i : child.key), + opts + )).join('') } str += '' diff --git a/test/app/index.js b/test/app/index.js index 4af2389..bca6561 100644 --- a/test/app/index.js +++ b/test/app/index.js @@ -275,55 +275,68 @@ test('rendering and updating null', t => { test('rendering in a container with pre-rendered HTML', t => { let el = document.createElement('div') + document.body.appendChild(el); - el.innerHTML = '
' + el.innerHTML = '
Meow
' let render = createApp(el) - render(
) + render(
Thrr
) t.equal( el.innerHTML, - '
', - 'no string-comparison occurs (nothing should happen)' + '
Thrr
', + 'inequivalent string updated' ) - el.attributes.autoFix = ' ' - el.innerHTML = '
Meow
' + el.innerHTML = '
Nyan!
' render = createApp(el) - render(
Thrr
) + render(

Nyan!

) t.equal( el.innerHTML, - '
Thrr
', - 'destory and re-rendered due to string inequivalence' + '

Nyan!

', + 're-rendered due to changed tagName' ) + + document.body.removeChild(el); + t.end() +}) + +test('rerendering custom element with changing props', t => { + let el = document.createElement('div') + document.body.appendChild(el); + const Comp = { + render({props, path}) { + return ( + woot + ); + } + } + + let render = createApp(el); + + el.innerHTML = '
woot
'; + el.children[0].attributes.chck = 1; + el.children[0].children[0].attributes.chck = 2; + + render(
); - el.innerHTML = '
Cat
' - render(
Neko
) t.equal( - el.innerHTML, - '
Cat
', - 'nothing should happen because this is not the first call to render' + el.children[0].attributes.chck, + 1, + 'should not rerender outer div' ) - - el.innerHTML = 'whatever' - el.attributes.checksum = adler32("

pre-rendered text

") - render = createApp(el) - render(

pre-rendered text

) t.equal( - el.innerHTML, - 'whatever', - 'nothing should happen because checksums are the same' + el.children[0].children[0].attributes.chck, + 2, + 'should not rerender inner span' ) - - el.innerHTML = '
Nyan!
' - el.attributes.checksum = adler32(el.innerHTML) - render = createApp(el) - render(

Nyan!

) t.equal( el.innerHTML, - '

Nyan!

', - 'destory and re-rendered due to checksum inequivalence' + '
woot
', + 'attributes should be updated when needed' ) - t.end() -}) + + document.body.removeChild(el); + t.end(); +}); test('rendering in a container with pre-rendered HTML and click events', t => { t.plan(12) diff --git a/test/app/thunk.js b/test/app/thunk.js index def05f1..686c3c1 100644 --- a/test/app/thunk.js +++ b/test/app/thunk.js @@ -249,10 +249,10 @@ test('path should stay the same on when thunk is updated', t => { document.body.appendChild(el) let MyButton = { onUpdate ({path}) { - t.equal(path, '0.0.0', 'onUpdate') + t.equal(path, '0.0', 'onUpdate') }, render ({path, children}) { - t.equal(path, '0.0.0', 'onRender') + t.equal(path, '0.0', 'onRender') return } } @@ -283,9 +283,10 @@ test('path should stay the same on when thunk is replaced', t => { } } render(
) - render(
) + render(
) render(
) render(
) - render(
) + // TODO: nested chunk gets same path to minify data - is it acceptable? + render(
) t.end() }) From 21f69d4c293910fe8354efbd71bbbf0e68e9436f Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Thu, 26 May 2016 15:07:29 +0600 Subject: [PATCH 03/10] [reuse markup] Separate side-effects from elements creation --- src/dom/create.js | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/dom/create.js b/src/dom/create.js index 84c1b89..54077e7 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -64,7 +64,7 @@ export function createWithSideEffects (vnode, path, dispatch, context) { el.attributes['__created'] = true return {element: el, sideEffects: null} case 'thunk': - return {element: createThunk(vnode, path, dispatch, context), sideEffects: null} + return createThunk(vnode, path, dispatch, context) case 'native': return createHTMLElement(vnode, path, dispatch, context) } @@ -87,6 +87,8 @@ function createTextNode (text) { function createThunk (vnode, path, dispatch, context) { let { props, children } = vnode + let thunkSideEffects = {ofParent: [], ofChildren: []} + let { onCreate } = vnode.options let model = { children, @@ -96,14 +98,15 @@ function createThunk (vnode, path, dispatch, context) { context } let output = vnode.fn(model) - // let childPath = createPath(path, output.key || '0') - let DOMElement = createElement(output, path, dispatch, context) + let { element, sideEffects } = createWithSideEffects(output, path, dispatch, context) + thunkSideEffects.ofChildren.push(sideEffects) + if (onCreate) dispatch(onCreate(model)) vnode.state = { vnode: output, model: model } - return DOMElement + return {element: element, sideEffects: thunkSideEffects} } function createHTMLElement (vnode, path, dispatch, context) { @@ -127,15 +130,17 @@ function createHTMLElement (vnode, path, dispatch, context) { DOMElement = newDOMElement; } - let textIterator = document.createNodeIterator(DOMElement, NodeFilter.SHOW_TEXT); - vnode.children.forEach((node, index) => { - if (isNull(node) || isUndefined(node)) return + var textIterator = document.createNodeIterator(DOMElement, NodeFilter.SHOW_TEXT); + for (var index in vnode.children) { + var DOM + var node = vnode.children[index] + if (isNull(node) || isUndefined(node)) continue if (node.type == 'text') { - let textnode = textIterator.nextNode(); + var textnode = textIterator.nextNode(); if (!textnode || textnode.nodeValue != node.nodeValue) { - let DOM = createWithSideEffects( + DOM = createWithSideEffects( node, createPath(path, node.key || index), dispatch, @@ -150,10 +155,10 @@ function createHTMLElement (vnode, path, dispatch, context) { } } - return + continue } - let DOM = createWithSideEffects( + DOM = createWithSideEffects( node, createPath(path, node.key || index), dispatch, @@ -164,7 +169,7 @@ function createHTMLElement (vnode, path, dispatch, context) { // newly created html node: add it DOMElement.appendChild(DOM.element) } - }) + } let attributes = Object.keys(vnode.attributes) sideEffects.ofParent = attributes.map(function(name){ From eaa0e46286b8f4558bf0cd945fc7009a7544cfe5 Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Tue, 31 May 2016 13:56:02 +0600 Subject: [PATCH 04/10] [reuse markup] Codestyle fixes & added eslintrc & editorconfig --- .editorconfig | 9 +++++++ .eslintrc | 60 +++++++++++++++++++++++++++++++++++++++++++++++ .gitignore | 1 + src/app/index.js | 1 - src/dom/create.js | 59 +++++++++++++++++++++++----------------------- src/dom/update.js | 6 ++--- test/app/index.js | 1 - 7 files changed, 103 insertions(+), 34 deletions(-) create mode 100644 .editorconfig create mode 100644 .eslintrc diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..c6c8b36 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,9 @@ +root = true + +[*] +indent_style = space +indent_size = 2 +end_of_line = lf +charset = utf-8 +trim_trailing_whitespace = true +insert_final_newline = true diff --git a/.eslintrc b/.eslintrc new file mode 100644 index 0000000..bdbb22b --- /dev/null +++ b/.eslintrc @@ -0,0 +1,60 @@ +{ + "parser": "babel-eslint", + "extends": "eslint:recommended", + "plugins": [ + "react" + ], + "env": { + "browser": true, + "es6": true + }, + "parserOptions": { + "ecmaFeatures": { + "modules": true, + "jsx": true + } + }, + "globals": { + "expect": true + }, + "rules": { + "react/jsx-uses-react": 2, + "react/jsx-uses-vars": 2, + "no-unused-vars": 2, + "no-cond-assign": 2, + "no-empty": 2, + "no-console": 2, + "semi": [2, "never"], + "camelcase": 2, + "comma-style": 2, + "comma-dangle": [2, "never"], + "no-mixed-spaces-and-tabs": 2, + "no-trailing-spaces": [2, { "skipBlankLines": true }], + "no-eval": 2, + "no-implied-eval": 2, + "no-new-func": 2, + "eqeqeq": 0, + "no-else-return": 2, + "no-redeclare": 2, + "no-dupe-keys": 2, + "radix": 2, + "strict": [2, "never"], + "no-shadow": 0, + "callback-return": [1, ["callback", "cb", "next", "done"]], + "no-delete-var": 2, + "no-undef-init": 2, + "no-shadow-restricted-names": 2, + "handle-callback-err": 0, + "no-lonely-if": 2, + "keyword-spacing": 2, + "constructor-super": 2, + "no-this-before-super": 2, + "no-dupe-class-members": 2, + "no-const-assign": 2, + "prefer-spread": 2, + "no-useless-concat": 2, + "no-var": 2, + "object-shorthand": 2, + "prefer-arrow-callback": 2 + } +} diff --git a/.gitignore b/.gitignore index b950123..84bf96f 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ dist lib es npm-debug.log +.idea/ diff --git a/src/app/index.js b/src/app/index.js index f4c7043..de4861f 100644 --- a/src/app/index.js +++ b/src/app/index.js @@ -1,6 +1,5 @@ import * as dom from '../dom' import {diffNode} from '../diff' -import empty from '@f/empty-element' import noop from '@f/noop' /** diff --git a/src/dom/create.js b/src/dom/create.js index 54077e7..6615f15 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -1,5 +1,5 @@ import createNativeElement from '@f/create-element' -import {createPath} from '../element' +import {createPath} from '../element/index' import {setAttribute} from './setAttribute' import isUndefined from '@f/is-undefined' import isString from '@f/is-string' @@ -15,10 +15,10 @@ export default function createElement(vnode, path, dispatch, context) { export function createElementThenEvents(vnode, path, dispatch, context) { let DOM = createWithSideEffects(vnode, path, dispatch, context) - runSideEffects(DOM.sideEffects, {noEventListeners: true, }) + runSideEffects(DOM.sideEffects, {noEventListeners: true}) return { DOMnode: DOM.element, - attachEvents: function(PreRenderedElement){ + attachEvents(PreRenderedElement){ runSideEffects(DOM.sideEffects, {onlyEventListeners: true}, PreRenderedElement) } } @@ -36,12 +36,12 @@ export function createElementThenEvents(vnode, path, dispatch, context) { */ function runSideEffects(sideEffects, option, DOMElement){ - if(sideEffects){ - sideEffects.ofParent.map(function(sideEffect){ sideEffect(option, DOMElement) }) - sideEffects.ofChildren.map(function(child,index){ - if(DOMElement){ + if (sideEffects) { + sideEffects.ofParent.map((sideEffect) => { sideEffect(option, DOMElement) }) + sideEffects.ofChildren.map((child,index) => { + if (DOMElement) { runSideEffects(child, option, DOMElement.childNodes[index]) - }else{ + } else { runSideEffects(child, option) } }) @@ -56,11 +56,12 @@ function runSideEffects(sideEffects, option, DOMElement){ */ export function createWithSideEffects (vnode, path, dispatch, context) { + let el switch (vnode.type) { case 'text': return {element: createTextNode(vnode.nodeValue), sideEffects: null} case 'empty': - const el = getCachedElement('noscript') + el = getCachedElement('noscript') el.attributes['__created'] = true return {element: el, sideEffects: null} case 'thunk': @@ -87,7 +88,7 @@ function createTextNode (text) { function createThunk (vnode, path, dispatch, context) { let { props, children } = vnode - let thunkSideEffects = {ofParent: [], ofChildren: []} + let sideEffects = {ofParent: [], ofChildren: []} let { onCreate } = vnode.options let model = { @@ -98,46 +99,46 @@ function createThunk (vnode, path, dispatch, context) { context } let output = vnode.fn(model) - let { element, sideEffects } = createWithSideEffects(output, path, dispatch, context) - thunkSideEffects.ofChildren.push(sideEffects) + let { element, effects } = createWithSideEffects(output, path, dispatch, context) + sideEffects.ofChildren.push(effects) if (onCreate) dispatch(onCreate(model)) vnode.state = { vnode: output, - model: model + model } - return {element: element, sideEffects: thunkSideEffects} + return {element, sideEffects} } function createHTMLElement (vnode, path, dispatch, context) { - let DOMElement; + let DOMElement let sideEffects = {ofParent: [], ofChildren: []} DOMElement = document.getElementById('_id' + path) if (!DOMElement) { // no such element on client -> create DOMElement = getCachedElement(vnode.tagName) - DOMElement.attributes['__created'] = true; // Parent will append this node when this flag is set + DOMElement.attributes['__created'] = true // Parent will append this node when this flag is set } else if (DOMElement.tagName.toLowerCase() != vnode.tagName.toLowerCase()) { // found element, but with wrong type -> recreate let newDOMElement = getCachedElement(vnode.tagName) - Array.prototype.forEach.call(DOMElement.childNodes, function(nodeChild){ - newDOMElement.appendChild(nodeChild); - }); + Array.prototype.forEach.call(DOMElement.childNodes, (nodeChild) => { + newDOMElement.appendChild(nodeChild) + }) - DOMElement.parentNode.replaceChild(newDOMElement, DOMElement); - DOMElement = newDOMElement; + DOMElement.parentNode.replaceChild(newDOMElement, DOMElement) + DOMElement = newDOMElement } - var textIterator = document.createNodeIterator(DOMElement, NodeFilter.SHOW_TEXT); - for (var index in vnode.children) { - var DOM - var node = vnode.children[index] + const textIterator = document.createNodeIterator(DOMElement, NodeFilter.SHOW_TEXT) + for (let index in vnode.children) { + let DOM + const node = vnode.children[index] if (isNull(node) || isUndefined(node)) continue if (node.type == 'text') { - var textnode = textIterator.nextNode(); + const textnode = textIterator.nextNode() if (!textnode || textnode.nodeValue != node.nodeValue) { DOM = createWithSideEffects( @@ -149,7 +150,7 @@ function createHTMLElement (vnode, path, dispatch, context) { sideEffects.ofChildren.push(DOM.sideEffects) if (!textnode) { - DOMElement.appendChild(DOM.element); + DOMElement.appendChild(DOM.element) } else { // node values not equal DOMElement.replaceChild(DOM.element, textnode) } @@ -172,11 +173,11 @@ function createHTMLElement (vnode, path, dispatch, context) { } let attributes = Object.keys(vnode.attributes) - sideEffects.ofParent = attributes.map(function(name){ + sideEffects.ofParent = attributes.map((name) => { return function(option, element = DOMElement) { setAttribute(element, name, vnode.attributes[name], null, option) } }) - return {element: DOMElement, sideEffects: sideEffects} + return {element: DOMElement, sideEffects} } diff --git a/src/dom/update.js b/src/dom/update.js index 49880a0..d695468 100644 --- a/src/dom/update.js +++ b/src/dom/update.js @@ -1,5 +1,5 @@ import {setAttribute, removeAttribute} from './setAttribute' -import {isThunk, createPath} from '../element' +import {isThunk} from '../element' import {Actions, diffNode} from '../diff' import reduceArray from '@f/reduce-array' import createElement from './create' @@ -93,7 +93,7 @@ function updateThunk (DOMElement, prev, next, path, dispatch, context) { if (onUpdate) dispatch(onUpdate(model)) next.state = { vnode: nextNode, - model: model + model } return DOMElement } @@ -119,7 +119,7 @@ function removeThunks (vnode, dispatch) { */ export let insertAtIndex = (parent, index, el) => { - var target = parent.childNodes[index] + let target = parent.childNodes[index] if (target) { parent.insertBefore(el, target) } else { diff --git a/test/app/index.js b/test/app/index.js index bca6561..e7dd89c 100644 --- a/test/app/index.js +++ b/test/app/index.js @@ -3,7 +3,6 @@ import test from 'tape' import {createApp} from '../../src/app' import {create as h} from '../../src/element' -import {str as adler32} from 'adler-32' import trigger from 'trigger-event' test('rendering elements', t => { From d0771f533f33ed7076371126ed38efb6334ad0d5 Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Tue, 31 May 2016 17:19:21 +0600 Subject: [PATCH 05/10] [reuse markup] Rewrite and refactor markup reuse code get rid of uids on server Remove generator to avoid regenerator runtime --- src/app/index.js | 39 +++++---- src/dom/create.js | 163 ++++++++++++++++++++----------------- src/string/renderString.js | 3 - test/app/index.js | 46 ++++++----- 4 files changed, 139 insertions(+), 112 deletions(-) diff --git a/src/app/index.js b/src/app/index.js index de4861f..797e291 100644 --- a/src/app/index.js +++ b/src/app/index.js @@ -1,11 +1,12 @@ import * as dom from '../dom' import {diffNode} from '../diff' import noop from '@f/noop' +import empty from '@f/empty-element' /** * Create a DOM renderer using a container element. Everything will be rendered - * inside of that container. Returns a function that accepts new state that can - * replace what is currently rendered. + * inside of that container. Can reuse markup inside the container (if any). + * Returns a function that accepts new state that can replace what is currently rendered. */ export function createApp (container, handler = noop, options = {}) { @@ -14,6 +15,11 @@ export function createApp (container, handler = noop, options = {}) { let rootId = options.id || '0' let dispatch = effect => effect && handler(effect) + if (typeof handler != 'function' && typeof handler == 'object') { + options = handler; + handler = noop; + } + let update = (newVnode, context) => { let changes = diffNode(oldVnode, newVnode, rootId) node = changes.reduce(dom.updateElement(dispatch, context), node) @@ -22,25 +28,30 @@ export function createApp (container, handler = noop, options = {}) { } let create = (vnode, context) => { - if (container){ - if (container.childNodes.length === 0) { - node = dom.createElement(vnode, rootId, dispatch, context) - container.appendChild(node) - } else { - let {DOMnode, attachEvents} = dom.createElementThenEvents(vnode, rootId, dispatch, context) - node = DOMnode - attachEvents(container.firstChild) - } - } else { - node = dom.createElement(vnode, rootId, dispatch, context) + node = dom.createElement(vnode, rootId, dispatch, context) + if (container) { + empty(container) + container.appendChild(node) } oldVnode = vnode return node } + let createWithReuse = (vnode, context) => { + let {DOMnode, attachEvents} = dom.createElementThenEvents(vnode, rootId, dispatch, context, container.firstChild) + node = DOMnode + attachEvents(container.firstChild) + oldVnode = vnode + return node + } + return (vnode, context = {}) => { return node !== null ? update(vnode, context) - : create(vnode, context) + : ( + !container || container.childNodes.length === 0 || !options.reuseMarkup + ? create(vnode, context) + : createWithReuse(vnode, context) + ) } } diff --git a/src/dom/create.js b/src/dom/create.js index 6615f15..49492bc 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -7,18 +7,23 @@ import isNumber from '@f/is-number' import isNull from '@f/is-null' const cache = {} +// node manipulation action types +const CREATE = 'create' +const REPLACE = 'replace' +const NOOP = 'noop' + export default function createElement(vnode, path, dispatch, context) { - let DOM = createWithSideEffects(vnode, path, dispatch, context) + let DOM = createWithSideEffects(vnode, path, dispatch, context) runSideEffects(DOM.sideEffects) return DOM.element } -export function createElementThenEvents(vnode, path, dispatch, context) { - let DOM = createWithSideEffects(vnode, path, dispatch, context) +export function createElementThenEvents(vnode, path, dispatch, context, container = null) { + let DOM = createWithSideEffects(vnode, path, dispatch, context, {originNode: container}) runSideEffects(DOM.sideEffects, {noEventListeners: true}) return { DOMnode: DOM.element, - attachEvents(PreRenderedElement){ + attachEvents(PreRenderedElement) { runSideEffects(DOM.sideEffects, {onlyEventListeners: true}, PreRenderedElement) } } @@ -32,10 +37,8 @@ export function createElementThenEvents(vnode, path, dispatch, context) { * {ofParent : P, ofChildren : C} * where P is either an empty array or an array of functions * and C is either an empty array or an array of trees of side effects - * */ - -function runSideEffects(sideEffects, option, DOMElement){ +function runSideEffects(sideEffects, option, DOMElement) { if (sideEffects) { sideEffects.ofParent.map((sideEffect) => { sideEffect(option, DOMElement) }) sideEffects.ofChildren.map((child,index) => { @@ -52,22 +55,17 @@ function runSideEffects(sideEffects, option, DOMElement){ * Create a DOM element with a tree of side effects from a virtual element by recursion. * When it finds custom elements it will render them, cache them, and keep going, * so they are treated like any other native element. - * */ - -export function createWithSideEffects (vnode, path, dispatch, context) { - let el +export function createWithSideEffects (vnode, path, dispatch, context, options = {}) { switch (vnode.type) { case 'text': - return {element: createTextNode(vnode.nodeValue), sideEffects: null} + return createTextNode(vnode, options) case 'empty': - el = getCachedElement('noscript') - el.attributes['__created'] = true - return {element: el, sideEffects: null} + return {element: getCachedElement('noscript'), sideEffects: null, action: CREATE} case 'thunk': - return createThunk(vnode, path, dispatch, context) + return createThunk(vnode, path, dispatch, context, options) case 'native': - return createHTMLElement(vnode, path, dispatch, context) + return createHTMLElement(vnode, path, dispatch, context, options) } } @@ -79,16 +77,30 @@ function getCachedElement (type) { return cached.cloneNode(false) } -function createTextNode (text) { - let value = isString(text) || isNumber(text) - ? text +function createTextNode ({nodeValue}, {originNode}) { + let value = isString(nodeValue) || isNumber(nodeValue) + ? nodeValue : '' - return document.createTextNode(value) + + // Determine action to perform after creating a text node + let action = CREATE + if (originNode && originNode.nodeValue) { + action = NOOP + if (originNode.nodeValue != value) { + action = REPLACE + } + } + + return { + element: action != NOOP ? document.createTextNode(value) : null, + sideEffects: null, + action + } } -function createThunk (vnode, path, dispatch, context) { +function createThunk (vnode, path, dispatch, context, options) { let { props, children } = vnode - let sideEffects = {ofParent: [], ofChildren: []} + let effects = {ofParent: [], ofChildren: []} let { onCreate } = vnode.options let model = { @@ -99,85 +111,86 @@ function createThunk (vnode, path, dispatch, context) { context } let output = vnode.fn(model) - let { element, effects } = createWithSideEffects(output, path, dispatch, context) - sideEffects.ofChildren.push(effects) + let { element, sideEffects, action } = createWithSideEffects(output, path, dispatch, context, options) + effects.ofChildren.push(sideEffects) if (onCreate) dispatch(onCreate(model)) vnode.state = { vnode: output, model } - return {element, sideEffects} + return {element, sideEffects: effects, action} } -function createHTMLElement (vnode, path, dispatch, context) { +function createHTMLElement (vnode, path, dispatch, context, {originNode}) { let DOMElement let sideEffects = {ofParent: [], ofChildren: []} + let action = NOOP - DOMElement = document.getElementById('_id' + path) - if (!DOMElement) { - // no such element on client -> create + if (!originNode) { // no such element in markup -> create & append DOMElement = getCachedElement(vnode.tagName) - DOMElement.attributes['__created'] = true // Parent will append this node when this flag is set - } else if (DOMElement.tagName.toLowerCase() != vnode.tagName.toLowerCase()) { - // found element, but with wrong type -> recreate + action = CREATE + } else if (!originNode.tagName || originNode.tagName.toLowerCase() != vnode.tagName.toLowerCase()) { + // found element, but with wrong type -> recreate & replace let newDOMElement = getCachedElement(vnode.tagName) - - Array.prototype.forEach.call(DOMElement.childNodes, (nodeChild) => { + Array.prototype.forEach.call(originNode.childNodes, (nodeChild) => { newDOMElement.appendChild(nodeChild) }) - DOMElement.parentNode.replaceChild(newDOMElement, DOMElement) DOMElement = newDOMElement + action = REPLACE + } else { // found node and type matches -> reuse + DOMElement = originNode } - const textIterator = document.createNodeIterator(DOMElement, NodeFilter.SHOW_TEXT) - for (let index in vnode.children) { - let DOM - const node = vnode.children[index] - if (isNull(node) || isUndefined(node)) continue - - if (node.type == 'text') { - const textnode = textIterator.nextNode() - - if (!textnode || textnode.nodeValue != node.nodeValue) { - DOM = createWithSideEffects( - node, - createPath(path, node.key || index), - dispatch, - context - ) - sideEffects.ofChildren.push(DOM.sideEffects) - - if (!textnode) { - DOMElement.appendChild(DOM.element) - } else { // node values not equal - DOMElement.replaceChild(DOM.element, textnode) - } - } + // traverse and [re]create child nodes if needed + let updateChild = nodesUpdater(DOMElement, path, dispatch, context) + vnode.children.forEach((node, index) => updateChild(node, index, sideEffects)) - continue + let attributes = Object.keys(vnode.attributes) + sideEffects.ofParent = attributes.map((name) => { + return function(option, element = DOMElement) { + setAttribute(element, name, vnode.attributes[name], null, option) } + }) + + return {element: DOMElement, sideEffects, action} +} + +/** + * Nodes updater factory: creates vnode for every found real DOM node. + * + * Hardly relies on order of child nodes, so it may cause big overhead even + * on minor differences, especially when difference occurs close to the + * beginning of the document. + * + * @returns {Function} + */ +function nodesUpdater(parentNode, path, dispatch, context) { + let i = 0 + + return (node, index, sideEffects) => { + if (isNull(node) || isUndefined(node)) return - DOM = createWithSideEffects( + const originNode = parentNode.childNodes[i++] || null + const DOM = createWithSideEffects( node, createPath(path, node.key || index), dispatch, - context + context, + {originNode} ) + sideEffects.ofChildren.push(DOM.sideEffects) - if (DOM.element.attributes && DOM.element.attributes['__created']) { - // newly created html node: add it - DOMElement.appendChild(DOM.element) - } - } - let attributes = Object.keys(vnode.attributes) - sideEffects.ofParent = attributes.map((name) => { - return function(option, element = DOMElement) { - setAttribute(element, name, vnode.attributes[name], null, option) + switch (DOM.action) { + case CREATE: + parentNode.appendChild(DOM.element) + break + case REPLACE: + parentNode.replaceChild(DOM.element, originNode) + break + default: } - }) - - return {element: DOMElement, sideEffects} + } } diff --git a/src/string/renderString.js b/src/string/renderString.js index f7c4625..03165ab 100644 --- a/src/string/renderString.js +++ b/src/string/renderString.js @@ -52,9 +52,6 @@ function renderThunk (vnode, path, context, opts) { function renderHTML (vnode, path, context, opts) { let {attributes, tagName, children} = vnode let innerHTML = attributes.innerHTML - if (opts.appendUids) { - attributes.id = '_id' + path; // for client-side restoration - } let str = '<' + tagName + attributesToString(attributes) + '>' if (innerHTML) { diff --git a/test/app/index.js b/test/app/index.js index e7dd89c..768001b 100644 --- a/test/app/index.js +++ b/test/app/index.js @@ -274,48 +274,49 @@ test('rendering and updating null', t => { test('rendering in a container with pre-rendered HTML', t => { let el = document.createElement('div') - document.body.appendChild(el); + document.body.appendChild(el) - el.innerHTML = '
Meow
' + el.innerHTML = '
Meow
' let render = createApp(el) render(
Thrr
) t.equal( el.innerHTML, - '
Thrr
', + '
Thrr
', 'inequivalent string updated' ) - el.innerHTML = '
Nyan!
' + // Should work fine for all except root element + el.innerHTML = '
Nyan!
' render = createApp(el) - render(

Nyan!

) + render(

Nyan!

) t.equal( el.innerHTML, - '

Nyan!

', + '

Nyan!

', 're-rendered due to changed tagName' ) - document.body.removeChild(el); + document.body.removeChild(el) t.end() }) test('rerendering custom element with changing props', t => { let el = document.createElement('div') - document.body.appendChild(el); + document.body.appendChild(el) const Comp = { render({props, path}) { return ( woot - ); + ) } } - let render = createApp(el); + let render = createApp(el, { reuseMarkup: true }) - el.innerHTML = '
woot
'; - el.children[0].attributes.chck = 1; - el.children[0].children[0].attributes.chck = 2; + el.innerHTML = '
woot
' + el.children[0].attributes.chck = 1 + el.children[0].children[0].attributes.chck = 2 - render(
); + render(
) t.equal( el.children[0].attributes.chck, @@ -329,21 +330,26 @@ test('rerendering custom element with changing props', t => { ) t.equal( el.innerHTML, - '
woot
', + '
woot
', 'attributes should be updated when needed' ) - document.body.removeChild(el); - t.end(); -}); + document.body.removeChild(el) + t.end() +}) test('rendering in a container with pre-rendered HTML and click events', t => { t.plan(12) let el = document.createElement('div') el.innerHTML = '
' let render = createApp(el) - let a = function(){t.assert("clicked")} - let b = function(){t.assert("clicked"); t.assert("clicked")} + let a = () => { + t.assert("clicked") + } + let b = () => { + t.assert("clicked") + t.assert("clicked") + } render(
) let arr = el.querySelectorAll('button, span') for (var item of arr) { From 7c7dfddd56e4610233d0bec92eb6bcecc3d182f7 Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Wed, 1 Jun 2016 15:20:53 +0600 Subject: [PATCH 06/10] Added DOM nodes pooling/recycling --- src/app/index.js | 6 ++++ src/dom/create.js | 29 +++++++++--------- src/dom/index.js | 5 +-- src/dom/pool.js | 78 +++++++++++++++++++++++++++++++++++++++++++++++ src/dom/update.js | 8 ++--- test/dom/pool.js | 47 ++++++++++++++++++++++++++++ test/index.js | 1 + 7 files changed, 154 insertions(+), 20 deletions(-) create mode 100644 src/dom/pool.js create mode 100644 test/dom/pool.js diff --git a/src/app/index.js b/src/app/index.js index 797e291..0fee92c 100644 --- a/src/app/index.js +++ b/src/app/index.js @@ -7,6 +7,11 @@ import empty from '@f/empty-element' * Create a DOM renderer using a container element. Everything will be rendered * inside of that container. Can reuse markup inside the container (if any). * Returns a function that accepts new state that can replace what is currently rendered. + * + * Options: + * - reuseMarkup (bool): try to reuse already rendered markup inside the container. + * This might be useful for isomorphic apps. + * - enableNodeRecycling (bool): try to reuse existing DOM nodes to minimize DOM GC */ export function createApp (container, handler = noop, options = {}) { @@ -19,6 +24,7 @@ export function createApp (container, handler = noop, options = {}) { options = handler; handler = noop; } + dom.enableNodeRecycling(options.enableNodeRecycling || false) let update = (newVnode, context) => { let changes = diffNode(oldVnode, newVnode, rootId) diff --git a/src/dom/create.js b/src/dom/create.js index 49492bc..315df5b 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -1,11 +1,12 @@ -import createNativeElement from '@f/create-element' import {createPath} from '../element/index' import {setAttribute} from './setAttribute' import isUndefined from '@f/is-undefined' import isString from '@f/is-string' import isNumber from '@f/is-number' import isNull from '@f/is-null' -const cache = {} +import Pool from './pool' + +const cache = new Pool() // node manipulation action types const CREATE = 'create' @@ -29,6 +30,14 @@ export function createElementThenEvents(vnode, path, dispatch, context, containe } } +export function enableNodeRecycling(flag) { + cache.enableRecycling(flag) +} + +export function storeInCache(node) { + cache.store(node) +} + /** /* Recursively traverse a tree of side effects created by `createWithSideEffects`, and run each side effect. * Passing a third parameter DOMElement and it will traverse the DOMElement as well. @@ -61,7 +70,7 @@ export function createWithSideEffects (vnode, path, dispatch, context, options = case 'text': return createTextNode(vnode, options) case 'empty': - return {element: getCachedElement('noscript'), sideEffects: null, action: CREATE} + return {element: cache.get('noscript'), sideEffects: null, action: CREATE} case 'thunk': return createThunk(vnode, path, dispatch, context, options) case 'native': @@ -69,14 +78,6 @@ export function createWithSideEffects (vnode, path, dispatch, context, options = } } -function getCachedElement (type) { - let cached = cache[type] - if (isUndefined(cached)) { - cached = cache[type] = createNativeElement(type) - } - return cached.cloneNode(false) -} - function createTextNode ({nodeValue}, {originNode}) { let value = isString(nodeValue) || isNumber(nodeValue) ? nodeValue @@ -128,11 +129,11 @@ function createHTMLElement (vnode, path, dispatch, context, {originNode}) { let action = NOOP if (!originNode) { // no such element in markup -> create & append - DOMElement = getCachedElement(vnode.tagName) + DOMElement = cache.get(vnode.tagName) action = CREATE } else if (!originNode.tagName || originNode.tagName.toLowerCase() != vnode.tagName.toLowerCase()) { // found element, but with wrong type -> recreate & replace - let newDOMElement = getCachedElement(vnode.tagName) + let newDOMElement = cache.get(vnode.tagName) Array.prototype.forEach.call(originNode.childNodes, (nodeChild) => { newDOMElement.appendChild(nodeChild) }) @@ -188,7 +189,7 @@ function nodesUpdater(parentNode, path, dispatch, context) { parentNode.appendChild(DOM.element) break case REPLACE: - parentNode.replaceChild(DOM.element, originNode) + cache.store(parentNode.replaceChild(DOM.element, originNode)) break default: } diff --git a/src/dom/index.js b/src/dom/index.js index 25b5400..fb0d9ba 100644 --- a/src/dom/index.js +++ b/src/dom/index.js @@ -1,9 +1,10 @@ import createElement from './create' -import {createElementThenEvents} from './create' +import {createElementThenEvents, enableNodeRecycling} from './create' import updateElement from './update' export { createElement, createElementThenEvents, - updateElement + updateElement, + enableNodeRecycling } diff --git a/src/dom/pool.js b/src/dom/pool.js new file mode 100644 index 0000000..1e294e1 --- /dev/null +++ b/src/dom/pool.js @@ -0,0 +1,78 @@ +import createNativeElement from '@f/create-element' + +export default class Pool { + constructor() { + this.storage = {} + this._recyclingEnabled = false + } + + store(el) { + if (!this._recyclingEnabled) { + return + } + + if (el && el.parentNode) { + el.parentNode.removeChild(el) + } + + if (!el.nodeType || el.nodeType != Node.ELEMENT_NODE) { + return + } + + let tagName = el.tagName.toLowerCase() + if (!this.storage[tagName]) { + this.storage[tagName] = [] + } + + // little cleanup + for (let i = 0; i < el.attributes.length; i++) { + el.removeAttribute(el.attributes[i].name) + } + + if (el.childNodes.length > 0) { + for (let i = 0; i < el.childNodes.length; i++) { + this.store(el.childNodes[i]) + } + } + + this.storage[tagName].push(el) + } + + enableRecycling(flag) { + this._recyclingEnabled = flag + } + + get(tagName) { + tagName = tagName.toLowerCase() + if (this._recyclingEnabled && this.storage[tagName] && this.storage[tagName].length > 0) { + return this.storage[tagName].pop() + } + return createNativeElement(tagName) + } + + preallocate(tagName, size) { + if (!this._recyclingEnabled) { + return + } + + tagName = tagName.toLowerCase() + if (this.storage[tagName] && this.storage[tagName].length >= size) { + return + } + + if (!this.storage[tagName]) { + this.storage[tagName] = [] + } + + let difference = size - this.storage[tagName].length + for (let i = 0; i < difference; i++) { + this.store(createNativeElement(tagName), false) + } + } + + // for tests only + _getStorageSizeFor(tagName) { + return (this.storage[tagName] || []).length + } +} + diff --git a/src/dom/update.js b/src/dom/update.js index d695468..9034460 100644 --- a/src/dom/update.js +++ b/src/dom/update.js @@ -2,7 +2,7 @@ import {setAttribute, removeAttribute} from './setAttribute' import {isThunk} from '../element' import {Actions, diffNode} from '../diff' import reduceArray from '@f/reduce-array' -import createElement from './create' +import createElement, {storeInCache} from './create' import toArray from '@f/to-array' import forEach from '@f/foreach' import noop from '@f/noop' @@ -33,13 +33,13 @@ export default function updateElement (dispatch, context) { replaceNode: (prev, next, path) => { let newEl = createElement(next, path, dispatch, context) let parentEl = DOMElement.parentNode - if (parentEl) parentEl.replaceChild(newEl, DOMElement) + if (parentEl) storeInCache(parentEl.replaceChild(newEl, DOMElement)) DOMElement = newEl removeThunks(prev, dispatch) }, removeNode: (prev) => { removeThunks(prev) - DOMElement.parentNode.removeChild(DOMElement) + storeInCache(DOMElement.parentNode.removeChild(DOMElement)) DOMElement = null } }, action) @@ -62,7 +62,7 @@ function updateChildren (DOMElement, changes, dispatch, context) { insertAtIndex(DOMElement, index, createElement(vnode, path, dispatch, context)) }, removeChild: (index) => { - DOMElement.removeChild(childNodes[index]) + storeInCache(DOMElement.removeChild(childNodes[index])) }, updateChild: (index, actions) => { let _update = updateElement(dispatch, context) diff --git a/test/dom/pool.js b/test/dom/pool.js new file mode 100644 index 0000000..18c9c42 --- /dev/null +++ b/test/dom/pool.js @@ -0,0 +1,47 @@ +import test from 'tape' +import Pool from '../../src/dom/pool' + +test('storeDomNode', t => { + let node = document.createElement('div') + node.__attr = true + let pool = new Pool() + pool.enableRecycling(true) + + pool.store(node) + + let storedNode = pool.get('div') + + t.equal(storedNode.__attr, true, 'Pool returned same node') + t.end() +}) + +test('getNewDomNode', t => { + let pool = new Pool() + let newNode = pool.get('div') + t.equal(newNode.tagName.toLowerCase(), 'div', 'Pool created and returned div') + + pool.enableRecycling(true) + newNode = pool.get('div') + t.equal(newNode.tagName.toLowerCase(), 'div', 'Pool created and returned div with enabled recycling') + t.end() +}) + +test('getNewSvgNode', t => { + let pool = new Pool() + let newNode = pool.get('circle') + + t.equal(newNode instanceof SVGElement, true, 'Pool created svg element') + t.end() +}) + +test('preallocateDomNodes', t => { + let pool = new Pool() + pool.enableRecycling(true) + pool.preallocate('div', 20) + + t.equal(pool._getStorageSizeFor('div'), 20, 'Preallocated 20 elements') + t.equal(pool.get('div').tagName.toLowerCase(), 'div', 'Stored element was div') + t.equal(pool._getStorageSizeFor('div'), 19, '19 elements left after .get()') + t.end() +}) + diff --git a/test/index.js b/test/index.js index 2a0b29c..bbab0d6 100644 --- a/test/index.js +++ b/test/index.js @@ -3,6 +3,7 @@ import './element/index' import './dom/setAttribute' import './dom/create' import './dom/update' +import './dom/pool' import './diff/attributes' import './diff/node' import './diff/children' From 2a7ea3cd2ebed82e0cf6174952acf21e53790263 Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Wed, 1 Jun 2016 17:46:51 +0600 Subject: [PATCH 07/10] Update docs --- docs/api/create-app.md | 10 ++++++---- docs/tips/pre-rendered.md | 37 +++++++++---------------------------- 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/docs/api/create-app.md b/docs/api/create-app.md index db65be8..5b2cb19 100644 --- a/docs/api/create-app.md +++ b/docs/api/create-app.md @@ -5,7 +5,10 @@ Returns a `render` function that you can use to render elements within `DOMEleme ### Arguments 1. `el` _(HTMLElement)_: A container element that will have virtual elements rendered inside of it. The element will never be touched. -2. `dispatch` _(Function)_: A function that can receive actions from the interface. This function will be passed into every component. It usually takes an [action](http://redux.js.org/docs/basics/Actions.html) that can be handled by a [store](http://redux.js.org/docs/basics/Store.html) +2. `dispatch` _(Function)_ [optional]: A function that can receive actions from the interface. This function will be passed into every component. It usually takes an [action](http://redux.js.org/docs/basics/Actions.html) that can be handled by a [store](http://redux.js.org/docs/basics/Store.html) +3. `options` _(Object)_ [optional]: A plain object that contains renderer options: + - `reuseMarkup: true` will cause deku to reuse container contents when possible. This may be useful for isomorphic apps. + - `enableNodeRecycling: true` will enable pooling/recycling of existing DOM nodes. This should lead to reduced GC and memory usage. ### Returns @@ -37,7 +40,6 @@ render() * You should **avoid using document.body as the container element**. You'll probably run into problems with other libraries. They'll often add elements to the `document.body` which can confuse the diff algorithm. -* When the container element is not empty, **deku would assume that the HTML elements inside the container are the pre-rendered elements**. Read [this page](/deku/docs/tips/pre-rendered.md) to learn more about working with pre-rendered elements. +* When the container element is not empty, **deku may try to reuse container contents**. Read [this page](/deku/docs/tips/pre-rendered.md) to learn more about working with pre-rendered elements. - -. All elements inside of the container will be removed when a virtual element is rendered into it. The renderer needs to have complete control of all of the elements within the container. +. All elements inside of the container will be removed when a virtual element is rendered into it, unless markup reuse option is on. The renderer needs to have complete control of all of the elements within the container. diff --git a/docs/tips/pre-rendered.md b/docs/tips/pre-rendered.md index 687e1ee..9d26f25 100644 --- a/docs/tips/pre-rendered.md +++ b/docs/tips/pre-rendered.md @@ -10,39 +10,20 @@ When the browser requests the HTML file, some of the elements may have been pre- On the client side, if we just create a render function as usual, the first call to the render function would not do anything. This is because deku would assume that the container's pre-rendered content is properly rendered. ```js -var render = createApp(document.getElementById("container")) -render(

pre-rendered text

) //do nothing +var render = createApp(document.getElementById("container"), { reuseMarkup: true }) +render(

pre-rendered text

) // do nothing, just assign event listeners, if any ``` -This means if the virtualDOM describes a different HTML element, deku is not going to fix it for you. +If the virtualDOM describes a different HTML element, deku will rerender it completely, even if `reuseMarkup` flag is set. ```js -var render = createApp(document.getElementById("container")) -render(

Meow!

) //do nothing +var render = createApp(document.getElementById("container"), { reuseMarkup: true }) +render(

Meow!

) // will perform full rerender ``` -Therefore, to be 100% safe, one may want to use the `autoFix` attribute: +### Notes -```html -

pre-rendered text

-``` - -In this case, on the first call to the render function, deku would destroy and recreate the elements inside the container, after a quick string-comparison of the pre-rendered and to-be-rendered versions: - -```js -var render = createApp(document.getElementById("container")) -render(

Meow!

) //re-rendered the HTML due to difference in HTML strings -``` - -Alternatively, one can also choose to do a [checksum comparison](https://en.wikipedia.org/wiki/Checksum) between the HTML strings instead. This can be done by getting the server to compute the [Adler32](https://en.wikipedia.org/wiki/Adler-32) checksum value of the string and add it to the container element's `checksum` attribute. +- To avoid injecting 'react-id'-like attributes into tags, Deku hardly relies on order of pre-rendered nodes. +- If starting part of pre-rendered nodes matches virtualDOM, Deku will reuse this part, but will rerender the rest. +- So this leads to the tip: if some components of your app are to be rendered on client-side only, it would be wise to place their markup closer to the end of the container to avoid full rerender of all other components. -```html -

pre-rendered text

-``` - -and similar to the using `autoFix` attribute, deku would fix the incorrect pre-rendered HTML on the first call to the render function. - -```js -var render = createApp(document.getElementById("container")) -render(

Meow!

) //re-rendered the HTML due to difference in adler32 checksums -``` From dbb94637f9bb9aa53b04cfe69c8fdadbf7c7b188 Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Wed, 1 Jun 2016 19:47:21 +0600 Subject: [PATCH 08/10] Fix code style --- .eslintrc | 60 ----------------------------------------- src/app/index.js | 6 ++--- src/dom/create.js | 24 ++++++++--------- src/dom/pool.js | 14 +++++----- src/dom/setAttribute.js | 6 +++-- test/app/index.js | 12 ++++----- test/dom/pool.js | 3 ++- 7 files changed, 34 insertions(+), 91 deletions(-) delete mode 100644 .eslintrc diff --git a/.eslintrc b/.eslintrc deleted file mode 100644 index bdbb22b..0000000 --- a/.eslintrc +++ /dev/null @@ -1,60 +0,0 @@ -{ - "parser": "babel-eslint", - "extends": "eslint:recommended", - "plugins": [ - "react" - ], - "env": { - "browser": true, - "es6": true - }, - "parserOptions": { - "ecmaFeatures": { - "modules": true, - "jsx": true - } - }, - "globals": { - "expect": true - }, - "rules": { - "react/jsx-uses-react": 2, - "react/jsx-uses-vars": 2, - "no-unused-vars": 2, - "no-cond-assign": 2, - "no-empty": 2, - "no-console": 2, - "semi": [2, "never"], - "camelcase": 2, - "comma-style": 2, - "comma-dangle": [2, "never"], - "no-mixed-spaces-and-tabs": 2, - "no-trailing-spaces": [2, { "skipBlankLines": true }], - "no-eval": 2, - "no-implied-eval": 2, - "no-new-func": 2, - "eqeqeq": 0, - "no-else-return": 2, - "no-redeclare": 2, - "no-dupe-keys": 2, - "radix": 2, - "strict": [2, "never"], - "no-shadow": 0, - "callback-return": [1, ["callback", "cb", "next", "done"]], - "no-delete-var": 2, - "no-undef-init": 2, - "no-shadow-restricted-names": 2, - "handle-callback-err": 0, - "no-lonely-if": 2, - "keyword-spacing": 2, - "constructor-super": 2, - "no-this-before-super": 2, - "no-dupe-class-members": 2, - "no-const-assign": 2, - "prefer-spread": 2, - "no-useless-concat": 2, - "no-var": 2, - "object-shorthand": 2, - "prefer-arrow-callback": 2 - } -} diff --git a/src/app/index.js b/src/app/index.js index 0fee92c..2b6b199 100644 --- a/src/app/index.js +++ b/src/app/index.js @@ -20,9 +20,9 @@ export function createApp (container, handler = noop, options = {}) { let rootId = options.id || '0' let dispatch = effect => effect && handler(effect) - if (typeof handler != 'function' && typeof handler == 'object') { - options = handler; - handler = noop; + if (typeof handler !== 'function' && typeof handler === 'object') { + options = handler + handler = noop } dom.enableNodeRecycling(options.enableNodeRecycling || false) diff --git a/src/dom/create.js b/src/dom/create.js index 315df5b..39f7300 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -13,28 +13,28 @@ const CREATE = 'create' const REPLACE = 'replace' const NOOP = 'noop' -export default function createElement(vnode, path, dispatch, context) { +export default function createElement (vnode, path, dispatch, context) { let DOM = createWithSideEffects(vnode, path, dispatch, context) runSideEffects(DOM.sideEffects) return DOM.element } -export function createElementThenEvents(vnode, path, dispatch, context, container = null) { +export function createElementThenEvents (vnode, path, dispatch, context, container = null) { let DOM = createWithSideEffects(vnode, path, dispatch, context, {originNode: container}) runSideEffects(DOM.sideEffects, {noEventListeners: true}) return { DOMnode: DOM.element, - attachEvents(PreRenderedElement) { + attachEvents (PreRenderedElement) { runSideEffects(DOM.sideEffects, {onlyEventListeners: true}, PreRenderedElement) } } } -export function enableNodeRecycling(flag) { +export function enableNodeRecycling (flag) { cache.enableRecycling(flag) } -export function storeInCache(node) { +export function storeInCache (node) { cache.store(node) } @@ -47,10 +47,10 @@ export function storeInCache(node) { * where P is either an empty array or an array of functions * and C is either an empty array or an array of trees of side effects */ -function runSideEffects(sideEffects, option, DOMElement) { +function runSideEffects (sideEffects, option, DOMElement) { if (sideEffects) { sideEffects.ofParent.map((sideEffect) => { sideEffect(option, DOMElement) }) - sideEffects.ofChildren.map((child,index) => { + sideEffects.ofChildren.map((child, index) => { if (DOMElement) { runSideEffects(child, option, DOMElement.childNodes[index]) } else { @@ -87,13 +87,13 @@ function createTextNode ({nodeValue}, {originNode}) { let action = CREATE if (originNode && originNode.nodeValue) { action = NOOP - if (originNode.nodeValue != value) { + if (originNode.nodeValue !== value) { action = REPLACE } } return { - element: action != NOOP ? document.createTextNode(value) : null, + element: action !== NOOP ? document.createTextNode(value) : null, sideEffects: null, action } @@ -131,7 +131,7 @@ function createHTMLElement (vnode, path, dispatch, context, {originNode}) { if (!originNode) { // no such element in markup -> create & append DOMElement = cache.get(vnode.tagName) action = CREATE - } else if (!originNode.tagName || originNode.tagName.toLowerCase() != vnode.tagName.toLowerCase()) { + } else if (!originNode.tagName || originNode.tagName.toLowerCase() !== vnode.tagName.toLowerCase()) { // found element, but with wrong type -> recreate & replace let newDOMElement = cache.get(vnode.tagName) Array.prototype.forEach.call(originNode.childNodes, (nodeChild) => { @@ -150,7 +150,7 @@ function createHTMLElement (vnode, path, dispatch, context, {originNode}) { let attributes = Object.keys(vnode.attributes) sideEffects.ofParent = attributes.map((name) => { - return function(option, element = DOMElement) { + return function (option, element = DOMElement) { setAttribute(element, name, vnode.attributes[name], null, option) } }) @@ -167,7 +167,7 @@ function createHTMLElement (vnode, path, dispatch, context, {originNode}) { * * @returns {Function} */ -function nodesUpdater(parentNode, path, dispatch, context) { +function nodesUpdater (parentNode, path, dispatch, context) { let i = 0 return (node, index, sideEffects) => { diff --git a/src/dom/pool.js b/src/dom/pool.js index 1e294e1..b5183d2 100644 --- a/src/dom/pool.js +++ b/src/dom/pool.js @@ -1,12 +1,12 @@ import createNativeElement from '@f/create-element' export default class Pool { - constructor() { + constructor () { this.storage = {} this._recyclingEnabled = false } - store(el) { + store (el) { if (!this._recyclingEnabled) { return } @@ -15,7 +15,7 @@ export default class Pool { el.parentNode.removeChild(el) } - if (!el.nodeType || el.nodeType != Node.ELEMENT_NODE) { + if (!el.nodeType || el.nodeType !== 1 /* Node.ELEMENT_NODE == 1 */) { return } @@ -38,11 +38,11 @@ export default class Pool { this.storage[tagName].push(el) } - enableRecycling(flag) { + enableRecycling (flag) { this._recyclingEnabled = flag } - get(tagName) { + get (tagName) { tagName = tagName.toLowerCase() if (this._recyclingEnabled && this.storage[tagName] && this.storage[tagName].length > 0) { return this.storage[tagName].pop() @@ -50,7 +50,7 @@ export default class Pool { return createNativeElement(tagName) } - preallocate(tagName, size) { + preallocate (tagName, size) { if (!this._recyclingEnabled) { return } @@ -71,7 +71,7 @@ export default class Pool { } // for tests only - _getStorageSizeFor(tagName) { + _getStorageSizeFor (tagName) { return (this.storage[tagName] || []).length } } diff --git a/src/dom/setAttribute.js b/src/dom/setAttribute.js index 4b15182..1376327 100644 --- a/src/dom/setAttribute.js +++ b/src/dom/setAttribute.js @@ -29,7 +29,7 @@ export function removeAttribute (DOMElement, name, previousValue) { export function setAttribute (DOMElement, name, value, previousValue, option = {}) { if (value === previousValue) return - if(!option.noEventListeners){ + if (!option.noEventListeners) { let eventType = events[name] if (eventType) { if (isFunction(previousValue)) DOMElement.removeEventListener(eventType, previousValue) @@ -37,7 +37,9 @@ export function setAttribute (DOMElement, name, value, previousValue, option = { return } } - if(option.onlyEventListeners) return + if (option.onlyEventListeners) { + return + } if (!isValidAttribute(value)) { removeAttribute(DOMElement, name, previousValue) return diff --git a/test/app/index.js b/test/app/index.js index 768001b..4a4e4aa 100644 --- a/test/app/index.js +++ b/test/app/index.js @@ -294,7 +294,7 @@ test('rendering in a container with pre-rendered HTML', t => { '

Nyan!

', 're-rendered due to changed tagName' ) - + document.body.removeChild(el) t.end() }) @@ -303,7 +303,7 @@ test('rerendering custom element with changing props', t => { let el = document.createElement('div') document.body.appendChild(el) const Comp = { - render({props, path}) { + render ({props, path}) { return ( woot ) @@ -316,7 +316,7 @@ test('rerendering custom element with changing props', t => { el.children[0].attributes.chck = 1 el.children[0].children[0].attributes.chck = 2 - render(
) + render(
) t.equal( el.children[0].attributes.chck, @@ -344,11 +344,11 @@ test('rendering in a container with pre-rendered HTML and click events', t => { el.innerHTML = '
' let render = createApp(el) let a = () => { - t.assert("clicked") + t.assert('clicked') } let b = () => { - t.assert("clicked") - t.assert("clicked") + t.assert('clicked') + t.assert('clicked') } render(
) let arr = el.querySelectorAll('button, span') diff --git a/test/dom/pool.js b/test/dom/pool.js index 18c9c42..aa9f313 100644 --- a/test/dom/pool.js +++ b/test/dom/pool.js @@ -1,3 +1,4 @@ +/* global SVGElement */ import test from 'tape' import Pool from '../../src/dom/pool' @@ -19,7 +20,7 @@ test('getNewDomNode', t => { let pool = new Pool() let newNode = pool.get('div') t.equal(newNode.tagName.toLowerCase(), 'div', 'Pool created and returned div') - + pool.enableRecycling(true) newNode = pool.get('div') t.equal(newNode.tagName.toLowerCase(), 'div', 'Pool created and returned div with enabled recycling') From 6f55011676e676cd3e1255c5916ed6b70437366c Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Thu, 2 Jun 2016 12:35:54 +0600 Subject: [PATCH 09/10] Fix DOM nodes cleanup-on-reuse bug --- src/dom/pool.js | 15 ++++++++------- test/dom/pool.js | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/dom/pool.js b/src/dom/pool.js index b5183d2..9624631 100644 --- a/src/dom/pool.js +++ b/src/dom/pool.js @@ -7,30 +7,29 @@ export default class Pool { } store (el) { - if (!this._recyclingEnabled) { + if (!this._recyclingEnabled || el._collected || !el.nodeType || el.nodeType !== 1 /* Node.ELEMENT_NODE == 1 */) { return } + el._collected = true if (el && el.parentNode) { el.parentNode.removeChild(el) } - if (!el.nodeType || el.nodeType !== 1 /* Node.ELEMENT_NODE == 1 */) { - return - } - let tagName = el.tagName.toLowerCase() if (!this.storage[tagName]) { this.storage[tagName] = [] } // little cleanup + el.className = '' for (let i = 0; i < el.attributes.length; i++) { el.removeAttribute(el.attributes[i].name) } if (el.childNodes.length > 0) { - for (let i = 0; i < el.childNodes.length; i++) { + // Iterate backwards, because childNodes is live collection + for (let i = el.childNodes.length - 1; i >= 0; i--) { this.store(el.childNodes[i]) } } @@ -45,7 +44,9 @@ export default class Pool { get (tagName) { tagName = tagName.toLowerCase() if (this._recyclingEnabled && this.storage[tagName] && this.storage[tagName].length > 0) { - return this.storage[tagName].pop() + let el = this.storage[tagName].pop() + delete el._collected + return el } return createNativeElement(tagName) } diff --git a/test/dom/pool.js b/test/dom/pool.js index aa9f313..887c6fd 100644 --- a/test/dom/pool.js +++ b/test/dom/pool.js @@ -16,6 +16,26 @@ test('storeDomNode', t => { t.end() }) +test('storeNestedNodes', t => { + let node = document.createElement('div') + for (let i = 0; i < 10; i++) { + node.appendChild(document.createElement('div')) + } + let pool = new Pool() + pool.enableRecycling(true) + + pool.store(node) + + let childNodesCount = 0 + while (pool._getStorageSizeFor('div') > 0) { + let storedNode = pool.get('div') + childNodesCount += storedNode.childNodes.length + } + + t.equal(childNodesCount, 0, 'Stored nested nodes were flattened and do not have children') + t.end() +}) + test('getNewDomNode', t => { let pool = new Pool() let newNode = pool.get('div') From c98fc023db4a1b5ca6d6c6cdf2a2648201c7bed3 Mon Sep 17 00:00:00 2001 From: Oleg Klimenko Date: Mon, 11 Jul 2016 14:27:26 +0600 Subject: [PATCH 10/10] CR: Fixed same paths in thunks --- src/dom/create.js | 3 ++- src/dom/update.js | 4 ++-- test/app/thunk.js | 7 +++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/dom/create.js b/src/dom/create.js index 39f7300..3f403bc 100644 --- a/src/dom/create.js +++ b/src/dom/create.js @@ -112,7 +112,8 @@ function createThunk (vnode, path, dispatch, context, options) { context } let output = vnode.fn(model) - let { element, sideEffects, action } = createWithSideEffects(output, path, dispatch, context, options) + let childPath = createPath(path, output.key || '0') + let { element, sideEffects, action } = createWithSideEffects(output, childPath, dispatch, context, options) effects.ofChildren.push(sideEffects) if (onCreate) dispatch(onCreate(model)) diff --git a/src/dom/update.js b/src/dom/update.js index 9034460..53beaf2 100644 --- a/src/dom/update.js +++ b/src/dom/update.js @@ -1,5 +1,5 @@ import {setAttribute, removeAttribute} from './setAttribute' -import {isThunk} from '../element' +import {isThunk, createPath} from '../element' import {Actions, diffNode} from '../diff' import reduceArray from '@f/reduce-array' import createElement, {storeInCache} from './create' @@ -88,7 +88,7 @@ function updateThunk (DOMElement, prev, next, path, dispatch, context) { context } let nextNode = next.fn(model) - let changes = diffNode(prevNode, nextNode, path) + let changes = diffNode(prevNode, nextNode, createPath(path, '0')) DOMElement = reduceArray(updateElement(dispatch, context), DOMElement, changes) if (onUpdate) dispatch(onUpdate(model)) next.state = { diff --git a/test/app/thunk.js b/test/app/thunk.js index 686c3c1..7145665 100644 --- a/test/app/thunk.js +++ b/test/app/thunk.js @@ -249,10 +249,10 @@ test('path should stay the same on when thunk is updated', t => { document.body.appendChild(el) let MyButton = { onUpdate ({path}) { - t.equal(path, '0.0', 'onUpdate') + t.equal(path, '0.0.0', 'onUpdate') }, render ({path, children}) { - t.equal(path, '0.0', 'onRender') + t.equal(path, '0.0.0', 'onRender') return } } @@ -286,7 +286,6 @@ test('path should stay the same on when thunk is replaced', t => { render(
) render(
) render(
) - // TODO: nested chunk gets same path to minify data - is it acceptable? - render(
) + render(
) t.end() })