fix(gatsby-remark-images): don’t add links if image is already linked (#6982)

* fix(gatsby-remark-images): don’t add links if image is already linked

fixes #6980
Co-authored-by: @pieh

* fix: handle weird nesting and mixed MD/HTML

* fix: remove bad check for parent links

Author: jlengstorf

packages/gatsby-remark-images/package.json

@@ -12,7 +12,8 @@
     "is-relative-url": "^2.0.0",
     "lodash": "^4.17.4",
     "slash": "^1.0.0",
-    "unist-util-select": "^1.5.0"
+    "unist-util-select": "^1.5.0",
+    "unist-util-visit-parents": "^2.0.1"
   },
   "devDependencies": {
     "@babel/cli": "7.0.0-beta.52",

packages/gatsby-remark-images/src/__tests__/__snapshots__/index.js.snap

@@ -1,5 +1,75 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`it handles goofy nesting properly 1`] = `
+"
+  <span
+    class=\\"gatsby-resp-image-wrapper\\"
+    style=\\"position: relative; display: block; ; max-width: 300px; margin-left: auto; margin-right: auto;\\"
+  >
+    <span
+      class=\\"gatsby-resp-image-background-image\\"
+      style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url('url('data:image/png;base64, iVBORw)'); background-size: cover; display: block;\\"
+    >
+      <img
+        class=\\"gatsby-resp-image-image\\"
+        style=\\"width: 100%; height: 100%; margin: 0; vertical-align: middle; position: absolute; top: 0; left: 0; box-shadow: inset 0px 0px 0px 400px white;\\"
+        alt=\\"test\\"
+        title=\\"\\"
+        src=\\"not-a-real-dir/images/this-image-already-has-a-link.jpeg\\"
+        srcset=\\"not-a-real-dir/images/this-image-already-has-a-link.jpeg, not-a-real-dir/images/this-image-already-has-a-link.jpeg\\"
+        sizes=\\"(max-width: 650px) 100vw, 650px\\"
+      />
+    </span>
+  </span>
+  "
+`;
+
+exports[`it leaves images that are already linked alone 1`] = `
+"
+  <span
+    class=\\"gatsby-resp-image-wrapper\\"
+    style=\\"position: relative; display: block; ; max-width: 300px; margin-left: auto; margin-right: auto;\\"
+  >
+    <span
+      class=\\"gatsby-resp-image-background-image\\"
+      style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url('url('data:image/png;base64, iVBORw)'); background-size: cover; display: block;\\"
+    >
+      <img
+        class=\\"gatsby-resp-image-image\\"
+        style=\\"width: 100%; height: 100%; margin: 0; vertical-align: middle; position: absolute; top: 0; left: 0; box-shadow: inset 0px 0px 0px 400px white;\\"
+        alt=\\"img\\"
+        title=\\"\\"
+        src=\\"not-a-real-dir/image/my-image.jpg\\"
+        srcset=\\"not-a-real-dir/image/my-image.jpg, not-a-real-dir/image/my-image.jpg\\"
+        sizes=\\"(max-width: 650px) 100vw, 650px\\"
+      />
+    </span>
+  </span>
+  "
+`;
+
+exports[`it leaves linked HTML img tags alone 1`] = `
+"<a href=\\"https://example.org\\">
+  
+  <span class=\\"gatsby-resp-image-wrapper\\" style=\\"position: relative; display: block; ; max-width: 300px; margin-left: auto; margin-right: auto;\\">
+    <span class=\\"gatsby-resp-image-background-image\\" style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url(&apos;url(&apos;data:image/png;base64, iVBORw)&apos;); background-size: cover; display: block;\\">
+      <img class=\\"gatsby-resp-image-image\\" style=\\"width: 100%; height: 100%; margin: 0; vertical-align: middle; position: absolute; top: 0; left: 0; box-shadow: inset 0px 0px 0px 400px white;\\" alt=\\"this image already has a link\\" title=\\"\\" src=\\"not-a-real-dir/images/this-image-already-has-a-link.jpeg\\" srcset=\\"not-a-real-dir/images/this-image-already-has-a-link.jpeg, not-a-real-dir/images/this-image-already-has-a-link.jpeg\\" sizes=\\"(max-width: 650px) 100vw, 650px\\">
+    </span>
+  </span>
+  
+</a>"
+`;
+
+exports[`it leaves single-line linked HTML img tags alone 1`] = `
+"
+  <span class=\\"gatsby-resp-image-wrapper\\" style=\\"position: relative; display: block; ; max-width: 300px; margin-left: auto; margin-right: auto;\\">
+    <span class=\\"gatsby-resp-image-background-image\\" style=\\"padding-bottom: 133.33333333333331%; position: relative; bottom: 0; left: 0; background-image: url(&apos;url(&apos;data:image/png;base64, iVBORw)&apos;); background-size: cover; display: block;\\">
+      <img class=\\"gatsby-resp-image-image\\" style=\\"width: 100%; height: 100%; margin: 0; vertical-align: middle; position: absolute; top: 0; left: 0; box-shadow: inset 0px 0px 0px 400px white;\\" alt=\\"this image already has a link\\" title=\\"\\" src=\\"not-a-real-dir/images/this-image-already-has-a-link.jpeg\\" srcset=\\"not-a-real-dir/images/this-image-already-has-a-link.jpeg, not-a-real-dir/images/this-image-already-has-a-link.jpeg\\" sizes=\\"(max-width: 650px) 100vw, 650px\\">
+    </span>
+  </span>
+  "
+`;
+
 exports[`it transforms HTML img tags 1`] = `
 "
   <a class=\\"gatsby-resp-image-link\\" href=\\"not-a-real-dir/image/my-image.jpeg\\" style=\\"display: block\\" target=\\"_blank\\" rel=\\"noopener\\">

packages/gatsby-remark-images/src/__tests__/index.js

@@ -146,3 +146,64 @@ test(`it leaves non-relative HTML img tags alone`, async () => {
   const nodes = await plugin(createPluginOptions(content, imagePath))
   expect(nodes[0].value).toBe(content)
 })
+
+test(`it leaves images that are already linked alone`, async () => {
+  const imagePath = `image/my-image.jpg`
+  const content = `
+[![img](./${imagePath})](https://google.com)
+`
+
+  const nodes = await plugin(createPluginOptions(content, imagePath))
+  const node = nodes.pop()
+
+  expect(node.type).toBe(`html`)
+  expect(node.value).toMatchSnapshot()
+  expect(node.value).not.toMatch(`<html>`)
+})
+
+test(`it leaves linked HTML img tags alone`, async () => {
+  const imagePath = `images/this-image-already-has-a-link.jpeg`
+
+  const content = `
+<a href="https://example.org">
+  <img src="./${imagePath}">
+</a>
+  `.trim()
+
+  const nodes = await plugin(createPluginOptions(content, imagePath))
+  const node = nodes.pop()
+
+  expect(node.type).toBe(`html`)
+  expect(node.value).toMatchSnapshot()
+  expect(node.value).not.toMatch(`<html>`)
+})
+
+test(`it leaves single-line linked HTML img tags alone`, async () => {
+  const imagePath = `images/this-image-already-has-a-link.jpeg`
+
+  const content = `
+<a href="https://example.org"><img src="./${imagePath}"></a>
+  `.trim()
+
+  const nodes = await plugin(createPluginOptions(content, imagePath))
+  const node = nodes.pop()
+
+  expect(node.type).toBe(`html`)
+  expect(node.value).toMatchSnapshot()
+  expect(node.value).not.toMatch(`<html>`)
+})
+
+test(`it handles goofy nesting properly`, async () => {
+  const imagePath = `images/this-image-already-has-a-link.jpeg`
+
+  const content = `
+  <a href="https://google.com">**![test](./${imagePath})**</a>
+  `.trim()
+
+  const nodes = await plugin(createPluginOptions(content, imagePath))
+  const node = nodes.pop()
+
+  expect(node.type).toBe(`html`)
+  expect(node.value).toMatchSnapshot()
+  expect(node.value).not.toMatch(`<html>`)
+})

packages/gatsby-remark-images/src/index.js

@@ -1,4 +1,5 @@
-const select = require(`unist-util-select`)
+// const select = require(`unist-util-select`)
+const visitWithParents = require(`unist-util-visit-parents`)
 const path = require(`path`)
 const isRelativeUrl = require(`is-relative-url`)
 const _ = require(`lodash`)
@@ -29,15 +30,34 @@ module.exports = (
 
   const options = _.defaults(pluginOptions, defaults)
 
+  const findParentLinks = ({ children }) =>
+    children.some(
+      node =>
+        (node.type === `html` && !!node.value.match(/<a /)) ||
+        node.type === `link`
+    )
+
+  // This will allow the use of html image tags
+  // const rawHtmlNodes = select(markdownAST, `html`)
+  let rawHtmlNodes = []
+  visitWithParents(markdownAST, `html`, (node, ancestors) => {
+    const inLink = ancestors.some(findParentLinks)
+
+    rawHtmlNodes.push({ node, inLink })
+  })
+
   // This will only work for markdown syntax image tags
-  const markdownImageNodes = select(markdownAST, `image`)
+  let markdownImageNodes = []
+
+  visitWithParents(markdownAST, `image`, (node, ancestors) => {
+    const inLink = ancestors.some(findParentLinks)
 
-  // This will also allow the use of html image tags
-  const rawHtmlNodes = select(markdownAST, `html`)
+    markdownImageNodes.push({ node, inLink })
+  })
 
   // Takes a node and generates the needed images and then returns
   // the needed HTML replacement for the image
-  const generateImagesAndUpdateNode = async function(node, resolve) {
+  const generateImagesAndUpdateNode = async function(node, resolve, inLink) {
     // Check if this markdownNode has a File parent. This plugin
     // won't work if the image isn't hosted locally.
     const parentNode = getNode(markdownNode.parent)
@@ -138,7 +158,7 @@ module.exports = (
         <img
           class="${imageClass}"
           style="${imageStyle}"
-          src="${fallbackSrc}" 
+          src="${fallbackSrc}"
           alt="${node.alt ? node.alt : defaultAlt}"
           title="${node.title ? node.title : ``}"
           src="${fallbackSrc}"
@@ -165,7 +185,7 @@ module.exports = (
   `
 
     // Make linking to original image optional.
-    if (options.linkImagesToOriginal) {
+    if (!inLink && options.linkImagesToOriginal) {
       rawHTML = `
   <a
     class="gatsby-resp-image-link"
@@ -196,7 +216,7 @@ module.exports = (
   return Promise.all(
     // Simple because there is no nesting in markdown
     markdownImageNodes.map(
-      node =>
+      ({ node, inLink }) =>
         new Promise(async (resolve, reject) => {
           const fileType = node.url.slice(-3)
 
@@ -207,7 +227,11 @@ module.exports = (
             fileType !== `gif` &&
             fileType !== `svg`
           ) {
-            const rawHTML = await generateImagesAndUpdateNode(node, resolve)
+            const rawHTML = await generateImagesAndUpdateNode(
+              node,
+              resolve,
+              inLink
+            )
 
             if (rawHTML) {
               // Replace the image node with an inline HTML node.
@@ -226,7 +250,7 @@ module.exports = (
     Promise.all(
       // Complex because HTML nodes can contain multiple images
       rawHtmlNodes.map(
-        node =>
+        ({ node, inLink }) =>
           new Promise(async (resolve, reject) => {
             if (!node.value) {
               return resolve()
@@ -265,7 +289,8 @@ module.exports = (
               ) {
                 const rawHTML = await generateImagesAndUpdateNode(
                   formattedImgTag,
-                  resolve
+                  resolve,
+                  inLink
                 )
 
                 if (rawHTML) {