Recycle DOM nodes

spec/dom-element-pool-spec.coffee

@@ -0,0 +1,55 @@
+DOMElementPool = require '../src/dom-element-pool'
+
+describe "DOMElementPool", ->
+  domElementPool = null
+
+  beforeEach ->
+    domElementPool = new DOMElementPool
+
+  it "builds DOM nodes, recycling them when they are freed", ->
+    [div, span1, span2, span3, span4, span5] = elements = [
+      domElementPool.build("div")
+      domElementPool.build("span")
+      domElementPool.build("span")
+      domElementPool.build("span")
+      domElementPool.build("span")
+      domElementPool.build("span")
+    ]
+
+    div.appendChild(span1)
+    span1.appendChild(span2)
+    div.appendChild(span3)
+    span3.appendChild(span4)
+
+    domElementPool.freeElementAndDescendants(div)
+    domElementPool.freeElementAndDescendants(span5)
+
+    expect(elements).toContain(domElementPool.build("div"))
+    expect(elements).toContain(domElementPool.build("span"))
+    expect(elements).toContain(domElementPool.build("span"))
+    expect(elements).toContain(domElementPool.build("span"))
+    expect(elements).toContain(domElementPool.build("span"))
+    expect(elements).toContain(domElementPool.build("span"))
+
+    expect(elements).not.toContain(domElementPool.build("div"))
+    expect(elements).not.toContain(domElementPool.build("span"))
+
+  it "forgets free nodes after being cleared", ->
+    span = domElementPool.build("span")
+    div = domElementPool.build("div")
+    domElementPool.freeElementAndDescendants(span)
+    domElementPool.freeElementAndDescendants(div)
+
+    domElementPool.clear()
+
+    expect(domElementPool.build("span")).not.toBe(span)
+    expect(domElementPool.build("div")).not.toBe(div)
+
+  it "throws an error when trying to free the same node twice", ->
+    div = domElementPool.build("div")
+    domElementPool.freeElementAndDescendants(div)
+    expect(-> domElementPool.freeElementAndDescendants(div)).toThrow()
+
+  it "throws an error when trying to free an invalid element", ->
+    expect(-> domElementPool.freeElementAndDescendants(null)).toThrow()
+    expect(-> domElementPool.freeElementAndDescendants(undefined)).toThrow()

spec/gutter-container-component-spec.coffee

@@ -1,5 +1,6 @@
 Gutter = require '../src/gutter'
 GutterContainerComponent = require '../src/gutter-container-component'
+DOMElementPool = require '../src/dom-element-pool'
 
 describe "GutterContainerComponent", ->
   [gutterContainerComponent] = []
@@ -22,9 +23,10 @@ describe "GutterContainerComponent", ->
     mockTestState
 
   beforeEach ->
+    domElementPool = new DOMElementPool
     mockEditor = {}
     mockMouseDown = ->
-    gutterContainerComponent = new GutterContainerComponent({editor: mockEditor, onMouseDown: mockMouseDown})
+    gutterContainerComponent = new GutterContainerComponent({editor: mockEditor, onMouseDown: mockMouseDown, domElementPool})
 
   it "creates a DOM node with no child gutter nodes when it is initialized", ->
     expect(gutterContainerComponent.getDomNode() instanceof HTMLElement).toBe true

spec/text-editor-component-spec.coffee

@@ -986,10 +986,11 @@ describe "TextEditorComponent", ->
       cursor = componentNode.querySelector('.cursor')
       cursorRect = cursor.getBoundingClientRect()
 
-      cursorLocationTextNode = component.lineNodeForScreenRow(0).querySelector('.source.js').firstChild
+      cursorLocationTextNode = component.lineNodeForScreenRow(0).querySelector('.source.js').childNodes[2]
+
       range = document.createRange()
-      range.setStart(cursorLocationTextNode, 3)
-      range.setEnd(cursorLocationTextNode, 4)
+      range.setStart(cursorLocationTextNode, 0)
+      range.setEnd(cursorLocationTextNode, 1)
       rangeRect = range.getBoundingClientRect()
 
       expect(cursorRect.left).toBe rangeRect.left

spec/text-utils-spec.coffee

@@ -17,9 +17,6 @@ describe 'text utilities', ->
       expect(textUtils.hasPairedCharacter('\uFE0E\uFE0E')).toBe false
       expect(textUtils.hasPairedCharacter('\u0301\u0301')).toBe false
 
-      expect(textUtils.hasPairedCharacter('\0\u0301')).toBe false
-      expect(textUtils.hasPairedCharacter('\0\uFE0E')).toBe false
-
   describe '.isPairedCharacter(string, index)', ->
     it 'returns true when the index is the start of a high/low surrogate pair, variation sequence, or combined character', ->
       expect(textUtils.isPairedCharacter('a\uD835\uDF97b\uD835\uDF97c', 0)).toBe false
@@ -47,6 +44,3 @@ describe 'text utilities', ->
       expect(textUtils.isPairedCharacter('ae\u0301c', 2)).toBe false
       expect(textUtils.isPairedCharacter('ae\u0301c', 3)).toBe false
       expect(textUtils.isPairedCharacter('ae\u0301c', 4)).toBe false
-
-      expect(textUtils.isPairedCharacter('\0\u0301c', 0)).toBe false
-      expect(textUtils.isPairedCharacter('\0\uFE0E', 0)).toBe false

src/dom-element-pool.coffee

@@ -0,0 +1,40 @@
+module.exports =
+class DOMElementPool
+  constructor: ->
+    @freeElementsByTagName = {}
+    @freedElements = new Set
+
+  clear: ->
+    @freedElements.clear()
+    for tagName, freeElements of @freeElementsByTagName
+      freeElements.length = 0
+
+  build: (tagName, className, textContent = "") ->
+    element = @freeElementsByTagName[tagName]?.pop()
+    element ?= document.createElement(tagName)
+    delete element.dataset[dataId] for dataId of element.dataset
+    element.removeAttribute("class")
+    element.removeAttribute("style")
+    element.className = className if className?
+    element.textContent = textContent
+
+    @freedElements.delete(element)
+
+    element
+
+  freeElementAndDescendants: (element) ->
+    @free(element)
+    for index in [element.children.length - 1..0] by -1
+      child = element.children[index]
+      @freeElementAndDescendants(child)
+
+  free: (element) ->
+    throw new Error("The element cannot be null or undefined.") unless element?
+    throw new Error("The element has already been freed!") if @freedElements.has(element)
+
+    tagName = element.tagName.toLowerCase()
+    @freeElementsByTagName[tagName] ?= []
+    @freeElementsByTagName[tagName].push(element)
+    @freedElements.add(element)
+
+    element.remove()

src/gutter-container-component.coffee

@@ -7,7 +7,7 @@ LineNumberGutterComponent = require './line-number-gutter-component'
 
 module.exports =
 class GutterContainerComponent
-  constructor: ({@onLineNumberGutterMouseDown, @editor}) ->
+  constructor: ({@onLineNumberGutterMouseDown, @editor, @domElementPool}) ->
     # An array of objects of the form: {name: {String}, component: {Object}}
     @gutterComponents = []
     @gutterComponentsByGutterName = {}
@@ -39,7 +39,7 @@ class GutterContainerComponent
       gutterComponent = @gutterComponentsByGutterName[gutter.name]
       if not gutterComponent
         if gutter.name is 'line-number'
-          gutterComponent = new LineNumberGutterComponent({onMouseDown: @onLineNumberGutterMouseDown, @editor, gutter})
+          gutterComponent = new LineNumberGutterComponent({onMouseDown: @onLineNumberGutterMouseDown, @editor, gutter, @domElementPool})
           @lineNumberGutterComponent = gutterComponent
         else
           gutterComponent = new CustomGutterComponent({gutter})

src/highlights-component.coffee

@@ -5,12 +5,11 @@ module.exports =
 class HighlightsComponent
   oldState: null
 
-  constructor: ->
+  constructor: (@domElementPool) ->
     @highlightNodesById = {}
     @regionNodesByHighlightId = {}
 
-    @domNode = document.createElement('div')
-    @domNode.classList.add('highlights')
+    @domNode = @domElementPool.build("div", "highlights")
 
   getDomNode: ->
     @domNode
@@ -30,8 +29,7 @@ class HighlightsComponent
     # add or update highlights
     for id, highlightState of newState
       unless @oldState[id]?
-        highlightNode = document.createElement('div')
-        highlightNode.classList.add('highlight')
+        highlightNode = @domElementPool.build("div", "highlight")
         @highlightNodesById[id] = highlightNode
         @regionNodesByHighlightId[id] = {}
         @domNode.appendChild(highlightNode)
@@ -75,12 +73,11 @@ class HighlightsComponent
     for newRegionState, i in newHighlightState.regions
       unless oldHighlightState.regions[i]?
         oldHighlightState.regions[i] = {}
-        regionNode = document.createElement('div')
+        regionNode = @domElementPool.build("div", "region")
         # This prevents highlights at the tiles boundaries to be hidden by the
         # subsequent tile. When this happens, subpixel anti-aliasing gets
         # disabled.
         regionNode.style.boxSizing = "border-box"
-        regionNode.classList.add('region')
         regionNode.classList.add(newHighlightState.deprecatedRegionClass) if newHighlightState.deprecatedRegionClass?
         @regionNodesByHighlightId[id][i] = regionNode
         highlightNode.appendChild(regionNode)

src/line-number-gutter-component.coffee

@@ -1,15 +1,17 @@
 TiledComponent = require './tiled-component'
 LineNumbersTileComponent = require './line-numbers-tile-component'
 WrapperDiv = document.createElement('div')
-DummyLineNumberComponent = LineNumbersTileComponent.createDummy()
+DOMElementPool = require './dom-element-pool'
 
 module.exports =
 class LineNumberGutterComponent extends TiledComponent
   dummyLineNumberNode: null
 
-  constructor: ({@onMouseDown, @editor, @gutter}) ->
+  constructor: ({@onMouseDown, @editor, @gutter, @domElementPool}) ->
     @visible = true
 
+    @dummyLineNumberComponent = LineNumbersTileComponent.createDummy(@domElementPool)
+
     @domNode = atom.views.getView(@gutter)
     @lineNumbersNode = @domNode.firstChild
     @lineNumbersNode.innerHTML = ''
@@ -60,7 +62,7 @@ class LineNumberGutterComponent extends TiledComponent
       @oldState.styles = {}
       @oldState.maxLineNumberDigits = @newState.maxLineNumberDigits
 
-  buildComponentForTile: (id) -> new LineNumbersTileComponent({id})
+  buildComponentForTile: (id) -> new LineNumbersTileComponent({id, @domElementPool})
 
   ###
   Section: Private Methods
@@ -69,14 +71,13 @@ class LineNumberGutterComponent extends TiledComponent
   # This dummy line number element holds the gutter to the appropriate width,
   # since the real line numbers are absolutely positioned for performance reasons.
   appendDummyLineNumber: ->
-    DummyLineNumberComponent.newState = @newState
-    WrapperDiv.innerHTML = DummyLineNumberComponent.buildLineNumberHTML({bufferRow: -1})
-    @dummyLineNumberNode = WrapperDiv.children[0]
+    @dummyLineNumberComponent.newState = @newState
+    @dummyLineNumberNode = @dummyLineNumberComponent.buildLineNumberNode({bufferRow: -1})
     @lineNumbersNode.appendChild(@dummyLineNumberNode)
 
   updateDummyLineNumber: ->
-    DummyLineNumberComponent.newState = @newState
-    @dummyLineNumberNode.innerHTML = DummyLineNumberComponent.buildLineNumberInnerHTML(0, false)
+    @dummyLineNumberComponent.newState = @newState
+    @dummyLineNumberComponent.setLineNumberInnerNodes(0, false, @dummyLineNumberNode)
 
   onMouseDown: (event) =>
     {target} = event

src/line-numbers-tile-component.coffee

@@ -1,18 +1,20 @@
 _ = require 'underscore-plus'
-WrapperDiv = document.createElement('div')
 
 module.exports =
 class LineNumbersTileComponent
-  @createDummy: ->
-    new LineNumbersTileComponent({id: -1})
+  @createDummy: (domElementPool) ->
+    new LineNumbersTileComponent({id: -1, domElementPool})
 
-  constructor: ({@id}) ->
+  constructor: ({@id, @domElementPool}) ->
     @lineNumberNodesById = {}
-    @domNode = document.createElement("div")
+    @domNode = @domElementPool.build("div")
     @domNode.style.position = "absolute"
     @domNode.style.display = "block"
     @domNode.style.top = 0 # Cover the space occupied by a dummy lineNumber
 
+  destroy: ->
+    @domElementPool.freeElementAndDescendants(@domNode)
+
   getDomNode: ->
     @domNode
 
@@ -46,7 +48,9 @@ class LineNumbersTileComponent
       @oldTileState.zIndex = @newTileState.zIndex
 
     if @newState.maxLineNumberDigits isnt @oldState.maxLineNumberDigits
-      node.remove() for id, node of @lineNumberNodesById
+      for id, node of @lineNumberNodesById
+        @domElementPool.freeElementAndDescendants(node)
+
       @oldState.tiles[@id] = {lineNumbers: {}}
       @oldTileState = @oldState.tiles[@id]
       @lineNumberNodesById = {}
@@ -56,11 +60,11 @@ class LineNumbersTileComponent
 
   updateLineNumbers: ->
     newLineNumberIds = null
-    newLineNumbersHTML = null
+    newLineNumberNodes = null
 
     for id, lineNumberState of @oldTileState.lineNumbers
       unless @newTileState.lineNumbers.hasOwnProperty(id)
-        @lineNumberNodesById[id].remove()
+        @domElementPool.freeElementAndDescendants(@lineNumberNodesById[id])
         delete @lineNumberNodesById[id]
         delete @oldTileState.lineNumbers[id]
 
@@ -69,16 +73,13 @@ class LineNumbersTileComponent
         @updateLineNumberNode(id, lineNumberState)
       else
         newLineNumberIds ?= []
-        newLineNumbersHTML ?= ""
+        newLineNumberNodes ?= []
         newLineNumberIds.push(id)
-        newLineNumbersHTML += @buildLineNumberHTML(lineNumberState)
+        newLineNumberNodes.push(@buildLineNumberNode(lineNumberState))
         @oldTileState.lineNumbers[id] = _.clone(lineNumberState)
 
     return unless newLineNumberIds?
 
-    WrapperDiv.innerHTML = newLineNumbersHTML
-    newLineNumberNodes = _.toArray(WrapperDiv.children)
-
     for id, i in newLineNumberIds
       lineNumberNode = newLineNumberNodes[i]
       @lineNumberNodesById[id] = lineNumberNode
@@ -94,24 +95,30 @@ class LineNumbersTileComponent
 
   screenRowForNode: (node) -> parseInt(node.dataset.screenRow)
 
-  buildLineNumberHTML: (lineNumberState) ->
-    {screenRow, bufferRow, softWrapped, top, decorationClasses} = lineNumberState
+  buildLineNumberNode: (lineNumberState) ->
+    {screenRow, bufferRow, softWrapped, top, decorationClasses, zIndex} = lineNumberState
+
     className = @buildLineNumberClassName(lineNumberState)
-    innerHTML = @buildLineNumberInnerHTML(bufferRow, softWrapped)
+    lineNumberNode = @domElementPool.build("div", className)
+    lineNumberNode.dataset.screenRow = screenRow
+    lineNumberNode.dataset.bufferRow = bufferRow
 
-    "<div class=\"#{className}\" data-buffer-row=\"#{bufferRow}\" data-screen-row=\"#{screenRow}\">#{innerHTML}</div>"
+    @setLineNumberInnerNodes(bufferRow, softWrapped, lineNumberNode)
+    lineNumberNode
 
-  buildLineNumberInnerHTML: (bufferRow, softWrapped) ->
+  setLineNumberInnerNodes: (bufferRow, softWrapped, lineNumberNode) ->
     {maxLineNumberDigits} = @newState
 
     if softWrapped
       lineNumber = "•"
     else
       lineNumber = (bufferRow + 1).toString()
 
-    padding = _.multiplyString('&nbsp;', maxLineNumberDigits - lineNumber.length)
-    iconHTML = '<div class="icon-right"></div>'
-    padding + lineNumber + iconHTML
+    padding = _.multiplyString("\u00a0", maxLineNumberDigits - lineNumber.length)
+    iconRight = @domElementPool.build("div", "icon-right")
+
+    lineNumberNode.textContent = padding + lineNumber
+    lineNumberNode.appendChild(iconRight)
 
   updateLineNumberNode: (lineNumberId, newLineNumberState) ->
     oldLineNumberState = @oldTileState.lineNumbers[lineNumberId]
@@ -123,7 +130,7 @@ class LineNumbersTileComponent
       oldLineNumberState.decorationClasses = _.clone(newLineNumberState.decorationClasses)
 
     unless oldLineNumberState.screenRow is newLineNumberState.screenRow and oldLineNumberState.bufferRow is newLineNumberState.bufferRow
-      node.innerHTML = @buildLineNumberInnerHTML(newLineNumberState.bufferRow, newLineNumberState.softWrapped)
+      @setLineNumberInnerNodes(newLineNumberState.bufferRow, newLineNumberState.softWrapped, node)
       node.dataset.screenRow = newLineNumberState.screenRow
       node.dataset.bufferRow = newLineNumberState.bufferRow
       oldLineNumberState.screenRow = newLineNumberState.screenRow

src/lines-component.coffee

@@ -10,7 +10,7 @@ module.exports =
 class LinesComponent extends TiledComponent
   placeholderTextDiv: null
 
-  constructor: ({@presenter, @hostElement, @useShadowDOM, visible}) ->
+  constructor: ({@presenter, @hostElement, @useShadowDOM, visible, @domElementPool}) ->
     @domNode = document.createElement('div')
     @domNode.classList.add('lines')
     @tilesNode = document.createElement("div")
@@ -60,7 +60,7 @@ class LinesComponent extends TiledComponent
 
     @oldState.indentGuidesVisible = @newState.indentGuidesVisible
 
-  buildComponentForTile: (id) -> new LinesTileComponent({id, @presenter})
+  buildComponentForTile: (id) -> new LinesTileComponent({id, @presenter, @domElementPool})
 
   buildEmptyState: ->
     {tiles: {}}