From bb4690545e777b24caa7c91caab8e068df85320d Mon Sep 17 00:00:00 2001 From: John Otander Date: Thu, 14 Mar 2019 14:51:17 -0600 Subject: [PATCH] fix(gatsby-remark-responsive-iframe): use html node rather than unknown 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: https://github.com/ChristopherBiscardi/gatsby-mdx/issues/183 - HTML injection: https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-transformer-remark/src/extend-node-type.js#L489-L492 --- .../src/__tests__/__snapshots__/index.js.snap | 70 +++++++++++++++++++ .../src/__tests__/index.js | 40 +++++++---- .../src/index.js | 28 ++++---- 3 files changed, 114 insertions(+), 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 index 251c16c73b105..d6e42fb623cff 100644 --- 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`] = ` +" +
+ + +
+ " +`; + +exports[`gatsby-remark-responsive-iframe transforms an iframe and maintains existing styles when a semicolon exists 1`] = ` +" +
+ + +
+ " +`; + exports[`gatsby-remark-responsive-iframe transforms an iframe with pixel width and height 1`] = ` "
+ +
+ " +`; + +exports[`gatsby-remark-responsive-iframe transforms an object and maintains existing styles when a semicolon exists 1`] = ` +" +
+ +
+ " +`; + exports[`gatsby-remark-responsive-iframe transforms an object with pixel width and height 1`] = ` "
{ 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"> + `) + + 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"> + `) + + 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 index b5b02c732fd50..10433052c2ad8 100644 --- 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 = {}) =>
` - 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 } } })