Repository: zestedesavoir/zmarkdown
Pull request #302 (2018-12-02 16:43:20)

remark-abbr: remove empty p children in correct order
Previously when we had to remove parent.children[17] and parent.children[21] we would
parent.children.splice(17, 1) and then parent.children.splice(21, 1), this later call removing
index 20 from the original parent.children since it got shifted by any previous .splice call.

#289


Patch:
diff --git a/packages/remark-abbr/dist/index.js b/packages/remark-abbr/dist/index.js
index cd900792..96c934ba 100644
--- a/packages/remark-abbr/dist/index.js
+++ b/packages/remark-abbr/dist/index.js
@@ -38,11 +38,20 @@ function plugin() {
 
   function transformer(tree) {
     var abbrs = {};
-    visit(tree, 'paragraph', find(abbrs));
+    var emptyParagraphsToRemove = new Map();
+
+    visit(tree, 'paragraph', find(abbrs, emptyParagraphsToRemove));
+    emptyParagraphsToRemove.forEach(function (indices, key) {
+      indices.reverse();
+      indices.forEach(function (index) {
+        key.children.splice(index, 1);
+      });
+    });
+
     visit(tree, replace(abbrs));
   }
 
-  function find(abbrs) {
+  function find(abbrs, emptyParagraphsToRemove) {
     return function one(node, index, parent) {
       for (var i = 0; i < node.children.length; i++) {
         var child = node.children[i];
@@ -52,8 +61,12 @@ function plugin() {
         node.children.splice(i, 1);
         i -= 1;
       }
-      // Remove paragraph if there is no child
-      if (node.children.length === 0) parent.children.splice(index, 1);
+      // Keep track of empty paragraphs to remove
+      if (node.children.length === 0) {
+        var indices = emptyParagraphsToRemove.get(parent) || [];
+        indices.push(index);
+        emptyParagraphsToRemove.set(parent, indices);
+      }
     };
   }
 
diff --git a/packages/remark-abbr/src/index.js b/packages/remark-abbr/src/index.js
index 24e8cafb..09ebad1a 100644
--- a/packages/remark-abbr/src/index.js
+++ b/packages/remark-abbr/src/index.js
@@ -33,11 +33,20 @@ function plugin () {
 
   function transformer (tree) {
     const abbrs = {}
-    visit(tree, 'paragraph', find(abbrs))
+    const emptyParagraphsToRemove = new Map()
+
+    visit(tree, 'paragraph', find(abbrs, emptyParagraphsToRemove))
+    emptyParagraphsToRemove.forEach((indices, key) => {
+      indices.reverse()
+      indices.forEach((index) => {
+        key.children.splice(index, 1)
+      })
+    })
+
     visit(tree, replace(abbrs))
   }
 
-  function find (abbrs) {
+  function find (abbrs, emptyParagraphsToRemove) {
     return function one (node, index, parent) {
       for (let i = 0; i < node.children.length; i++) {
         const child = node.children[i]
@@ -47,8 +56,12 @@ function plugin () {
         node.children.splice(i, 1)
         i -= 1
       }
-      // Remove paragraph if there is no child
-      if (node.children.length === 0) parent.children.splice(index, 1)
+      // Keep track of empty paragraphs to remove
+      if (node.children.length === 0) {
+        const indices = emptyParagraphsToRemove.get(parent) || []
+        indices.push(index)
+        emptyParagraphsToRemove.set(parent, indices)
+      }
     }
   }
 


Test patch:
diff --git a/packages/remark-abbr/__tests__/__snapshots__/index.js.snap b/packages/remark-abbr/__tests__/__snapshots__/index.js.snap
index d55aa6cf..5db122ad 100644
--- a/packages/remark-abbr/__tests__/__snapshots__/index.js.snap
+++ b/packages/remark-abbr/__tests__/__snapshots__/index.js.snap
@@ -9,6 +9,8 @@ exports[`compiles to markdown 1`] = `
 *[HTML]: HyperText Markup Language"
 `;
 
+exports[`does not break with references in their own paragraphs 1`] = `"<p>Here is a test featuring <abbr title=\\"A B C\\">abc</abbr> and <abbr title=\\"D E F\\">def</abbr></p>"`;
+
 exports[`no reference 1`] = `"<p>No reference!</p>"`;
 
 exports[`passes the first regression test 1`] = `
diff --git a/packages/remark-abbr/__tests__/index.js b/packages/remark-abbr/__tests__/index.js
index accf5c82..fbe1bae4 100644
--- a/packages/remark-abbr/__tests__/index.js
+++ b/packages/remark-abbr/__tests__/index.js
@@ -155,3 +155,15 @@ it('does not parse words containing abbr', () => {
 
   expect(contents).not.toContain('<abbr')
 })
+
+it('does not break with references in their own paragraphs', () => {
+  const {contents} = render(dedent`
+    Here is a test featuring abc and def
+
+    *[abc]: A B C
+
+    *[def]: D E F
+  `)
+
+  expect(contents).toMatchSnapshot()
+})