From 0cfd54fc196cbbf115ce619edc51d876e11eecd3 Mon Sep 17 00:00:00 2001 From: Nicolas Gaborit Date: Fri, 11 May 2018 04:43:59 +0200 Subject: [PATCH] Speed up decorations rendering (#1801) * order, assign decoration ranges to node children * move decorations ordering method to utils * Add immutable dep. Prettify * Try to improve perfs * Fix orderChildDecorations * Compute correct order from the start for range starts * Add tests for order-child-decorations * Optimize text decoration rendering * Rewrite with simpler API. Apply it to Content as well * Lint * Fix tests --- packages/slate-react/package.json | 2 + .../slate-react/src/components/content.js | 21 +-- packages/slate-react/src/components/node.js | 34 +++-- packages/slate-react/src/components/text.js | 7 +- .../src/utils/get-children-decorations.js | 129 ++++++++++++++++++ packages/slate-react/test/index.js | 1 + .../test/utils/get-children-decorations.js | 93 +++++++++++++ packages/slate-react/test/utils/index.js | 21 +++ 8 files changed, 287 insertions(+), 21 deletions(-) create mode 100644 packages/slate-react/src/utils/get-children-decorations.js create mode 100644 packages/slate-react/test/utils/get-children-decorations.js create mode 100644 packages/slate-react/test/utils/index.js diff --git a/packages/slate-react/package.json b/packages/slate-react/package.json index f332f4fef..46800400c 100644 --- a/packages/slate-react/package.json +++ b/packages/slate-react/package.json @@ -30,11 +30,13 @@ "slate-prop-types": "^0.4.28" }, "peerDependencies": { + "immutable": "^3.8.1", "react": "^0.14.0 || ^15.0.0 || ^16.0.0", "react-dom": "^0.14.0 || ^15.0.0 || ^16.0.0", "slate": ">=0.32.0" }, "devDependencies": { + "immutable": "^3.8.1", "mocha": "^2.5.3", "slate": "^0.33.5", "slate-hyperscript": "^0.5.11", diff --git a/packages/slate-react/src/components/content.js b/packages/slate-react/src/components/content.js index e282c90ce..d6731bd64 100644 --- a/packages/slate-react/src/components/content.js +++ b/packages/slate-react/src/components/content.js @@ -15,6 +15,7 @@ import EVENT_HANDLERS from '../constants/event-handlers' import Node from './node' import findDOMRange from '../utils/find-dom-range' import findRange from '../utils/find-range' +import getChildrenDecorations from '../utils/get-children-decorations' import scrollToSelection from '../utils/scroll-to-selection' import removeAllRanges from '../utils/remove-all-ranges' @@ -453,13 +454,17 @@ class Content extends React.Component { tagName, spellCheck, } = props - const { value } = editor + const { value, stack } = editor const Container = tagName - const { document, selection } = value + const { document, selection, decorations } = value const indexes = document.getSelectionIndexes(selection, selection.isFocused) + const decs = document.getDecorations(stack).concat(decorations || []) + const childrenDecorations = getChildrenDecorations(document, decs) + const children = document.nodes.toArray().map((child, i) => { const isSelected = !!indexes && indexes.start <= i && i < indexes.end - return this.renderNode(child, isSelected) + + return this.renderNode(child, isSelected, childrenDecorations[i]) }) const handlers = EVENT_HANDLERS.reduce((obj, handler) => { @@ -532,18 +537,16 @@ class Content extends React.Component { * @return {Element} */ - renderNode = (child, isSelected) => { + renderNode = (child, isSelected, decorations) => { const { editor, readOnly } = this.props const { value } = editor - const { document, decorations } = value - const { stack } = editor - let decs = document.getDecorations(stack) - if (decorations) decs = decorations.concat(decs) + const { document } = value + return ( { + const decs = decorations.concat(node.getDecorations(stack)) + const childrenDecorations = getChildrenDecorations(node, decs) + + let children = [] + node.nodes.forEach((child, i) => { const isChildSelected = !!indexes && indexes.start <= i && i < indexes.end - return this.renderNode(child, isChildSelected) + + children.push( + this.renderNode(child, isChildSelected, childrenDecorations[i]) + ) }) - // Attributes that the developer must to mix into the element in their + // Attributes that the developer must mix into the element in their // custom node renderer component. const attributes = { 'data-key': node.key } @@ -172,18 +186,18 @@ class Node extends React.Component { * * @param {Node} child * @param {Boolean} isSelected + * @param {Array} decorations * @return {Element} */ - renderNode = (child, isSelected) => { - const { block, decorations, editor, node, readOnly } = this.props - const { stack } = editor + renderNode = (child, isSelected, decorations) => { + const { block, editor, node, readOnly } = this.props const Component = child.object == 'text' ? Text : Node - const decs = decorations.concat(node.getDecorations(stack)) + return ( { const { startKey, endKey } = d if (startKey == key || endKey == key) return true + if (startKey === endKey) return false const startsBefore = document.areDescendantsSorted(startKey, key) + if (!startsBefore) return false const endsAfter = document.areDescendantsSorted(key, endKey) - return startsBefore && endsAfter + return endsAfter }) - const leaves = node.getLeaves(decs) + // PERF: Take advantage of cache by avoiding arguments + const leaves = decs.size === 0 ? node.getLeaves() : node.getLeaves(decs) let offset = 0 const children = leaves.map((leaf, i) => { diff --git a/packages/slate-react/src/utils/get-children-decorations.js b/packages/slate-react/src/utils/get-children-decorations.js new file mode 100644 index 000000000..9426d3da3 --- /dev/null +++ b/packages/slate-react/src/utils/get-children-decorations.js @@ -0,0 +1,129 @@ +import { Set } from 'immutable' + +/** + * Split the decorations in lists of relevant decorations for each child. + * + * @param {Node} node + * @param {List} decorations + * @return {Array>} + */ + +function getChildrenDecorations(node, decorations) { + const activeDecorations = Set().asMutable() + const childrenDecorations = [] + + orderChildDecorations(node, decorations).forEach(item => { + if (item.isRangeStart) { + // Item is a decoration start + activeDecorations.add(item.decoration) + } else if (item.isRangeEnd) { + // item is a decoration end + activeDecorations.remove(item.decoration) + } else { + // Item is a child node + childrenDecorations.push(activeDecorations.toList()) + } + }) + + return childrenDecorations +} + +/** + * Orders the children of provided node and its decoration endpoints (start, end) + * so that decorations can be passed only to relevant children (see use in Node.render()) + * + * @param {Node} node + * @param {List} decorations + * @return {Array} + * + * where type Item = + * { + * child: Node, + * // Index of the child in its parent + * index: number + * } + * or { + * // True if this represents the start of the given decoration + * isRangeStart: boolean, + * // True if this represents the end of the given decoration + * isRangeEnd: boolean, + * decoration: Range + * } + */ + +function orderChildDecorations(node, decorations) { + if (decorations.isEmpty()) { + return node.nodes.toArray().map((child, index) => ({ + child, + index, + })) + } + + // Map each key to its global order + const keyOrders = { [node.key]: 0 } + let globalOrder = 1 + node.forEachDescendant(child => { + keyOrders[child.key] = globalOrder + globalOrder = globalOrder + 1 + }) + + const childNodes = node.nodes.toArray() + + const endPoints = childNodes.map((child, index) => ({ + child, + index, + order: keyOrders[child.key], + })) + + decorations.forEach(decoration => { + // Range start. + // A rangeStart should be before the child containing its startKey, in order + // to consider it active before going down the child. + const startKeyOrder = keyOrders[decoration.startKey] + const containingChildOrder = + startKeyOrder === undefined + ? 0 + : getContainingChildOrder(childNodes, keyOrders, startKeyOrder) + endPoints.push({ + isRangeStart: true, + order: containingChildOrder - 0.5, + decoration, + }) + + // Range end. + const endKeyOrder = (keyOrders[decoration.endKey] || globalOrder) + 0.5 + endPoints.push({ + isRangeEnd: true, + order: endKeyOrder, + decoration, + }) + }) + + return endPoints.sort((a, b) => (a.order > b.order ? 1 : -1)) +} + +/* + * Returns the key order of the child right before the given order. + */ + +function getContainingChildOrder(children, keyOrders, order) { + // Find the first child that is after the given key + const nextChildIndex = children.findIndex( + child => order < keyOrders[child.key] + ) + + if (nextChildIndex <= 0) { + return 0 + } + + const containingChild = children[nextChildIndex - 1] + return keyOrders[containingChild.key] +} + +/** + * Export. + * + * @type {Function} + */ + +export default getChildrenDecorations diff --git a/packages/slate-react/test/index.js b/packages/slate-react/test/index.js index 38fa6d76d..9f5e403d5 100644 --- a/packages/slate-react/test/index.js +++ b/packages/slate-react/test/index.js @@ -11,6 +11,7 @@ import { resetKeyGenerator } from 'slate' describe('slate-react', () => { require('./plugins') require('./rendering') + require('./utils') }) /** diff --git a/packages/slate-react/test/utils/get-children-decorations.js b/packages/slate-react/test/utils/get-children-decorations.js new file mode 100644 index 000000000..114802e78 --- /dev/null +++ b/packages/slate-react/test/utils/get-children-decorations.js @@ -0,0 +1,93 @@ +/** @jsx h */ + +import { List } from 'immutable' +import assert from 'assert' + +import h from '../helpers/h' + +import getChildrenDecorations from '../../src/utils/get-children-decorations' + +const value = ( + + + + First line + + + Second line + + + +) + +const { document } = value +const [paragraphB] = document.nodes.toArray() + +describe('getChildrenDecorations', () => { + it('should return the child list when no decorations are given', () => { + const actual = getChildrenDecorations(document, List()) + + const expected = [[], []] + + assert.deepEqual(actual.map(l => l.toArray()), expected) + }) + + it('should wrap a block with the range it contains', () => { + const decoration1 = { + startKey: 'c', + startOffset: 1, + endKey: 'c', + endOffset: 2, + decoration: 'd1', + } + + const actual = getChildrenDecorations(document, List([decoration1])) + + const expected = [[decoration1], []] + + assert.deepEqual(actual.map(l => l.toArray()), expected) + }) + + it('should sort two decorations inside a node', () => { + const decoration1 = { + startKey: 'c', + startOffset: 1, + endKey: 'c', + endOffset: 2, + decoration: 'd1', + } + + const decoration2 = { + startKey: 'c', + startOffset: 1, + endKey: 'e', + endOffset: 2, + decoration: 'd2', + } + + const actual = getChildrenDecorations( + document, + List([decoration1, decoration2]) + ) + + const expected = [[decoration1, decoration2], [decoration2]] + + assert.deepEqual(actual.map(l => l.toArray()), expected) + }) + + it('should sort decorations outside the node', () => { + const decoration1 = { + startKey: 'c', + startOffset: 1, + endKey: 'e', + endOffset: 2, + decoration: 'd1', + } + + const actual = getChildrenDecorations(paragraphB, List([decoration1])) + + const expected = [[decoration1]] + + assert.deepEqual(actual.map(l => l.toArray()), expected) + }) +}) diff --git a/packages/slate-react/test/utils/index.js b/packages/slate-react/test/utils/index.js new file mode 100644 index 000000000..a3f92c7d5 --- /dev/null +++ b/packages/slate-react/test/utils/index.js @@ -0,0 +1,21 @@ +/** + * Dependencies. + */ + +import { resetKeyGenerator } from 'slate' + +/** + * Tests. + */ + +describe('utils', () => { + require('./get-children-decorations') +}) + +/** + * Reset Slate's internal state before each text. + */ + +beforeEach(() => { + resetKeyGenerator() +})