fix(mjml-core): prevent empty style tags in head (#2682)

mjmlio · Jun 8, 2023 · 49535a0 · 49535a0
1 parent 988819d
commit 49535a0
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 18 deletions.
diff --git a/packages/mjml-core/src/helpers/skeleton.js b/packages/mjml-core/src/helpers/skeleton.js
@@ -1,7 +1,8 @@
-import { map, reduce, negate, isNil, isFunction } from 'lodash'
+import { negate, isNil } from 'lodash'
 import buildPreview from './preview'
 import { buildFontsTags } from './fonts'
 import buildMediaQueriesTags from './mediaQueries'
+import { buildStyleFromComponents, buildStyleFromTags } from './styles'
 
 export default function skeleton(options) {
   const {
@@ -11,8 +12,8 @@ export default function skeleton(options) {
     content = '',
     fonts = {},
     mediaQueries = {},
-    headStyle = [],
-    componentsHeadStyle = {},
+    headStyle = {},
+    componentsHeadStyle = [],
     headRaw = [],
     preview,
     title = '',
@@ -59,21 +60,8 @@ export default function skeleton(options) {
     <![endif]-->
     ${buildFontsTags(content, inlineStyle, fonts)}
     ${buildMediaQueriesTags(breakpoint, mediaQueries, forceOWADesktop)}
-    <style type="text/css">
-    ${reduce(
-      componentsHeadStyle,
-      (result, compHeadStyle) => `${result}\n${compHeadStyle(breakpoint)}`,
-      '',
-    )}
-    ${reduce(
-      headStyle,
-      (result, headStyle) => `${result}\n${headStyle(breakpoint)}`,
-      '',
-    )}
-    </style>
-    <style type="text/css">
-    ${map(style, (s) => (isFunction(s) ? s(breakpoint) : s)).join('')}
-    </style>
+    ${buildStyleFromComponents(breakpoint, componentsHeadStyle, headStyle)}
+    ${buildStyleFromTags(breakpoint, style)}
     ${headRaw.filter(negate(isNil)).join('\n')}
   </head>
   <body style="word-spacing:normal;${

diff --git a/packages/mjml-core/src/helpers/styles.js b/packages/mjml-core/src/helpers/styles.js
@@ -0,0 +1,34 @@
+import { isFunction } from 'lodash'
+
+export function buildStyleFromComponents(
+  breakpoint,
+  componentsHeadStyles,
+  headStylesObject,
+) {
+  const headStyles = Object.values(headStylesObject)
+
+  if (componentsHeadStyles.length === 0 && headStyles.length === 0) {
+    return ''
+  }
+
+  return `
+    <style type="text/css">${[...componentsHeadStyles, ...headStyles].reduce(
+      (result, styleFunction) => `${result}\n${styleFunction(breakpoint)}`,
+      '',
+    )}
+    </style>`
+}
+
+export function buildStyleFromTags(breakpoint, styles) {
+  if (styles.length === 0) {
+    return ''
+  }
+
+  return ` 
+    <style type="text/css">${styles.reduce(
+      (result, style) =>
+        `${result}\n${isFunction(style) ? style(breakpoint) : style}`,
+      '',
+    )}
+    </style>`
+}
diff --git a/packages/mjml-core/tests/index.js b/packages/mjml-core/tests/index.js
@@ -2,4 +2,5 @@ require('./jsonToXml-test')
 require('./mergeOutlookConditionnals-test')
 require('./minifyOutlookConditionnals-test')
 require('./shorthandParser-test')
+require('./skeleton-test')
 require('./widthParser-test')
diff --git a/packages/mjml-core/tests/skeleton-test.js b/packages/mjml-core/tests/skeleton-test.js
@@ -0,0 +1,70 @@
+const chai = require('chai')
+const { load } = require('cheerio')
+const skeleton = require('../lib/helpers/skeleton')
+
+// The conditional style tag for Outlook does not get parsed by cheerio,
+// so each outputStyleCount excludes it
+const testValues = [
+  {
+    options: {},
+    outputStyleCount: 1,
+  },
+  {
+    options: {
+      componentsHeadStyle: [
+        () => '.custom-component-1 .custom-child { background: red; }',
+      ],
+    },
+    outputStyleCount: 2,
+  },
+  {
+    options: {
+      headStyle: {
+        'custom-component': () =>
+          '.custom-component .custom-child { background: orange; }',
+      },
+    },
+    outputStyleCount: 2,
+  },
+  {
+    options: {
+      componentsHeadStyle: [
+        () => '.custom-component-1 .custom-child { background: yellow; }',
+      ],
+      headStyle: {
+        'custom-component': () =>
+          '.custom-component .custom-child { background: green; }',
+      },
+    },
+    outputStyleCount: 2,
+  },
+  {
+    options: {
+      style: ['#title { background: blue; }'],
+    },
+    outputStyleCount: 2,
+  },
+  {
+    options: {
+      componentsHeadStyle: [
+        () => '.custom-component-1 .custom-child { background: purple; }',
+      ],
+      headStyle: {
+        'custom-component': () =>
+          '.custom-component .custom-child { background: black; }',
+      },
+      style: [() => '#title { background: white; }'],
+    },
+    outputStyleCount: 3,
+  },
+]
+
+testValues.forEach((testUnit) => {
+  const { options, outputStyleCount } = testUnit
+
+  const $ = load(skeleton(options))
+
+  chai
+    .expect($('head style').get().length, 'Unexpected number of style tags')
+    .to.equal(outputStyleCount)
+})