fix(atoms): Avoid rerendering atoms when section content changes. (#471)

fixes #421

src/js/models/atom-node.js

@@ -13,16 +13,13 @@ export default class AtomNode {
   }
 
   render() {
-    this.teardown();
-
-    let rendered = this.atom.render({
-      options: this.atomOptions,
-      env: this.env,
-      value: this.model.value,
-      payload: this.model.payload
-    });
+    if (!this._rendered) {
+      let {atomOptions: options, env, model: { value, payload } } = this;
+      // cache initial render
+      this._rendered = this.atom.render({options, env, value, payload});
+    }
 
-    this._validateAndAppendRenderResult(rendered);
+    this._validateAndAppendRenderResult(this._rendered);
   }
 
   get env() {
@@ -54,7 +51,5 @@ export default class AtomNode {
       !!rendered.nodeType
     );
     this.element.appendChild(rendered);
-    this._rendered = rendered;
   }
-
 }

src/js/renderers/editor-dom.js

@@ -449,9 +449,14 @@ class Visitor {
     } = renderAtom(atomModel, parentElement, renderNode.prev);
     const atom = this._findAtom(atomModel.name);
 
-    const atomNode = new AtomNode(
-      editor, atom, atomModel, atomElement, options
-    );
+    let atomNode = renderNode.atomNode;
+    if (!atomNode) {
+      // create new AtomNode
+      atomNode = new AtomNode(editor, atom, atomModel, atomElement, options);
+    } else {
+      // retarget atomNode to new atom element
+      atomNode.element = atomElement;
+    }
 
     atomNode.render();
 

tests/helpers/post-abstract.js

@@ -65,13 +65,15 @@ function parsePositionOffsets(text) {
   return offsets;
 }
 
+const DEFAULT_ATOM_NAME = 'some-atom';
+
 function parseTextIntoMarkers(text, builder) {
   text = text.replace(cursorRegex,'');
   let markers = [];
 
   if (text.indexOf('@') !== -1) {
     let atomIndex = text.indexOf('@');
-    let atom = builder.atom('some-atom');
+    let atom = builder.atom(DEFAULT_ATOM_NAME);
     let pieces = [text.slice(0, atomIndex), atom, text.slice(atomIndex+1)];
     pieces.forEach(piece => {
       if (piece === atom) {
@@ -211,5 +213,6 @@ function buildFromText(texts) {
 
 export default {
   build,
-  buildFromText
+  buildFromText,
+  DEFAULT_ATOM_NAME
 };

tests/unit/editor/atom-lifecycle-test.js

@@ -5,6 +5,7 @@ let editorElement, editor;
 import { MOBILEDOC_VERSION as MOBILEDOC_VERSION_0_3 } from 'mobiledoc-kit/renderers/mobiledoc/0-3';
 
 const { module, test } = Helpers;
+const { postAbstract: { DEFAULT_ATOM_NAME } } = Helpers;
 
 module('Unit: Editor: Atom Lifecycle', {
   beforeEach() {
@@ -18,10 +19,11 @@ module('Unit: Editor: Atom Lifecycle', {
   }
 });
 
-
-function makeEl(id) {
+function makeEl(id, text='@atom') {
   let el = document.createElement('span');
   el.id = id;
+  text = document.createTextNode(text);
+  el.appendChild(text);
   return el;
 }
 
@@ -231,15 +233,63 @@ test('onTeardown hook is called when editor is destroyed', (assert) => {
   assert.ok(teardown, 'onTeardown hook called');
 });
 
+test('onTeardown hook is called when atom is destroyed', (assert) => {
+  let teardown;
+
+  let atom = {
+    name: DEFAULT_ATOM_NAME,
+    type: 'dom',
+    render({env}) {
+      env.onTeardown(() => teardown = true);
+      return makeEl('atom-id','atom-text');
+    }
+  };
+  editor = Helpers.editor.buildFromText('abc@d|ef', {autofocus: true, atoms:[atom], element: editorElement});
+  assert.hasElement('#editor #atom-id:contains(atom-text)', 'precond - shows atom');
+  assert.ok(!teardown, 'precond - no teardown yet');
+  Helpers.dom.triggerDelete(editor);
+
+  assert.hasElement('#editor #atom-id:contains(atom-text)', 'precond - still shows atom');
+  assert.ok(!teardown, 'precond - no teardown yet');
+  Helpers.dom.triggerDelete(editor);
+
+  assert.hasNoElement('*:contains(atom-text)', 'atom destroyed');
+  assert.ok(teardown, 'calls teardown');
+});
+
+// See https://github.com/bustlelabs/mobiledoc-kit/issues/421
+test('render is not called again when modifying other parts of the section', (assert) => {
+  let renderCount = 0;
+  let atom = {
+    name: DEFAULT_ATOM_NAME,
+    type: 'dom',
+    render() {
+      renderCount++;
+      return makeEl('the-atom');
+    }
+  };
+  editor = Helpers.editor.buildFromText('abc|@def', {autofocus: true, atoms:[atom], element: editorElement});
+  assert.equal(renderCount, 1, 'renders the atom initially');
+  editor.insertText('123');
+  assert.hasElement('#editor *:contains(abc123)', 'precond - inserts text');
+  assert.equal(renderCount, 1, 'does not rerender the atom');
+});
+
 test('mutating the content of an atom does not trigger an update', (assert) => {
+  assert.expect(5);
   const done = assert.async();
 
   const atomName = 'test-atom';
 
+  let renderCount = 0;
+  let teardown;
+
   const atom = {
     name: atomName,
     type: 'dom',
-    render() {
+    render({env}) {
+      renderCount++;
+      env.onTeardown(() => teardown = true);
       return makeEl('the-atom');
     }
   };
@@ -254,12 +304,15 @@ test('mutating the content of an atom does not trigger an update', (assert) => {
 
   assert.hasNoElement('#editor #the-atom', 'precond - atom not rendered');
   editor.render(editorElement);
+  assert.equal(renderCount, 1, 'renders atom');
 
   $("#the-atom").html("updated");
 
   // ensure the mutations have had time to trigger
   Helpers.wait(function(){
     assert.ok(!updateTriggered);
+    assert.equal(renderCount, 1, 'does not rerender atom');
+    assert.ok(!teardown, 'does not teardown atom');
     done();
   });
 });