Repository: maizzle/framework
Pull request #853 (2022-12-08 22:51:41)

refactor: removing unused css
This PR tries to fixes a bug with removing unused CSS, where some selectors were being removed even if they were part of a media query or pseudo, like in #852.

Tricky one because it doesn't creep up when you use the regular utility first approach, so this worked as expected:

```html
<div class="dark:bg-slate-900"></div>
```

However when composing classes, like with Tailwind's `@apply`, the class would have been wrongfully removed from the HTML:

```postcss
/* src/css/utilities.css */

.primary-text-color {
  @apply text-gray-100;
  @apply dark:text-gray-800 !important;
}
```

```html
<extends src="src/layouts/main.html">
  <block name="template">
    <div class="primary-text-color">test</div>
  </block>
</extends>
```

The `<style>` tag in the compiled HTML would have looked like this:

```html
<html>
<head>
	<style>
	  @media (prefers-color-scheme: dark) {
	    .primary-text-color {
	      color: #1f2937 !important;
	    }
	  }
	  :root {
	    color-scheme: light dark;
	  }
	</style>
</head>
<body>
  <!-- The div is missing the .primary-text-color class! -->
  <div style="color: #f3f4f6">test</div>
</body>
</html>
```


Patch:
diff --git a/src/transformers/index.js b/src/transformers/index.js
index cee5d92..68b8833 100644
--- a/src/transformers/index.js
+++ b/src/transformers/index.js
@@ -28,7 +28,6 @@ exports.process = async (html, config) => {
   html = await inline(html, config)
   html = await shorthandInlineCSS(html, config)
   html = await removeUnusedCSS(html, config)
-  html = await removeInlinedClasses(html, config)
   html = await removeInlineSizes(html, config)
   html = await removeInlineBgColor(html, config)
   html = await removeAttributes(html, config)
@@ -54,7 +53,7 @@ exports.addURLParams = (html, config) => addURLParams(html, config, true)
 exports.preventWidows = (html, config) => preventWidows(html, config)
 exports.replaceStrings = (html, config) => replaceStrings(html, config, true)
 exports.safeClassNames = (html, config) => safeClassNames(html, config, true)
-exports.removeUnusedCSS = (html, config) => removeUnusedCSS(html, config, true)
+exports.removeUnusedCSS = (html, config) => removeUnusedCSS(html, config)
 exports.removeAttributes = (html, config) => removeAttributes(html, config, true)
 exports.attributeToStyle = (html, config) => attributeToStyle(html, config, true)
 exports.removeInlineSizes = (html, config) => removeInlineSizes(html, config, true)
diff --git a/src/transformers/removeInlinedSelectors.js b/src/transformers/removeInlinedSelectors.js
index a0ecfe6..1a4f6eb 100644
--- a/src/transformers/removeInlinedSelectors.js
+++ b/src/transformers/removeInlinedSelectors.js
@@ -20,16 +20,26 @@ const plugin = posthtmlOptions => tree => {
     // For each style tag...
     if (node.tag === 'style') {
       const {root} = postcss().process(node.content)
+      const preservedClasses = []
 
-      root.walkRules(rule => {
-        // Skip media queries and such...
-        if (rule.parent.type === 'atrule') {
-          return
+      // Preserve selectors in at rules
+      root.walkAtRules(rule => {
+        if (['media', 'supports'].includes(rule.name)) {
+          rule.walkRules(rule => {
+            preservedClasses.push(rule.selector)
+          })
         }
+      })
 
+      root.walkRules(rule => {
         const {selector} = rule
         const prop = get(rule.nodes[0], 'prop')
 
+        // Preserve pseudo selectors
+        if (selector.includes(':')) {
+          preservedClasses.push(selector)
+        }
+
         try {
           // If we find the selector in the HTML...
           tree.match(matchHelper(selector), n => {
@@ -39,23 +49,16 @@ const plugin = posthtmlOptions => tree => {
 
             // If the class is in the style attribute (inlined), remove it
             if (has(styleAttr, prop)) {
-              // Remove the class attribute
-              remove(classAttr, s => selector.includes(s))
+              // Remove the class as long as it's not a preserved class
+              if (!preservedClasses.some(item => item.endsWith(selector) || item.startsWith(selector))) {
+                remove(classAttr, classToRemove => selector.includes(classToRemove))
+              }
 
               // Remove the rule in the <style> tag
-              rule.remove()
-            }
-
-            /**
-             * Remove from <style> selectors that were used to create shorthand declarations
-             * like `margin: 0 0 0 16px` (transformed with mergeLonghand when inlining).
-             */
-            Object.keys(styleAttr).forEach(key => {
-              if (prop && prop.includes(key)) {
+              if (rule.parent.type !== 'atrule') {
                 rule.remove()
-                remove(classAttr, s => selector.includes(s))
               }
-            })
+            }
 
             n.attrs = parsedAttrs.compose()
 
diff --git a/src/transformers/removeUnusedCss.js b/src/transformers/removeUnusedCss.js
index a798452..60c79a7 100644
--- a/src/transformers/removeUnusedCss.js
+++ b/src/transformers/removeUnusedCss.js
@@ -1,15 +1,13 @@
 const {comb} = require('email-comb')
 const {get, merge} = require('lodash')
+const removeInlinedClasses = require('./removeInlinedSelectors')
 
-module.exports = async (html, config = {}, direct = false) => {
+module.exports = async (html, config = {}) => {
+  // If it's explicitly disabled, return the HTML
   if (get(config, 'removeUnusedCSS') === false) {
     return html
   }
 
-  if (!direct && !get(config, 'removeUnusedCSS')) {
-    return html
-  }
-
   const safelist = [
     '*body*', // Gmail
     '.gmail*', // Gmail
@@ -36,9 +34,9 @@ module.exports = async (html, config = {}, direct = false) => {
     whitelist: [...get(config, 'whitelist', []), ...safelist]
   }
 
-  const options = typeof config === 'boolean' && config ?
-    defaultOptions :
-    merge(defaultOptions, get(config, 'removeUnusedCSS', config))
+  const options = merge(defaultOptions, get(config, 'removeUnusedCSS', config))
+
+  html = await removeInlinedClasses(html, options)
 
   return comb(html, options).result
 }


Test patch:
diff --git a/test/expected/components/kitchen-sink.html b/test/expected/components/kitchen-sink.html
index 231ae6b..aea4cd0 100644
--- a/test/expected/components/kitchen-sink.html
+++ b/test/expected/components/kitchen-sink.html
@@ -5,6 +5,8 @@ bar
 <!DOCTYPE html>
 <html lang="en" xmlns:v="urn:schemas-microsoft-com:vml">
 <head>
+  <style>
+  </style>
 </head>
 <body class="bg-red-500">
   <h1>H1 in fill:template</h1>
diff --git a/test/test-posthtml.js b/test/test-posthtml.js
index 0ee1e1b..496a554 100644
--- a/test/test-posthtml.js
+++ b/test/test-posthtml.js
@@ -60,7 +60,8 @@ test('components', async t => {
         components: {
           folders: ['test/stubs/layouts', 'test/stubs/components']
         }
-      }
+      },
+      removeUnusedCSS: false
     },
     beforeRender(html, config) {
       config.foo = 'bar'
@@ -89,7 +90,8 @@ test('components (backwards compatibility)', async t => {
             delimiters: ['[[', ']]']
           }
         }
-      }
+      },
+      removeUnusedCSS: false
     }
   }
 
diff --git a/test/test-transformers.js b/test/test-transformers.js
index 9cb74b9..f729924 100644
--- a/test/test-transformers.js
+++ b/test/test-transformers.js
@@ -162,7 +162,11 @@ test('remove unused CSS', async t => {
   <html>
     <head>
       <style>
+        @media (screen) {
+          .ignore {color: yellow}
+        }
         .foo {color: red}
+        .foo:hover {color: blue}
         .bar-baz {color: blue}
         .baz {color: white}
       </style>
@@ -172,70 +176,38 @@ test('remove unused CSS', async t => {
     </body>
   </html>`
 
-  const enabledResult = `<!DOCTYPE html>
+  const withOptionsResult = `<!DOCTYPE html>
   <html>
     <head>
       <style>
         .foo {color: red}
-      </style>
+        .foo:hover {color: blue}
+        .bar-baz {color: blue}</style>
     </head>
     <body>
       <div class="foo {{ test }}">test div with some text</div>
     </body>
   </html>`
 
-  const withOptionsResult = `<!DOCTYPE html>
+  const unsetResult = `<!DOCTYPE html>
   <html>
     <head>
       <style>
         .foo {color: red}
-        .bar-baz {color: blue}
-      </style>
+        .foo:hover {color: blue}</style>
     </head>
     <body>
       <div class="foo {{ test }}">test div with some text</div>
     </body>
   </html>`
 
-  const enabled = await Maizzle.removeUnusedCSS(html)
   const disabled = await Maizzle.removeUnusedCSS(html, {removeUnusedCSS: false})
   const withOptions = await Maizzle.removeUnusedCSS(html, {whitelist: ['.bar*']})
+  const unset = await Maizzle.removeUnusedCSS(html)
 
-  t.is(enabled, enabledResult)
   t.is(disabled, html)
   t.is(withOptions, withOptionsResult)
-})
-
-test('remove unused CSS (disabled)', async t => {
-  const html = `<!DOCTYPE html>
-  <html>
-    <head>
-      <style>
-        .foo {color: red}
-      </style>
-    </head>
-    <body>
-      <div class="foo">test div with some text</div>
-    </body>
-  </html>`
-
-  const result = `<!DOCTYPE html>
-  <html>
-    <head>
-      <style>
-        .foo {color: red}
-      </style>
-    </head>
-    <body>
-      <div class="foo">test div with some text</div>
-    </body>
-  </html>`
-
-  const disabled = await Maizzle.removeUnusedCSS(html, {removeUnusedCSS: false})
-  const unset = await Maizzle.removeUnusedCSS(html)
-
-  t.is(disabled, result)
-  t.is(unset, result)
+  t.is(unset, unsetResult)
 })
 
 test('remove attributes', async t => {
@@ -539,13 +511,9 @@ test('remove inlined selectors', async t => {
           color: #00a8ff;
         }
 
-        .m-0 {margin: 0}
-
-        .mb-4 {margin-bottom: 16px}
-
-        .mt-0 {margin-top: 0}
-
-        .remove {color: red}
+        .padded {
+          padding: 0 20px;
+        }
 
         [data-ogsc] .hidden {display: none}
 
@@ -569,10 +537,10 @@ test('remove inlined selectors', async t => {
       </style>
     </head>
     <body>
-      <div no-value id="keepId" class="remove keep ignore foo-class" style="color: red; display: inline">
-        <h1 class="m-0 mb-4 mt-0 hover-text-blue" style="margin: 0 0 16px;">Title</h1>
+      <div no-value id="keepId" class="keep ignore foo-class" style="color: red; display: inline">
+        <h1 class="padded hover-text-blue" style="padding: 0 20px">Title</h1>
         <img src="https://example.com/image.jpg" style="border: 0; vertical-align: middle">
-        <div id="keepId" class="remove keep ignore" style="color: red; display: inline">text</div>
+        <div id="keepId" class="keep ignore" style="color: red; display: inline">text</div>
       </div>
     </body>
   </html>`
@@ -599,10 +567,10 @@ test('remove inlined selectors', async t => {
       <style>.keep {margin: 0}</style>
     </head>
     <body>
-      <div no-value id="keepId" class="keep foo-class" style="color: red; display: inline">
-        <h1 class="hover-text-blue" style="margin: 0 0 16px">Title</h1>
+      <div no-value id="keepId" class="keep ignore foo-class" style="color: red; display: inline">
+        <h1 class="hover-text-blue" style="padding: 0 20px">Title</h1>
         <img src="https://example.com/image.jpg" style="border: 0; vertical-align: middle">
-        <div id="keepId" class="keep" style="color: red; display: inline">text</div>
+        <div id="keepId" class="keep ignore" style="color: red; display: inline">text</div>
       </div>
     </body>
   </html>`