From 74ae169fcc9c954ae2e8a83f32fe4b454f258509 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 23 Aug 2017 14:52:50 +0200 Subject: [PATCH] Maintain a map of line components instead of line nodes and text nodes Other than simplifying the code, this will help us understand whether https://github.com/atom/atom/issues/15263 might be related to a node reuse issue. Signed-off-by: Nathan Sobo --- spec/text-editor-component-spec.js | 4 +- src/text-editor-component.js | 75 ++++++++++++++---------------- 2 files changed, 37 insertions(+), 42 deletions(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index e67360f96..84d964b06 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -4225,12 +4225,12 @@ function lineNumberNodeForScreenRow (component, row) { function lineNodeForScreenRow (component, row) { const renderedScreenLine = component.renderedScreenLineForRow(row) - return component.lineNodesByScreenLineId.get(renderedScreenLine.id) + return component.lineComponentsByScreenLineId.get(renderedScreenLine.id).element } function textNodesForScreenRow (component, row) { const screenLine = component.renderedScreenLineForRow(row) - return component.textNodesByScreenLineId.get(screenLine.id) + return component.lineComponentsByScreenLineId.get(screenLine.id).textNodes } function setScrollTop (component, scrollTop) { diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 0c36a0985..e36aa2aa2 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -124,8 +124,7 @@ class TextEditorComponent { this.blockDecorationSentinel.style.height = '1px' this.heightsByBlockDecoration = new WeakMap() this.blockDecorationResizeObserver = new ResizeObserver(this.didResizeBlockDecorations.bind(this)) - this.lineNodesByScreenLineId = new Map() - this.textNodesByScreenLineId = new Map() + this.lineComponentsByScreenLineId = new Map() this.overlayComponents = new Set() this.overlayDimensionsByElement = new WeakMap() this.shouldRenderDummyScrollbars = true @@ -590,7 +589,7 @@ class TextEditorComponent { } if (this.hasInitialMeasurements) { - const {lineNodesByScreenLineId, textNodesByScreenLineId} = this + const {lineComponentsByScreenLineId} = this const startRow = this.getRenderedStartRow() const endRow = this.getRenderedEndRow() @@ -619,8 +618,7 @@ class TextEditorComponent { highlightDecorations: this.decorationsToRender.highlights.get(tileStartRow), displayLayer: this.props.model.displayLayer, nodePool: this.lineNodesPool, - lineNodesByScreenLineId, - textNodesByScreenLineId + lineComponentsByScreenLineId })) } @@ -633,8 +631,7 @@ class TextEditorComponent { screenRow, displayLayer: this.props.model.displayLayer, nodePool: this.lineNodesPool, - lineNodesByScreenLineId, - textNodesByScreenLineId + lineComponentsByScreenLineId })) } }) @@ -2196,7 +2193,8 @@ class TextEditorComponent { measureLongestLineWidth () { if (this.longestLineToMeasure) { - this.measurements.longestLineWidth = this.lineNodesByScreenLineId.get(this.longestLineToMeasure.id).firstChild.offsetWidth + const lineComponent = this.lineComponentsByScreenLineId.get(this.longestLineToMeasure.id) + this.measurements.longestLineWidth = lineComponent.element.firstChild.offsetWidth this.longestLineToMeasure = null } } @@ -2226,9 +2224,9 @@ class TextEditorComponent { columnsToMeasure.sort((a, b) => a - b) const screenLine = this.renderedScreenLineForRow(row) - const lineNode = this.lineNodesByScreenLineId.get(screenLine.id) + const lineComponent = this.lineComponentsByScreenLineId.get(screenLine.id) - if (!lineNode) { + if (!lineComponent) { const error = new Error('Requested measurement of a line that is not currently rendered') error.metadata = { row, @@ -2240,7 +2238,8 @@ class TextEditorComponent { throw error } - const textNodes = this.textNodesByScreenLineId.get(screenLine.id) + const lineNode = lineComponent.element + const textNodes = lineComponent.textNodes let positionsForLine = this.horizontalPixelPositionsByScreenLineId.get(screenLine.id) if (positionsForLine == null) { positionsForLine = new Map() @@ -2355,7 +2354,7 @@ class TextEditorComponent { const linesClientLeft = this.refs.lineTiles.getBoundingClientRect().left const targetClientLeft = linesClientLeft + Math.max(0, left) - const textNodes = this.textNodesByScreenLineId.get(screenLine.id) + const {textNodes} = this.lineComponentsByScreenLineId.get(screenLine.id) let containingTextNodeIndex { @@ -3591,7 +3590,7 @@ class LinesTileComponent { createLines () { const { tileStartRow, screenLines, lineDecorations, textDecorations, - nodePool, displayLayer, lineNodesByScreenLineId, textNodesByScreenLineId + nodePool, displayLayer, lineComponentsByScreenLineId } = this.props this.lineComponents = [] @@ -3603,8 +3602,7 @@ class LinesTileComponent { textDecorations: textDecorations[i], displayLayer, nodePool, - lineNodesByScreenLineId, - textNodesByScreenLineId + lineComponentsByScreenLineId }) this.element.appendChild(component.element) this.lineComponents.push(component) @@ -3614,7 +3612,7 @@ class LinesTileComponent { updateLines (oldProps, newProps) { var { screenLines, tileStartRow, lineDecorations, textDecorations, - nodePool, displayLayer, lineNodesByScreenLineId, textNodesByScreenLineId + nodePool, displayLayer, lineComponentsByScreenLineId } = newProps var oldScreenLines = oldProps.screenLines @@ -3637,8 +3635,7 @@ class LinesTileComponent { textDecorations: textDecorations[newScreenLineIndex], displayLayer, nodePool, - lineNodesByScreenLineId, - textNodesByScreenLineId + lineComponentsByScreenLineId }) this.element.appendChild(newScreenLineComponent.element) this.lineComponents.push(newScreenLineComponent) @@ -3674,8 +3671,7 @@ class LinesTileComponent { textDecorations: textDecorations[newScreenLineIndex], displayLayer, nodePool, - lineNodesByScreenLineId, - textNodesByScreenLineId + lineComponentsByScreenLineId }) this.element.insertBefore(newScreenLineComponent.element, this.getFirstElementForScreenLine(oldProps, oldScreenLine)) newScreenLineComponents.push(newScreenLineComponent) @@ -3701,8 +3697,7 @@ class LinesTileComponent { textDecorations: textDecorations[newScreenLineIndex], displayLayer, nodePool, - lineNodesByScreenLineId, - textNodesByScreenLineId + lineComponentsByScreenLineId }) this.element.insertBefore(newScreenLineComponent.element, oldScreenLineComponent.element) oldScreenLineComponent.destroy() @@ -3737,11 +3732,11 @@ class LinesTileComponent { } } - return oldProps.lineNodesByScreenLineId.get(screenLine.id) + return oldProps.lineComponentsByScreenLineId.get(screenLine.id).element } updateBlockDecorations (oldProps, newProps) { - var {blockDecorations, lineNodesByScreenLineId} = newProps + var {blockDecorations, lineComponentsByScreenLineId} = newProps if (oldProps.blockDecorations) { oldProps.blockDecorations.forEach((oldDecorations, screenLineId) => { @@ -3766,7 +3761,7 @@ class LinesTileComponent { if (oldDecorations && oldDecorations.includes(newDecoration)) continue var element = TextEditor.viewForItem(newDecoration.item) - var lineNode = lineNodesByScreenLineId.get(screenLineId) + var lineNode = lineComponentsByScreenLineId.get(screenLineId).element if (newDecoration.position === 'after') { this.element.insertBefore(element, lineNode.nextSibling) } else { @@ -3878,11 +3873,11 @@ class LinesTileComponent { class LineComponent { constructor (props) { - const {nodePool, screenRow, screenLine, lineNodesByScreenLineId, offScreen} = props + const {nodePool, screenRow, screenLine, lineComponentsByScreenLineId, offScreen} = props this.props = props this.element = nodePool.getElement('DIV', this.buildClassName(), null) this.element.dataset.screenRow = screenRow - lineNodesByScreenLineId.set(screenLine.id, this.element) + this.textNodes = [] if (offScreen) { this.element.style.position = 'absolute' @@ -3891,6 +3886,7 @@ class LineComponent { } this.appendContents() + lineComponentsByScreenLineId.set(screenLine.id, this) } update (newProps) { @@ -3912,10 +3908,10 @@ class LineComponent { } destroy () { - const {nodePool, lineNodesByScreenLineId, textNodesByScreenLineId, screenLine} = this.props - if (lineNodesByScreenLineId.get(screenLine.id) === this.element) { - lineNodesByScreenLineId.delete(screenLine.id) - textNodesByScreenLineId.delete(screenLine.id) + const {nodePool, lineComponentsByScreenLineId, screenLine} = this.props + + if (lineComponentsByScreenLineId.get(screenLine.id) === this) { + lineComponentsByScreenLineId.delete(screenLine.id) } this.element.remove() @@ -3923,10 +3919,9 @@ class LineComponent { } appendContents () { - const {displayLayer, nodePool, screenLine, textDecorations, textNodesByScreenLineId} = this.props + const {displayLayer, nodePool, screenLine, textDecorations} = this.props - const textNodes = [] - textNodesByScreenLineId.set(screenLine.id, textNodes) + this.textNodes.length = 0 const {lineText, tags} = screenLine let openScopeNode = nodePool.getElement('SPAN', null, null) @@ -3957,7 +3952,7 @@ class LineComponent { const nextTokenColumn = column + tag while (nextDecoration && nextDecoration.column <= nextTokenColumn) { const text = lineText.substring(column, nextDecoration.column) - this.appendTextNode(textNodes, openScopeNode, text, activeClassName, activeStyle) + this.appendTextNode(openScopeNode, text, activeClassName, activeStyle) column = nextDecoration.column activeClassName = nextDecoration.className activeStyle = nextDecoration.style @@ -3966,7 +3961,7 @@ class LineComponent { if (column < nextTokenColumn) { const text = lineText.substring(column, nextTokenColumn) - this.appendTextNode(textNodes, openScopeNode, text, activeClassName, activeStyle) + this.appendTextNode(openScopeNode, text, activeClassName, activeStyle) column = nextTokenColumn } } @@ -3976,7 +3971,7 @@ class LineComponent { if (column === 0) { const textNode = nodePool.getTextNode(' ') this.element.appendChild(textNode) - textNodes.push(textNode) + this.textNodes.push(textNode) } if (lineText.endsWith(displayLayer.foldCharacter)) { @@ -3985,11 +3980,11 @@ class LineComponent { // measurements when such marker is the last character on the line. const textNode = nodePool.getTextNode(ZERO_WIDTH_NBSP_CHARACTER) this.element.appendChild(textNode) - textNodes.push(textNode) + this.textNodes.push(textNode) } } - appendTextNode (textNodes, openScopeNode, text, activeClassName, activeStyle) { + appendTextNode (openScopeNode, text, activeClassName, activeStyle) { const {nodePool} = this.props if (activeClassName || activeStyle) { @@ -4000,7 +3995,7 @@ class LineComponent { const textNode = nodePool.getTextNode(text) openScopeNode.appendChild(textNode) - textNodes.push(textNode) + this.textNodes.push(textNode) } buildClassName () {