fix(gatsby-remark-responsive-iframe): use html node rather than unkno…

…wn node type (#12543) ## Description The gatsby-remark-responsive-images plugin will now directly insert an HTML node rather than HAST data attributes. This should have no effect because we're already embedding HTML as part of the compilation process. It will break any potential plugin that might be depending on the existence of these data attributes, but as far as I can tell it isn't being used (especially considering the node type is marked as "unknown"). This makes it easier for other projects like gatsby-mdx to support existing gatsby-remark-* plugins because then we can watch for HTML nodes injected into the MDAST and transform them to JSX. ## Related: - Downstream issue: ChristopherBiscardi/gatsby-mdx#183 - HTML injection: https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-transformer-remark/src/extend-node-type.js#L489-L492
gatsbyjs · Mar 14, 2019 · bb46905 · bb46905
1 parent 57159d0
commit bb46905
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 24 deletions.
diff --git a/packages/gatsby-remark-responsive-iframe/src/__tests__/__snapshots__/index.js.snap b/packages/gatsby-remark-responsive-iframe/src/__tests__/__snapshots__/index.js.snap
@@ -40,6 +40,42 @@ exports[`gatsby-remark-responsive-iframe doesn't transform an object with dimens
       "
 `;
 
+exports[`gatsby-remark-responsive-iframe transforms an iframe and maintains existing styles 1`] = `
+"
+          <div
+            class=\\"gatsby-resp-iframe-wrapper\\"
+            style=\\"padding-bottom: NaN%; position: relative; height: 0; overflow: hidden;\\"
+          >
+            <iframe url=\\"http://www.example.com/\\" style=\\"border:0;
+            position: absolute;
+            top: 0;
+            left: 0;
+            width: 100%;
+            height: 100%;
+          \\"></iframe>
+    
+          </div>
+          "
+`;
+
+exports[`gatsby-remark-responsive-iframe transforms an iframe and maintains existing styles when a semicolon exists 1`] = `
+"
+          <div
+            class=\\"gatsby-resp-iframe-wrapper\\"
+            style=\\"padding-bottom: NaN%; position: relative; height: 0; overflow: hidden;\\"
+          >
+            <iframe url=\\"http://www.example.com/\\" style=\\"border:0;
+            position: absolute;
+            top: 0;
+            left: 0;
+            width: 100%;
+            height: 100%;
+          \\"></iframe>
+    
+          </div>
+          "
+`;
+
 exports[`gatsby-remark-responsive-iframe transforms an iframe with pixel width and height 1`] = `
 "
           <div
@@ -76,6 +112,40 @@ exports[`gatsby-remark-responsive-iframe transforms an iframe with unitless widt
           "
 `;
 
+exports[`gatsby-remark-responsive-iframe transforms an object and maintains existing styles 1`] = `
+"
+          <div
+            class=\\"gatsby-resp-iframe-wrapper\\"
+            style=\\"padding-bottom: NaN%; position: relative; height: 0; overflow: hidden;\\"
+          >
+            <object url=\\"http://www.example.com/\\" style=\\"
+            position: absolute;
+            top: 0;
+            left: 0;
+            width: 100%;
+            height: 100%;
+          \\"></object>
+          </div>
+          "
+`;
+
+exports[`gatsby-remark-responsive-iframe transforms an object and maintains existing styles when a semicolon exists 1`] = `
+"
+          <div
+            class=\\"gatsby-resp-iframe-wrapper\\"
+            style=\\"padding-bottom: NaN%; position: relative; height: 0; overflow: hidden;\\"
+          >
+            <object url=\\"http://www.example.com/\\" style=\\"
+            position: absolute;
+            top: 0;
+            left: 0;
+            width: 100%;
+            height: 100%;
+          \\"></object>
+          </div>
+          "
+`;
+
 exports[`gatsby-remark-responsive-iframe transforms an object with pixel width and height 1`] = `
 "
           <div

diff --git a/packages/gatsby-remark-responsive-iframe/src/__tests__/index.js b/packages/gatsby-remark-responsive-iframe/src/__tests__/index.js
@@ -28,14 +28,10 @@ describe(`gatsby-remark-responsive-iframe`, () => {
 
       const transformed = await plugin({ markdownAST })
       const node = find(transformed, function(node) {
-        return node.type === `unknown`
+        return node.type === `html`
       })
       expect(node).toBeDefined()
-      expect(node.data).toBeDefined()
-      expect(node.data.hChildren).toBeDefined()
-      expect(node.data.hChildren.length).toBeGreaterThan(0)
-      const [child] = node.data.hChildren
-      expect(child.value).toMatchSnapshot()
+      expect(node.value).toMatchSnapshot()
     })
   })
   ;[`iframe`, `object`].forEach(tag => {
@@ -46,14 +42,34 @@ describe(`gatsby-remark-responsive-iframe`, () => {
 
       const transformed = await plugin({ markdownAST })
       const node = find(transformed, function(node) {
-        return node.type === `unknown`
+        return node.type === `html`
       })
       expect(node).toBeDefined()
-      expect(node.data).toBeDefined()
-      expect(node.data.hChildren).toBeDefined()
-      expect(node.data.hChildren.length).toBeGreaterThan(0)
-      const [child] = node.data.hChildren
-      expect(child.value).toMatchSnapshot()
+      expect(node.value).toMatchSnapshot()
+    })
+    it(`transforms an ${tag} and maintains existing styles`, async () => {
+      const markdownAST = remark.parse(`
+<${tag} url="http://www.example.com/" style="border:0" width="600px" height="400px"></${tag}>
+    `)
+
+      const transformed = await plugin({ markdownAST })
+      const node = find(transformed, function(node) {
+        return node.type === `html`
+      })
+      expect(node).toBeDefined()
+      expect(node.value).toMatchSnapshot()
+    })
+    it(`transforms an ${tag} and maintains existing styles when a semicolon exists`, async () => {
+      const markdownAST = remark.parse(`
+<${tag} url="http://www.example.com/" style="border:0;" width="600px" height="400px"></${tag}>
+    `)
+
+      const transformed = await plugin({ markdownAST })
+      const node = find(transformed, function(node) {
+        return node.type === `html`
+      })
+      expect(node).toBeDefined()
+      expect(node.value).toMatchSnapshot()
     })
   })
 

diff --git a/packages/gatsby-remark-responsive-iframe/src/index.js b/packages/gatsby-remark-responsive-iframe/src/index.js
@@ -13,6 +13,8 @@ const isUnitlessNumber = n => {
 const isUnitlessOrPixelNumber = n =>
   n && (isUnitlessNumber(n) || isPixelNumber(n))
 
+const needsSemicolon = str => !str.endsWith(`;`)
+
 // Aspect ratio can only be determined if both width and height are unitless or
 // pixel values. Any other values mean the responsive wrapper is not applied.
 const acceptedDimensions = (width, height) =>
@@ -32,9 +34,20 @@ module.exports = ({ markdownAST }, pluginOptions = {}) =>
         const height = iframe.attr(`height`)
 
         if (acceptedDimensions(width, height)) {
+          const existingStyle = $(`iframe`).attr(`style`) // Other plugins might set border: 0
+          // so we make sure that we maintain those existing styles. If other styles like height or
+          // width are already defined they will be overridden anyway.
+
+          let fullStyle = ``
+          if (existingStyle && needsSemicolon(existingStyle)) {
+            fullStyle = `${existingStyle};`
+          } else if (existingStyle) {
+            fullStyle = existingStyle
+          }
+
           $(`iframe, object`).attr(
             `style`,
-            `
+            `${fullStyle}
             position: absolute;
             top: 0;
             left: 0;
@@ -61,17 +74,8 @@ module.exports = ({ markdownAST }, pluginOptions = {}) =>
           </div>
           `
 
-          node.data = {
-            hChildren: [{ type: `raw`, value: rawHTML }],
-          }
-          // Set type to unknown so mdast-util-to-hast will treat this node as a
-          // div not an iframe — it gets quite confused otherwise.
-          node.type = `unknown`
-
-          // Also apparently, for html node types, you have to delete the value
-          // in order for mdast-util-to-hast to use hChildren. If even if
-          // you change the node type to unknown...
-          delete node.value
+          node.type = `html`
+          node.value = rawHTML
         }
       }
     })