From 8851855b5b666cc6b178427de2899f8e509ab81a Mon Sep 17 00:00:00 2001 From: Nicolas Gaborit Date: Wed, 26 Oct 2016 21:46:40 +0200 Subject: [PATCH] Node methods optimizations (#364) * Node.getParent exits earlier * Add Node.getAncestors method * Remove numerous getParent in Node.getClosest * Remove use of assertDescendant in getPath Still throws when not finding the descendant though * Remove assertDescendant from Node.updateDescendant * Remove assertDescendant from Node.removeDescendant * Fix Node.findDescendant, which always returned first level descendants * Add Node.findDescendantDeep * Memoize Node.getAncestors * Implement and use Node.get{First|Last}Text * Add jsdom devDepencency Required as peer dependency by mocha-jsdom --- package.json | 1 + src/components/void.js | 2 +- src/models/node.js | 195 ++++++++++++++++++++++------- src/models/selection.js | 12 +- src/models/state.js | 2 +- src/transforms/at-current-range.js | 10 +- 6 files changed, 164 insertions(+), 58 deletions(-) diff --git a/package.json b/package.json index 1938cdedd..4bc976730 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "http-server": "^0.9.0", "is-image": "^1.0.1", "is-url": "^1.2.2", + "jsdom": "9.8.0", "jsdom": "9.6.0", "jsdom-global": "2.1.0", "microtime": "2.1.1", diff --git a/src/components/void.js b/src/components/void.js index 22a610d5f..b727c35be 100644 --- a/src/components/void.js +++ b/src/components/void.js @@ -130,7 +130,7 @@ class Void extends React.Component { renderLeaf = () => { const { node, schema, state } = this.props - const child = node.getTexts().first() + const child = node.getFirstText() const ranges = child.getRanges() const text = '' const marks = Mark.createSet() diff --git a/src/models/node.js b/src/models/node.js index 428b70260..5472c7a85 100644 --- a/src/models/node.js +++ b/src/models/node.js @@ -105,23 +105,53 @@ const Node = { }, /** - * Recursively find all ancestor nodes by `iterator`. + * Recursively find all descendant nodes by `iterator`. Breadth first. * * @param {Function} iterator - * @return {Node} node + * @return {Node or Null} node */ findDescendant(iterator) { - return ( - this.nodes.find(iterator) || - this.nodes - .map(node => node.kind == 'text' ? null : node.findDescendant(iterator)) - .find(exists => exists) - ) + const found = this.nodes.find(iterator) + if (found) return found + + let descendantFound = null + this.nodes.find(node => { + if (node.kind != 'text') { + descendantFound = node.findDescendant(iterator) + return descendantFound + } else { + return false + } + }) + + return descendantFound }, /** - * Recursively filter all ancestor nodes with `iterator`. + * Recursively find all descendant nodes by `iterator`. Depth first. + * + * @param {Function} iterator + * @return {Node or Null} node + */ + + findDescendantDeep(iterator) { + let descendantFound = null + + const found = this.nodes.find(node => { + if (node.kind != 'text') { + descendantFound = node.findDescendantDeep(iterator) + return descendantFound || iterator(node) + } + + return iterator(node) ? node : null + }) + + return descendantFound || found + }, + + /** + * Recursively filter all descendant nodes with `iterator`. * * @param {Function} iterator * @return {List} nodes @@ -136,7 +166,7 @@ const Node = { }, /** - * Recursively filter all ancestor nodes with `iterator`, depth-first. + * Recursively filter all descendant nodes with `iterator`, depth-first. * * @param {Function} iterator * @return {List} nodes @@ -286,14 +316,13 @@ const Node = { */ getClosest(key, iterator) { - let node = this.assertDescendant(key) - - while (node = this.getParent(node)) { - if (node == this) return null - if (iterator(node)) return node + let ancestors = this.getAncestors(key) + if (!ancestors) { + throw new Error(`Could not find a descendant node with key "${key}".`) } - return null + // Exclude this node itself + return ancestors.rest().findLast(iterator) }, /** @@ -410,16 +439,7 @@ const Node = { getDescendant(key) { key = Normalize.key(key) - let child = this.getChild(key) - if (child) return child - - this.nodes.find((node) => { - if (node.kind == 'text') return false - child = node.getDescendant(key) - return child - }) - - return child + return this.findDescendantDeep(node => node.key == key) }, /** @@ -654,10 +674,10 @@ const Node = { let last if (child.kind == 'block') { - last = child.getTexts().last() + last = child.getLastText() } else { const block = this.getClosestBlock(key) - last = block.getTexts().last() + last = block.getLastText() } const next = this.getNextText(last) @@ -748,10 +768,13 @@ const Node = { let node = null - this.nodes.forEach((child) => { - if (child.kind == 'text') return - const match = child.getParent(key) - if (match) node = match + this.nodes.find((child) => { + if (child.kind == 'text') { + return false + } else { + node = child.getParent(key) + return node + } }) return node @@ -760,7 +783,7 @@ const Node = { /** * Get the path of a descendant node by `key`. * - * @param {String || Node} node + * @param {String || Node} key * @return {Array} */ @@ -769,17 +792,50 @@ const Node = { if (key == this.key) return [] - let child = this.assertDescendant(key) let path = [] + let childKey = key let parent - while (parent = this.getParent(child)) { - const index = parent.nodes.indexOf(child) + // Efficient with getParent memoization + while (parent = this.getParent(childKey)) { + const index = parent.nodes.findIndex(n => n.key === childKey) path.unshift(index) - child = parent + childKey = parent.key } - return path + if (childKey === key) { + // Did not loop once, meaning we could not find the child + throw new Error(`Could not find a descendant node with key "${key}".`) + } else { + return path + } + }, + + /** + * Get the path of ancestors of a descendant node by `key`. + * + * @param {String || Node} node + * @return {List or Null} + */ + + getAncestors(key) { + key = Normalize.key(key) + + if (key == this.key) return List() + if (this.hasChild(key)) return List([this]) + + let ancestors + this.nodes.find((node) => { + if (node.kind == 'text') return false + ancestors = node.getAncestors(key) + return ancestors + }) + + if (ancestors) { + return ancestors.unshift(this) + } else { + return null + } }, /** @@ -824,10 +880,10 @@ const Node = { let first if (child.kind == 'block') { - first = child.getTexts().first() + first = child.getFirstText() } else { const block = this.getClosestBlock(key) - first = block.getTexts().first() + first = block.getFirstText() } const previous = this.getPreviousText(first) @@ -881,6 +937,34 @@ const Node = { }, Block.createList()) }, + /** + * Get the first child text node. + * + * @return {Node || Null} node + */ + + getFirstText() { + return this.findDescendantDeep(node => node.kind == 'text') + }, + + /** + * Get the last child text node. + * + * @return {Node} node + */ + + getLastText() { + let descendantFound = null + + const found = this.nodes.findLast((node) => { + if (node.kind == 'text') return true + descendantFound = node.getLastText() + return descendantFound + }) + + return descendantFound || found + }, + /** * Get all of the text nodes in a `range`. * @@ -1163,10 +1247,13 @@ const Node = { */ removeDescendant(key) { + key = Normalize.key(key) + let node = this - const desc = node.assertDescendant(key) - let parent = node.getParent(desc) - const index = parent.nodes.indexOf(desc) + let parent = node.getParent(key) + if (!parent) throw new Error(`Could not find a descendant node with key "${key}".`) + + const index = parent.nodes.findIndex(n => n.key === key) const isParent = node == parent const nodes = parent.nodes.splice(index, 1) @@ -1277,8 +1364,22 @@ const Node = { */ updateDescendant(node) { - this.assertDescendant(node) - return this.mapDescendants(d => d.key == node.key ? node : d) + let found = false + + const result = this.mapDescendants(d => { + if (d.key == node.key) { + found = true + return node + } else { + return d + } + }) + + if (!found) { + throw new Error(`Could not update descendant node with key "${node.key}".`) + } else { + return result + } }, /** @@ -1304,6 +1405,8 @@ memoize(Node, [ 'filterDescendants', 'filterDescendantsDeep', 'findDescendant', + 'findDescendantDeep', + 'getAncestors', 'getBlocks', 'getBlocksAtRange', 'getCharactersAtRange', @@ -1322,6 +1425,7 @@ memoize(Node, [ 'getDepth', 'getDescendant', 'getDescendantDecorators', + 'getFirstText', 'getFragmentAtRange', 'getFurthest', 'getFurthestBlock', @@ -1329,6 +1433,7 @@ memoize(Node, [ 'getHighestChild', 'getHighestOnlyChildParent', 'getInlinesAtRange', + 'getLastText', 'getMarksAtRange', 'getNextBlock', 'getNextSibling', diff --git a/src/models/selection.js b/src/models/selection.js index c16e1be79..560658043 100644 --- a/src/models/selection.js +++ b/src/models/selection.js @@ -148,7 +148,7 @@ class Selection extends new Record(DEFAULTS) { hasAnchorAtStartOf(node) { if (this.anchorOffset != 0) return false - const first = node.kind == 'text' ? node : node.getTexts().first() + const first = node.kind == 'text' ? node : node.getFirstText() return this.anchorKey == first.key } @@ -160,7 +160,7 @@ class Selection extends new Record(DEFAULTS) { */ hasAnchorAtEndOf(node) { - const last = node.kind == 'text' ? node : node.getTexts().last() + const last = node.kind == 'text' ? node : node.getLastText() return this.anchorKey == last.key && this.anchorOffset == last.length } @@ -202,7 +202,7 @@ class Selection extends new Record(DEFAULTS) { */ hasFocusAtEndOf(node) { - const last = node.kind == 'text' ? node : node.getTexts().last() + const last = node.kind == 'text' ? node : node.getLastText() return this.focusKey == last.key && this.focusOffset == last.length } @@ -215,7 +215,7 @@ class Selection extends new Record(DEFAULTS) { hasFocusAtStartOf(node) { if (this.focusOffset != 0) return false - const first = node.kind == 'text' ? node : node.getTexts().first() + const first = node.kind == 'text' ? node : node.getFirstText() return this.focusKey == first.key } @@ -260,7 +260,7 @@ class Selection extends new Record(DEFAULTS) { const { isExpanded, startKey, startOffset } = this if (isExpanded) return false if (startOffset != 0) return false - const first = node.kind == 'text' ? node : node.getTexts().first() + const first = node.kind == 'text' ? node : node.getFirstText() return startKey == first.key } @@ -274,7 +274,7 @@ class Selection extends new Record(DEFAULTS) { isAtEndOf(node) { const { endKey, endOffset, isExpanded } = this if (isExpanded) return false - const last = node.kind == 'text' ? node : node.getTexts().last() + const last = node.kind == 'text' ? node : node.getLastText() return endKey == last.key && endOffset == last.length } diff --git a/src/models/state.js b/src/models/state.js index 56ead2f7e..6e1725c1f 100644 --- a/src/models/state.js +++ b/src/models/state.js @@ -47,7 +47,7 @@ class State extends new Record(DEFAULTS) { let selection = Selection.create(properties.selection) if (selection.isUnset) { - const text = document.getTexts().first() + const text = document.getFirstText() selection = selection.collapseToStartOf(text) } diff --git a/src/transforms/at-current-range.js b/src/transforms/at-current-range.js index fad058576..4d376b703 100644 --- a/src/transforms/at-current-range.js +++ b/src/transforms/at-current-range.js @@ -70,7 +70,7 @@ export function _delete(transform) { after = selection.collapseToEndOf(previous) } } else { - const last = previous.getTexts().last() + const last = previous.getLastText() after = selection.collapseToEndOf(last) } } @@ -143,7 +143,7 @@ export function deleteBackward(transform, n = 1) { after = selection.collapseToEndOf(previous) } } else { - const last = previous.getTexts().last() + const last = previous.getLastText() after = selection.collapseToEndOf(last) } } else { @@ -206,7 +206,7 @@ export function deleteForward(transform, n = 1) { after = selection.collapseToEndOf(previous) } } else { - const last = previous.getTexts().last() + const last = previous.getLastText() after = selection.collapseToEndOf(last) } } @@ -259,7 +259,7 @@ export function insertFragment(transform, fragment) { if (!fragment.length) return transform - const lastText = fragment.getTexts().last() + const lastText = fragment.getLastText() const lastInline = fragment.getClosestInline(lastText) const beforeTexts = document.getTexts() const appending = selection.hasEdgeAtEndOf(document.getDescendant(selection.endKey)) @@ -575,7 +575,7 @@ export function wrapInline(transform, properties) { } else if (selection.startOffset == 0) { - const text = previous ? document.getNextText(previous) : document.getTexts().first() + const text = previous ? document.getNextText(previous) : document.getFirstText() after = selection.moveToRangeOf(text) }