feat(transforms): Replace regexp body transforms with DOM approach

Summary:
We originally didn't do this because creating a DOM tree was loading images.
Using range.createContextualFragment seems to do it without the tree ever
being attached.

Accompanying changes to src/pro are here:
https://phab.nylas.com/D3300
https://github.com/nylas/edgehill/compare/bengotow/draft-dom-transformations?expand=1

Also rename applyTransformsToDraft => applyTransformsForSending. Needed
a new name because the function signature has changed. AFAIK there are no
open source plugins using the old functions.

Test Plan: All specs updated

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3299

docs/ComposerExtensions.md

@@ -35,12 +35,9 @@ class ProductsExtension extends ComposerExtension
         	return ["with the word '#{word}'?"]
 	  return []
 
-   @applyTransformsToDraft: ({draft}) ->
+   @applyTransformsForSending: ({draftBodyRootNode, draft}) ->
     if @warningsForSending({draft})
-      updated = draft.clone()
-      updated.body += "<br>This email \
-       	contains competitor's product names \
-      	or trademarks used in context."
-      return updated
-    return draft
+      el = document.createElement('p');
+      el.innerText = "This email contains competitor's product names or trademarks used in context."
+      draftBodyRootNode.appendChild(el)
 ```

internal_packages/composer-emoji/lib/emoji-composer-extension.jsx

@@ -133,20 +133,38 @@ class EmojiComposerExtension extends ComposerExtension {
     return null;
   };
 
-  static applyTransformsToDraft = ({draft}) => {
-    const nextDraft = draft.clone();
-    nextDraft.body = nextDraft.body.replace(/<img class="emoji ([a-zA-Z0-9-_]*)" [^<]+>/g, (match, emojiName) =>
-      emoji.get(emojiName)
-    );
-    return nextDraft;
+  static applyTransformsForSending = ({draftBodyRootNode}) => {
+    const imgs = draftBodyRootNode.querySelectorAll('img')
+    for (const imgEl of Array.from(imgs)) {
+      const names = imgEl.className.split(' ');
+      if (names[0] === 'emoji') {
+        const emojiChar = emoji.get(names[1]);
+        if (emojiChar) {
+          imgEl.parentNode.replaceChild(document.createTextNode(emojiChar), imgEl);
+        }
+      }
+    }
   }
 
-  static unapplyTransformsToDraft = ({draft}) => {
-    const nextDraft = draft.clone();
-    nextDraft.body = nextDraft.body.replace(RegExpUtils.emojiRegex(), (match) =>
-      `<img class="emoji ${emoji.which(match)}" src="${EmojiStore.getImagePath(emoji.which(match))}" width="14" height="14" style="margin-top: -5px;">`
-    );
-    return nextDraft;
+  static unapplyTransformsForSending = ({draftBodyRootNode}) => {
+    const treeWalker = document.createTreeWalker(draftBodyRootNode, NodeFilter.SHOW_TEXT);
+    while (treeWalker.nextNode()) {
+      const textNode = treeWalker.currentNode;
+      const match = RegExpUtils.emojiRegex().exec(textNode.textContent);
+      if (match) {
+        const emojiPlusTrailingEl = textNode.splitText(match.index);
+        emojiPlusTrailingEl.splitText(match.length);
+        const emojiEl = emojiPlusTrailingEl;
+        const imgEl = document.createElement('img');
+        const emojiName = emoji.which(match[0])
+        imgEl.className = `emoji ${emojiName}`;
+        imgEl.src = EmojiStore.getImagePath(emojiName);
+        imgEl.width = '14';
+        imgEl.height = '14';
+        imgEl.style.marginTop = '-5px';
+        emojiEl.parentNode.replaceChild(imgEl, emojiEl);
+      }
+    }
   }
 
   static _findEmojiOptions(sel) {

internal_packages/composer-markdown/lib/markdown-composer-extension.coffee

@@ -2,20 +2,16 @@ marked = require 'marked'
 Utils = require './utils'
 {ComposerExtension} = require 'nylas-exports'
 
-
 rawBodies = {}
 
 class MarkdownComposerExtension extends ComposerExtension
 
-  @applyTransformsToDraft: ({draft}) ->
-    nextDraft = draft.clone()
-    rawBodies[draft.clientId] = nextDraft.body
-    nextDraft.body = marked(Utils.getTextFromHtml(draft.body))
-    return nextDraft
+  @applyTransformsForSending: ({draftBodyRootNode, draft}) ->
+    rawBodies[draft.clientId] = draftBodyRootNode.innerHTML
+    draftBodyRootNode.innerHTML = marked(draftBodyRootNode.innerText)
 
-  @unapplyTransformsToDraft: ({draft}) ->
-    nextDraft = draft.clone()
-    nextDraft.body = rawBodies[nextDraft.clientId] ? nextDraft.body
-    return nextDraft
+  @unapplyTransformsForSending: ({draftBodyRootNode, draft}) ->
+    if rawBodies[draft.clientId]
+      draftBodyRootNode.innerHTML = rawBodies[draft.clientId]
 
 module.exports = MarkdownComposerExtension

internal_packages/composer-spellcheck/lib/spellcheck-composer-extension.es6

@@ -163,11 +163,19 @@ export default class SpellcheckComposerExtension extends ComposerExtension {
     });
   }
 
-  static applyTransformsToDraft = ({draft}) => {
-    const nextDraft = draft.clone();
-    nextDraft.body = nextDraft.body.replace(/<\/?spelling[^>]*>/g, '');
-    return nextDraft;
+  static applyTransformsForSending = ({draftBodyRootNode}) => {
+    const spellingEls = draftBodyRootNode.querySelectorAll('spelling');
+    for (const spellingEl of Array.from(spellingEls)) {
+      // move contents out of the spelling node, remove the node
+      const parent = spellingEl.parentNode;
+      while (spellingEl.firstChild) {
+        parent.insertBefore(spellingEl.firstChild, spellingEl);
+      }
+      parent.removeChild(spellingEl);
+    }
   }
 
-  static unapplyTransformsToDraft = () => 'unnecessary'
+  static unapplyTransformsForSending = () => {
+    // no need to put spelling nodes back!
+  }
 }

internal_packages/composer-spellcheck/spec/spellcheck-composer-extension-spec.es6

@@ -8,8 +8,8 @@ import {NylasSpellchecker, Message} from 'nylas-exports';
 
 const initialPath = path.join(__dirname, 'fixtures', 'california-with-misspellings-before.html');
 const initialHTML = fs.readFileSync(initialPath).toString();
-const expectedPath = path.join(__dirname, 'fixtures', 'california-with-misspellings-after.html');
-const expectedHTML = fs.readFileSync(expectedPath).toString();
+const afterPath = path.join(__dirname, 'fixtures', 'california-with-misspellings-after.html');
+const afterHTML = fs.readFileSync(afterPath).toString();
 
 describe('SpellcheckComposerExtension', function spellcheckComposerExtension() {
   beforeEach(() => {
@@ -30,23 +30,19 @@ describe('SpellcheckComposerExtension', function spellcheckComposerExtension() {
       };
 
       SpellcheckComposerExtension.update(editor);
-      expect(node.innerHTML).toEqual(expectedHTML);
+      expect(node.innerHTML).toEqual(afterHTML);
     });
   });
 
-  describe("applyTransformsToDraft", () => {
+  describe("applyTransformsForSending", () => {
     it("removes the spelling annotations it inserted", () => {
-      const draft = new Message({ body: expectedHTML });
-      const out = SpellcheckComposerExtension.applyTransformsToDraft({draft});
-      expect(out.body).toEqual(initialHTML);
-    });
-  });
-
-  describe("unapplyTransformsToDraft", () => {
-    it("returns the magic no-op option", () => {
-      const draft = new Message({ body: expectedHTML });
-      const out = SpellcheckComposerExtension.unapplyTransformsToDraft({draft});
-      expect(out).toEqual('unnecessary');
+      const draft = new Message({ body: afterHTML });
+      const fragment = document.createDocumentFragment();
+      const draftBodyRootNode = document.createElement('root')
+      fragment.appendChild(draftBodyRootNode)
+      draftBodyRootNode.innerHTML = afterHTML
+      SpellcheckComposerExtension.applyTransformsForSending({draftBodyRootNode, draft});
+      expect(draftBodyRootNode.innerHTML).toEqual(initialHTML);
     });
   });
 });

internal_packages/composer-templates/lib/template-composer-extension.es6

@@ -10,20 +10,16 @@ export default class TemplatesComposerExtension extends ComposerExtension {
     return warnings;
   }
 
-  static applyTransformsToDraft = ({draft}) => {
-    const nextDraft = draft.clone();
-    nextDraft.body = nextDraft.body.replace(/<\/?code[^>]*>/g, (match) =>
+  static applyTransformsForSending = ({draftBodyRootNode}) => {
+    draftBodyRootNode.innerHTML = draftBodyRootNode.innerHTML.replace(/<\/?code[^>]*>/g, (match) =>
       `<!-- ${match} -->`
     );
-    return nextDraft;
   }
 
-  static unapplyTransformsToDraft = ({draft}) => {
-    const nextDraft = draft.clone();
-    nextDraft.body = nextDraft.body.replace(/<!-- (<\/?code[^>]*>) -->/g, (match, node) =>
+  static unapplyTransformsForSending = ({draftBodyRootNode}) => {
+    draftBodyRootNode.innerHTML = draftBodyRootNode.innerHTML.replace(/<!-- (<\/?code[^>]*>) -->/g, (match, node) =>
       node
     );
-    return nextDraft;
   }
 
   static onClick({editor, event}) {

internal_packages/composer/spec/composer-view-spec.cjsx

@@ -180,7 +180,7 @@ describe "ComposerView", ->
 
     describe "empty body warning", ->
       it "warns if the body of the email is still the pristine body", ->
-        pristineBody = "<head></head><body><br><br></body>"
+        pristineBody = "<br><br>"
 
         useDraft.call @,
           to: [u1]

spec/stores/draft-helpers-spec.es6

@@ -8,7 +8,7 @@ import {
 describe('DraftHelpers', function describeBlock() {
   describe('prepareDraftForSyncback', () => {
     beforeEach(() => {
-      spyOn(DraftHelpers, 'applyExtensionTransformsToDraft').andCallFake((draft) => Promise.resolve(draft))
+      spyOn(DraftHelpers, 'applyExtensionTransforms').andCallFake((draft) => Promise.resolve(draft))
       spyOn(Actions, 'queueTask')
     });
 

src/components/overlaid-components/overlaid-composer-extension.es6

@@ -1,88 +1,41 @@
 import React from 'react'
 import ReactDOMServer from 'react-dom/server'
 import ComposerExtension from '../../extensions/composer-extension'
-// import {ANCHOR_CLASS, IMG_SRC} from './anchor-constants'
 import OverlaidComponents from './overlaid-components'
 import CustomContenteditableComponents from './custom-contenteditable-components'
 
+// In this code, "anchor" refers to the "img" tag used in the draft while the
+// user is editing it.
+
+// <overlay> is used when the draft is sent.
 export default class OverlaidComposerExtension extends ComposerExtension {
 
-  // https://regex101.com/r/fW6sV3/2
-  static _serializedExtractRe() {
-    return /<overlay .*?data-overlay-id="(.*?)" data-component-props="(.*?)" data-component-key="(.*?)" data-style="(.*?)".*?>.*?<\/overlay>/gmi
-  }
-
-  static _serializedReplacerRe(id) {
-    return new RegExp(`<overlay .*?data-overlay-id="${id}".*?>.*?<\/overlay>`, 'gim')
-  }
-
-  // https://regex101.com/r/rK3uA3/1
-  static _anchorExtractRe() {
-    return /<img .*?data-overlay-id="(.*?)" data-component-props="(.*?)" data-component-key="(.*?)" style="(.*?)".*?>/gmi
-  }
-
-  static _anchorReplacerRe(id) {
-    return new RegExp(`<img .*?data-overlay-id="${id}".*?>`, 'gim')
-  }
-
-  static *overlayMatches(re, body) {
-    let result = re.exec(body);
-    while (result) {
-      let props = result[2];
-      props = JSON.parse(props.replace(/&quot;/g, `"`));
-      const data = {
-        dataOverlayId: result[1],
-        dataComponentProps: props,
-        dataComponentKey: result[3],
-        dataStyle: result[4],
+  static applyTransformsForSending({draftBodyRootNode, draft}) {
+    const overlayImgEls = Array.from(draftBodyRootNode.querySelectorAll('img[data-overlay-id]'));
+    for (const imgEl of overlayImgEls) {
+      const Component = CustomContenteditableComponents.get(imgEl.dataset.componentKey);
+      if (!Component) {
+        continue;
       }
-      yield data
-      result = re.exec(body);
+
+      const props = Object.assign({draft, isPreview: true}, imgEl.dataset.componentProps);
+      const reactElement = React.createElement(Component, props);
+
+      const overlayEl = document.createElement('overlay');
+      overlayEl.innerHTML = ReactDOMServer.renderToStaticMarkup(reactElement);
+      Object.assign(overlayEl.dataset, imgEl.dataset);
+
+      imgEl.parentNode.replaceChild(overlayEl, imgEl);
     }
-    return
   }
 
-  static applyTransformsToDraft({draft}) {
-    const self = OverlaidComposerExtension;
-    const outDraft = draft.clone();
-    let outBody = outDraft.body;
-    const matcher = self.overlayMatches(self._anchorExtractRe(), outDraft.body)
-
-    for (const match of matcher) {
-      const component = CustomContenteditableComponents.get(match.dataComponentKey);
-      if (!component) {
-        continue
-      }
-      const props = Object.assign({draft, isPreview: true}, match.dataComponentProps);
-      const el = React.createElement(component, props);
-      let html = ReactDOMServer.renderToStaticMarkup(el);
-
-      html = `<overlay data-overlay-id="${match.dataOverlayId}" data-component-props="${OverlaidComponents.propsToDOMAttr(match.dataComponentProps)}" data-component-key="${match.dataComponentKey}" data-style="${match.dataStyle}">${html}</overlay>`
-
-      outBody = outBody.replace(
-        OverlaidComposerExtension._anchorReplacerRe(match.dataOverlayId),
-        html
-      )
+  static unapplyTransformsForSending({draftBodyRootNode}) {
+    const overlayEls = Array.from(draftBodyRootNode.querySelectorAll('overlay[data-overlay-id]'));
+    for (const overlayEl of overlayEls) {
+      const {componentKey, componentProps, overlayId, style} = overlayEl.dataset;
+      const {anchorTag} = OverlaidComponents.buildAnchorTag(componentKey, componentProps, overlayId, style);
+      const anchorFragment = document.createRange().createContextualFragment(anchorTag);
+      overlayEl.parentNode.replaceChild(anchorFragment, overlayEl);
     }
-
-    outDraft.body = outBody;
-    return outDraft;
-  }
-
-  static unapplyTransformsToDraft({draft}) {
-    const self = OverlaidComposerExtension;
-    const outDraft = draft.clone();
-    let outBody = outDraft.body
-
-    const matcher = self.overlayMatches(self._serializedExtractRe(), outDraft.body);
-
-    for (const match of matcher) {
-      const {anchorTag} = OverlaidComponents.buildAnchorTag(match.dataComponentKey, match.dataComponentProps, match.dataOverlayId, match.dataStyle);
-
-      outBody = outBody.replace(OverlaidComposerExtension._serializedReplacerRe(match.dataOverlayId), anchorTag)
-    }
-
-    outDraft.body = outBody;
-    return outDraft;
   }
 }

src/extensions/composer-extension.coffee

@@ -142,15 +142,15 @@ class ComposerExtension extends ContenteditableExtension
 
   - `draft`: A {Message} the user is about to finish editing.
   ###
-  @applyTransformsToDraft: ({draft}) ->
-    return draft
+  @applyTransformsForSending: ({draft, draftBodyRootNode}) ->
+    return
 
   ###
   Public: unapplyTransformsToDraft should revert the changes made in
   `applyTransformsToDraft`. See the documentation for that method for more
   information.
   ###
-  @unapplyTransformsToDraft: ({draft}) ->
-    return draft
+  @unapplyTransformsForSending: ({draft, draftBodyRootNode}) ->
+    return
 
 module.exports = ComposerExtension

src/flux/stores/draft-editing-session.coffee

@@ -209,14 +209,21 @@ class DraftEditingSession
     if !draft.body?
       throw new Error("DraftEditingSession._setDraft - new draft has no body!")
 
-    # Reverse draft transformations performed by third-party plugins when the draft
-    # was last saved to disk
-    return Promise.each ExtensionRegistry.Composer.extensions(), (ext) ->
-      if ext.applyTransformsToDraft and ext.unapplyTransformsToDraft
-        Promise.resolve(ext.unapplyTransformsToDraft({draft})).then (untransformed) ->
-          unless untransformed is 'unnecessary'
-            draft = untransformed
+    extensions = ExtensionRegistry.Composer.extensions()
+
+    # Run `extensions[].unapplyTransformsForSending`
+    fragment = document.createDocumentFragment()
+    draftBodyRootNode = document.createElement('root')
+    fragment.appendChild(draftBodyRootNode)
+    draftBodyRootNode.innerHTML = draft.body
+
+    return Promise.each extensions, (ext) ->
+      if ext.applyTransformsForSending and ext.unapplyTransformsForSending
+        Promise.resolve(ext.unapplyTransformsForSending({
+          draftBodyRootNode: draftBodyRootNode,
+          draft: draft}))
     .then =>
+      draft.body = draftBodyRootNode.innerHTML
       @_draft = draft
 
       # We keep track of the draft's initial body if it's pristine when the editing

src/flux/stores/draft-helpers.es6

@@ -1,4 +1,3 @@
-import _ from 'underscore'
 import Actions from '../actions'
 import DatabaseStore from './database-store'
 import Message from '../models/message'
@@ -75,56 +74,48 @@ export function appendQuotedTextToDraft(draft) {
   })
 }
 
-export function applyExtensionTransformsToDraft(draft) {
-  let latestTransformed = draft
-  const extensions = ExtensionRegistry.Composer.extensions()
-  const transformPromise = (
-    Promise.each(extensions, (ext) => {
-      const extApply = ext.applyTransformsToDraft
-      const extUnapply = ext.unapplyTransformsToDraft
+export function applyExtensionTransforms(draft) {
+  const extensions = ExtensionRegistry.Composer.extensions();
 
-      if (!extApply || !extUnapply) {
-        return Promise.resolve()
-      }
+  const fragment = document.createDocumentFragment();
+  const draftBodyRootNode = document.createElement('root');
+  fragment.appendChild(draftBodyRootNode);
+  draftBodyRootNode.innerHTML = draft.body;
 
-      return Promise.resolve(extUnapply({draft: latestTransformed})).then((cleaned) => {
-        const base = cleaned === 'unnecessary' ? latestTransformed : cleaned;
-        return Promise.resolve(extApply({draft: base})).then((transformed) => (
-          Promise.resolve(extUnapply({draft: transformed.clone()})).then((reverted) => {
-            const untransformed = reverted === 'unnecessary' ? base : reverted;
-            if (!_.isEqual(_.pick(untransformed, AllowedTransformFields), _.pick(base, AllowedTransformFields))) {
-              console.log("-- BEFORE --")
-              console.log(base.body)
-              console.log("-- TRANSFORMED --")
-              console.log(transformed.body)
-              console.log("-- UNTRANSFORMED (should match BEFORE) --")
-              console.log(untransformed.body)
-              // FIXME: We're removing the error reporting for now, but the real fix is finding out why the console opens when dev mode is false.
-              // NylasEnv.reportError(new Error(`Extension ${ext.name} applied a transform to the draft that it could not reverse.`))
-            }
-            latestTransformed = transformed
-            return Promise.resolve()
-          })
-        ))
-      })
-    })
-  )
-  return transformPromise
-  .then(() => Promise.resolve(latestTransformed))
+  return Promise.each(extensions, (ext) => {
+    const extApply = ext.applyTransformsForSending;
+    const extUnapply = ext.unapplyTransformsForSending;
+
+    if (!extApply || !extUnapply) {
+      return Promise.resolve();
+    }
+
+    return Promise.resolve(extUnapply({draft, draftBodyRootNode})).then(() => {
+      return Promise.resolve(extApply({draft, draftBodyRootNode}));
+    });
+  }).then(() => {
+    draft.body = draftBodyRootNode.innerHTML;
+    return draft;
+  });
 }
 
 export function prepareDraftForSyncback(session) {
   return session.ensureCorrectAccount({noSyncback: true})
-  .then(() => applyExtensionTransformsToDraft(session.draft()))
+  .then(() =>
+    applyExtensionTransforms(session.draft()))
   .then((transformed) => {
     if (!transformed.replyToMessageId || !shouldAppendQuotedText(transformed)) {
-      return Promise.resolve(transformed)
+      return Promise.resolve(transformed);
     }
-    return appendQuotedTextToDraft(transformed)
+    return appendQuotedTextToDraft(transformed);
   })
   .then((draft) => (
-    DatabaseStore.inTransaction((t) => t.persistModel(draft))
-    .then(() => Promise.resolve(queueDraftFileUploads(draft)))
+    DatabaseStore.inTransaction((t) =>
+      t.persistModel(draft)
+    )
+    .then(() =>
+      Promise.resolve(queueDraftFileUploads(draft))
+    )
     .thenReturn(draft)
   ))
 }

src/pro

@@ -1 +1 @@
-Subproject commit 45756b80645edcedfa7d015dcff8a26bcb521ec0
+Subproject commit a5d7c9c6c1b13559d1bb997eece8c7a5f648ea5c