mod25; performance improvements

package.json

@@ -2,7 +2,7 @@
   "name": "knockout-esnext",
   "description": "A modernized fork of Knockout.js intended for ES6+ browsers only",
   "homepage": "http://knockoutjs.com/",
-  "version": "3.5.1-mod25-pre",
+  "version": "3.5.1-mod25",
   "license": "MIT",
   "author": "The Knockout.js team",
   "main": "build/output/knockout-latest.js",
@@ -11,7 +11,7 @@
     "prepublish": "grunt",
     "pretest": "npm run rollup-dev",
     "test": "node spec/runner.node.js",
-    "rollup-dist": "rollup -c --environment BUILD_TARGET_ENV:dist",
+    "rollup-dist": "eslint --max-warnings 0 src/ && rollup -c --environment BUILD_TARGET_ENV:dist",
     "rollup-dev": "rollup -c --environment BUILD_TARGET_ENV:dev",
     "lint": "eslint --max-warnings 5 src/"
   },

src/.eslintrc

@@ -31,7 +31,7 @@
         "no-trailing-spaces": 0,
         "no-unused-expressions": 0,
         "no-tabs": 2,
-        "no-console": 0,
+        "no-console": 2,
         "no-unused-vars": [2, {"vars": "local", "args": "none"}],
         "no-else-return": 2,
         "space-unary-ops": [2, {"words": true, "nonwords": false}],

src/binding/defaultBindings/attr.js

@@ -9,7 +9,7 @@ bindingHandlers.attr = {
 
             // Find the namespace of this attribute, if any.
             let prefixLen = attrName.indexOf(':');
-            let namespace = prefixLen > 0 && element.lookupNamespaceURI && element.lookupNamespaceURI(attrName.substr(0, prefixLen));
+            let namespace = prefixLen > 0 && element.lookupNamespaceURI && element.lookupNamespaceURI(attrName.substring(0, prefixLen));
 
             // To cover cases like "attr: { checked:someProp }", we want to remove the attribute entirely
             // when someProp is a "no value"-like value (strictly null, false, or undefined)

src/binding/expressionRewriting.js

@@ -90,7 +90,7 @@ export const parseObjectLiteral = (objectLiteralString) => {
                 let match = tokens[i - 1].match(DIVISION_LOOK_BEHIND);
                 if (match && !KEYWORD_REGEX_LOOK_BEHIND[match[0]]) {
                     // The slash is actually a division punctuator; re-parse the remainder of the string (not including the slash)
-                    str = str.substr(str.indexOf(tok) + 1);
+                    str = str.substring(str.indexOf(tok) + 1);
                     tokens = str.match(BINDING_TOKEN);
                     i = -1;
                     // Continue with just the slash
@@ -154,10 +154,10 @@ export const preProcessBindings = (bindingsStringOrKeyValueArray, bindingOptions
     }
 
     if (propertyAccessorResultStrings.length) {
-        _processKeyValue(PROPERTY_WRITERS_BINDING_KEY, "{" + propertyAccessorResultStrings.substr(1) + " }");
+        _processKeyValue(PROPERTY_WRITERS_BINDING_KEY, "{" + propertyAccessorResultStrings.substring(1) + " }");
     }
 
-    return resultStrings.substr(1);
+    return resultStrings.substring(1);
 };
 
 export const bindingRewriteValidators = [];

src/binding/selectExtensions.js

@@ -8,8 +8,7 @@ const OPTION_VALUE_DOM_DATA_KEY = nextDomDataKey();
 export const readSelectOrOptionValue = (element) => {
     switch (element.tagName.toLowerCase()) {
         case 'option':
-            return (element[HAS_DOM_DATA_EXPANDO_PROPERTY]) ?
-                getDomData(element, OPTION_VALUE_DOM_DATA_KEY) : element.value;
+            return element[HAS_DOM_DATA_EXPANDO_PROPERTY] ? getDomData(element, OPTION_VALUE_DOM_DATA_KEY) : element.value;
         case 'select': {
             let selectedIndex = element.selectedIndex;
             return selectedIndex >= 0 ? readSelectOrOptionValue(element.options[selectedIndex]) : undefined;

src/memoization.js

@@ -1,25 +1,24 @@
 
-/** @type {Map<string, function>} */
-const _memoMap = new Map();
+/** 
+ * @type {Map<string, function>}
+ * @internal
+ */
+export const _memoMap = new Map();
 
-const MEMO_ID_PREFIX = Math.random() + '_';
-const MEMO_TEXT_START = '[ko_memo:'; // length 9 (= magic number used inside `parseMemoText`)
+const MEMO_TEXT_START = '[!KoMemo:'; // length 9 (= magic number used inside `parseMemoText`)
+const MEMO_ID_PREFIX = Date.now().toString(36) + '_';
 
-export const parseMemoText = (memoText) => memoText.startsWith(MEMO_TEXT_START) ? memoText.substr(9, memoText.length - 10) : null; //@inline
+export const parseMemoText = (memoText) => memoText.startsWith(MEMO_TEXT_START) ? memoText.substring(9) : null; //@inline
 
 let _nextMemoId = 1;
 
-// exported for knockout-internal performance optimizations only
-export let _hasMemoizedCallbacks = false;
-
 export const memoize = (callback) => {
     if (typeof callback !== "function") {
         throw new Error("You can only pass a function to ko.memoization.memoize()");
     }
     let memoId = MEMO_ID_PREFIX + (_nextMemoId++);
     _memoMap.set(memoId, callback);
-    _hasMemoizedCallbacks = true;
-    return '<!--' + MEMO_TEXT_START + memoId + ']-->';
+    return '<!--' + MEMO_TEXT_START + memoId + '-->';
 };
 
 export const unmemoize = (memoId, callbackParams) => {
@@ -32,43 +31,35 @@ export const unmemoize = (memoId, callbackParams) => {
         return true;
     } finally {
         _memoMap.delete(memoId);
-        _hasMemoizedCallbacks = !!_memoMap.size;
     }
 };
 
 export const unmemoizeDomNodeAndDescendants = (domNode, extraCallbackParamsArray) => {
-    if (!_hasMemoizedCallbacks || !domNode) {
+    if (!_memoMap.size || !domNode) {
         return;
     }
-    let memos = [];
+    let nodeAndMemoIdObjs = [];
 
     // (1) find memo comments in sub-tree
-    for (let node = domNode, nextNodes = []; node; node = nextNodes && nextNodes.shift()) {
-        let nodeType = node.nodeType;
-        if (nodeType === 8) {
-            let nodeValue = node.nodeValue, // local nodeValue looks redundant but will reduce size of inlined `parseMemoText` call
-                memoId = parseMemoText(nodeValue);
-            if (memoId) {
-                memos.push({node, memoId});
+    for (let node = domNode, nextNodes = [], memoId, memoText, nodeType; node; node = nextNodes.length && nextNodes.shift()) {
+        if ((nodeType = node.nodeType) === 8) {
+            // (!) additional memoText assignment to allow inlining of parseMemoText() call
+            if ((memoText = node.nodeValue) && (memoId = parseMemoText(memoText))) {
+                nodeAndMemoIdObjs.push({node, memoId});
             }
-        } else if (nodeType === 1) {
-            let childNodes = node.childNodes;
-            if (childNodes.length) {
-                if (nextNodes.length) {
-                    nextNodes.unshift(...childNodes);
-                } else {
-                    nextNodes = [...childNodes];
-                }
+        } else if (nodeType === 1 && node.hasChildNodes()) {
+            if (nextNodes.length) {
+                nextNodes.unshift(...node.childNodes);
+            } else {
+                nextNodes = [...node.childNodes];
             }
         }
     }
 
     // (2) unmemoize & run memoized callbacks
-    for (let memo of memos) {
-        let node = memo.node,
-            combinedParams = extraCallbackParamsArray ? [node, ...extraCallbackParamsArray] : [node];
-
-        unmemoize(memo.memoId, combinedParams);
+    for (let o of nodeAndMemoIdObjs) {
+        let node = o.node;
+        unmemoize(o.memoId, extraCallbackParamsArray ? [node, ...extraCallbackParamsArray] : [node]);
         node.nodeValue = ''; // Neuter this node so we don't try to unmemoize it again
         node.remove(); // If possible, erase it totally (not always possible - someone else might just hold a reference to it then call unmemoizeDomNodeAndDescendants again)
     }

src/subscribables/extenders.js

@@ -12,7 +12,7 @@ export function applyExtenders(requestedExtenders) {
             if (typeof extenderHandler === 'function') {
                 target = extenderHandler(target, requestedExtenders[key]) || target;
             } else {
-                console.warn('Missing extender: ' + key);
+                console.warn('Missing extender: ' + key); // eslint-disable-line no-console
             }
         }
     }

src/templating/native/nativeTemplateEngine.js

@@ -14,10 +14,13 @@ export class NativeTemplateEngine extends TemplateEngine {
      */
     renderTemplateSource(templateSource, bindingContext, options, templateDocument) {
         let templateNode = templateSource.nodes();
-
+        
         if (templateNode) {
-            // Array.from is 35% slower than spread in Chrome 79
-            return [...templateNode.cloneNode(true).childNodes];
+            // Use-case "single-child templateNode" is very frequent, so deserves a faster treatment
+            // Array.from is 35% slower than spread in Chrome 79; 
+            // Spread is 25% slower than copy-by-for-loop, but more readable
+            return (templateNode.childNodes.length === 1) ? [templateNode.firstChild.cloneNode(true)]
+                                                          : [...templateNode.cloneNode(true).childNodes];
         }
         let templateText = templateSource.text();
         return parseHtmlFragment(templateText, templateDocument);

src/templating/templating.js

@@ -1,5 +1,5 @@
 import {nextSibling, setDomNodeChildren, emptyNode, childNodes, allowedBindings} from '../virtualElements';
-import {unmemoizeDomNodeAndDescendants, _hasMemoizedCallbacks} from '../memoization';
+import {unmemoizeDomNodeAndDescendants, _memoMap} from '../memoization';
 import {fixUpContinuousNodeArray, replaceDomNodes, moveCleanedNodesToContainerElement} from '../utils';
 import {ensureTemplateIsRewritten} from './templateRewriting';
 import {isObservableArray, isObservable, unwrapObservable} from '../subscribables/observableUtils';
@@ -89,7 +89,7 @@ const _activateBindingsOnContinuousNodeArray = (continuousNodeArray, bindingCont
         (node) => (node.nodeType === 1 || node.nodeType === 8) && applyBindings(bindingContext, node)
     );
 
-    if (_hasMemoizedCallbacks) {
+    if (_memoMap.size) {
         _invokeForEachNodeInContinuousRange(firstNode, lastNode,
             (node) => (node.nodeType === 1 || node.nodeType === 8) && unmemoizeDomNodeAndDescendants(node, [bindingContext])
         );

src/utils.domData.js

@@ -1,5 +1,6 @@
 
 export const DOM_DATASTORE_PROP = Symbol('ko-domdata');
+
 const KEY_PREFIX = 'ko_' + Date.now().toString(36) + '_';
 
 let _keyCount = 0;
@@ -20,7 +21,7 @@ export const setDomData = (node, key, value) => {
  * @param {Node} node
  * @return {boolean} - true if there was actually a domData deleted on the node
  */
-export const clearDomData = (node) => !!node[DOM_DATASTORE_PROP] && delete node[DOM_DATASTORE_PROP];
+export const clearDomData = (node) => node[DOM_DATASTORE_PROP] ? !(node[DOM_DATASTORE_PROP] = undefined) : false;
 
 /**
  * Returns a function that removes a given item from an array located under the node's domData[itemArrayDomDataKey].

src/utils.domManipulation.js

@@ -2,65 +2,53 @@ import {moveCleanedNodesToContainerElement} from './utils';
 import {emptyDomNode} from './utils.domNodes';
 import {unwrapObservable} from './subscribables/observableUtils';
 
-const NONE = [0, '', ''],
-    TABLE = [1, '<table>', '</table>'],
-    TBODY = [2, '<table><tbody>', '</tbody></table>'],
-    TR = [3, '<table><tbody><tr>', '</tr></tbody></table>'],
-    SELECT = [1, '<select multiple="multiple">', '</select>'],
-    LOOKUP = {
-        thead: TABLE, THEAD: TABLE,
-        tbody: TABLE, TBODY: TABLE,
-        tfoot: TABLE, TFOOT: TABLE,
-        tr: TBODY, TR: TBODY, 
-        td: TR, TD: TR,
-        th: TR, TH: TR,
-        option: SELECT, OPTION: SELECT,
-        optgroup: SELECT, OPTGROUP: SELECT
-    },
-    TAGS_REGEX = /^(?:<!--.*?-->\s*?)*?<([a-zA-Z]+)[\s>]/;
-
-export const parseHtmlFragment = (html, documentContext) => {
-    if (!documentContext) {
-        documentContext = document;
-    }
-    let windowContext = documentContext.parentWindow || documentContext.defaultView || window;
-
-    // Based on jQuery's "clean" function, but only accounting for table-related elements.
-    // If you have referenced jQuery, this won't be used anyway - KO will use jQuery's "clean" function directly
+const TABLE = [1, '<table>', '</table>'];
+const TBODY = [2, '<table><tbody>', '</tbody></table>'];
+const TR = [3, '<table><tbody><tr>', '</tr></tbody></table>'];
+const SELECT = [1, '<select multiple="multiple">', '</select>'];
+const WRAP_BY_TAG_NAME = {
+    thead: TABLE, THEAD: TABLE,
+    tbody: TABLE, TBODY: TABLE,
+    tfoot: TABLE, TFOOT: TABLE,
+    tr: TBODY, TR: TBODY, 
+    td: TR, TD: TR,
+    th: TR, TH: TR,
+    option: SELECT, OPTION: SELECT,
+    optgroup: SELECT, OPTGROUP: SELECT
+};
 
-    // Note that there's still an issue in IE < 9 whereby it will discard comment nodes that are the first child of
-    // a descendant node. For example: "<div><!-- mycomment -->abc</div>" will get parsed as "<div>abc</div>"
-    // This won't affect anyone who has referenced jQuery, and there's always the workaround of inserting a dummy node
-    // (possibly a text node) in front of the comment. So, KO does not attempt to workaround this IE issue automatically at present.
+// TODO try replacing regex call w/ "scan for first tagName function
+const TAGS_REGEX = /^(?:<!--.*?-->\s*?)*<([a-zA-Z]+)[\s>]/;
 
-    // Trim whitespace, otherwise indexOf won't work as expected
-    let div = documentContext.createElement('div'),
-        wrap = (TAGS_REGEX.test((html || '').trim()) && LOOKUP[RegExp.$1]) || NONE,
-        depth = wrap[0];
+/**
+ * A DIV element used for parsing HTML fragments exclusively for the own document (which should cover 99% of cases). 
+ * @type {?HTMLDivElement} 
+ */
+let _reusedDiv;
 
-    // Go to html and back, then peel off extra wrappers
-    // Note that we always prefix with some dummy text, because otherwise, IE<9 will strip out leading comment nodes in descendants. Total madness.
-    let markup = 'ignored<div>' + wrap[1] + html + wrap[2] + '</div>';
-    if (typeof windowContext['innerShiv'] === 'function') {
-        // Note that innerShiv is deprecated in favour of html5shiv. We should consider adding
-        // support for html5shiv (except if no explicit support is needed, e.g., if html5shiv
-        // somehow shims the native APIs so it just works anyway)
-        div.appendChild(windowContext['innerShiv'](markup));
+export const parseHtmlFragment = (html, doc = document) => {
+    let container = (doc === document) ? (_reusedDiv || (_reusedDiv = doc.createElement('div'))) : doc.createElement('div'),
+        wrap = TAGS_REGEX.test(html.trim()) && WRAP_BY_TAG_NAME[RegExp.$1];
+
+    if (wrap) {
+        container.innerHTML = '<div>' + wrap[1] + html + wrap[2] + '</div>';
+        for (let depth = wrap[0]; depth >= 0; --depth) {
+            container = container.lastChild;
+        }
     } else {
-        div.innerHTML = markup;
-    }
-
-    // Move to the right depth
-    while (depth--) {
-        div = div.lastChild;
+        container.innerHTML = '<div>' + html + '</div>';
+        container = container.lastChild;
     }
 
-    // return [...div.lastChild.childNodes];
-    // Rest operator is slow (manual creation of nodes array is 60% faster in FF81, 80% faster in Chrome; re-check in the future)
+    // Tried spread -> return [...div.lastChild.childNodes];
+    // But rest operator is slow; for-loop filling nodes array is 60% faster in FF81, 80% faster in Chrome (TODO: re-check in the future)
     let nodesArray = [];
-    for (let i = 0, nodeList = div.lastChild.childNodes, len = nodeList.length; i < len; i++) {
+    for (let i = 0, nodeList = container.childNodes, len = nodeList.length; i < len; i++) {
         nodesArray[i] = nodeList[i];
     }
+
+    container.remove(); // make sure to cut ties with the reused div
+
     return nodesArray;
 };