fix(strategy): error when minifying inline CSS style placeholders #1

Author: asyncliz

src/minifyHTMLLiterals.ts

@@ -172,7 +172,7 @@ export interface Result {
 export function defaultGenerateSourceMap(
   ms: MagicStringLike,
   fileName: string
-): SourceMap {
+) {
   return ms.generateMap({
     file: `${fileName}.map`,
     source: fileName,
@@ -188,20 +188,20 @@ export function defaultGenerateSourceMap(
  * @param template the template to check
  * @returns true if the template should be minified
  */
-export function defaultShouldMinify(template: Template): boolean {
+export function defaultShouldMinify(template: Template) {
   return !!template.tag && template.tag.toLowerCase().includes('html');
 }
 
 /**
  * The default validation.
  */
 export const defaultValidation: Validation = {
-  ensurePlaceholderValid(placeholder: any) {
+  ensurePlaceholderValid(placeholder) {
     if (typeof placeholder !== 'string' || !placeholder.length) {
       throw new Error('getPlaceholder() must return a non-empty string');
     }
   },
-  ensureHTMLPartsValid(parts: TemplatePart[], htmlParts: string[]) {
+  ensureHTMLPartsValid(parts, htmlParts) {
     if (parts.length !== htmlParts.length) {
       throw new Error(
         'splitHTMLByPlaceholder() must return same number of strings as template parts'

src/strategy.ts

@@ -70,7 +70,11 @@ export const defaultMinifyOptions: Options = {
  * The default strategy. This uses <code>html-minifier</code> to minify HTML.
  */
 export const defaultStrategy: Strategy<Options> = {
-  getPlaceholder(parts: TemplatePart[]): string {
+  getPlaceholder(parts) {
+    // Using @ and (); will cause the expression not to be removed in CSS.
+    // However, sometimes the semicolon can be removed (ex: inline styles).
+    // In those cases, we want to make sure that the HTML splitting also
+    // accounts for the missing semicolon.
     const suffix = '();';
     let placeholder = '@TEMPLATE_EXPRESSION';
     while (parts.some(part => part.text.includes(placeholder + suffix))) {
@@ -79,13 +83,49 @@ export const defaultStrategy: Strategy<Options> = {
 
     return placeholder + suffix;
   },
-  combineHTMLStrings(parts: TemplatePart[], placeholder: string): string {
+  combineHTMLStrings(parts, placeholder) {
     return parts.map(part => part.text).join(placeholder);
   },
-  minifyHTML(html: string, options?: Options): string {
-    return minify(html, options);
+  minifyHTML(html, options = {}) {
+    let minifyCSSOptions: any;
+    if (options.minifyCSS) {
+      if (
+        options.minifyCSS !== true &&
+        typeof options.minifyCSS !== 'function'
+      ) {
+        minifyCSSOptions = { ...options.minifyCSS };
+      } else {
+        minifyCSSOptions = {};
+      }
+    } else {
+      minifyCSSOptions = false;
+    }
+
+    if (minifyCSSOptions && typeof minifyCSSOptions.level === 'undefined') {
+      minifyCSSOptions.level = {
+        1: {
+          transform(_property: string, value: string) {
+            if (
+              value.startsWith('@TEMPLATE_EXPRESSION') &&
+              !value.endsWith(';')
+            ) {
+              // The CSS minifier has removed the semicolon from the placeholder
+              // and we need to add it back.
+              return `${value};`;
+            }
+          }
+        }
+      };
+    }
+
+    return minify(html, {
+      ...options,
+      minifyCSS: minifyCSSOptions
+    });
   },
-  splitHTMLByPlaceholder(html: string, placeholder: string): string[] {
+  splitHTMLByPlaceholder(html, placeholder) {
+    // Make the last character (a semicolon) optional. See above.
+    // return html.split(new RegExp(`${placeholder}?`, 'g'));
     return html.split(placeholder);
   }
 };

test/strategy.spec.ts

@@ -19,24 +19,69 @@ describe('strategy', () => {
     ];
 
     describe('getPlaceholder()', () => {
-      it('should return "@TEMPLATE_EXPRESSION();"', () => {
+      it('should return a string with @ and () in it with no spaces', () => {
         const placeholder = defaultStrategy.getPlaceholder(parts);
-        expect(placeholder).to.equal('@TEMPLATE_EXPRESSION();');
+        expect(placeholder.indexOf('@')).to.equal(0, 'should start with @');
+        expect(placeholder).to.include('()', 'should contain function parens');
       });
 
       it('should append "_" if placeholder exists in templates', () => {
-        expect(
-          defaultStrategy.getPlaceholder([
-            { text: '@TEMPLATE_EXPRESSION();', start: 0, end: 19 }
-          ])
-        ).to.equal('@TEMPLATE_EXPRESSION_();');
+        const regularPlaceholder = defaultStrategy.getPlaceholder(parts);
+        const oneUnderscore = defaultStrategy.getPlaceholder([
+          { text: regularPlaceholder, start: 0, end: regularPlaceholder.length }
+        ]);
+
+        expect(oneUnderscore).not.to.equal(regularPlaceholder);
+        expect(oneUnderscore).to.include('_');
+
+        const twoUnderscores = defaultStrategy.getPlaceholder([
+          {
+            text: regularPlaceholder,
+            start: 0,
+            end: regularPlaceholder.length
+          },
+          {
+            text: oneUnderscore,
+            start: regularPlaceholder.length,
+            end: regularPlaceholder.length + oneUnderscore.length
+          }
+        ]);
+
+        expect(twoUnderscores).not.to.equal(regularPlaceholder);
+        expect(twoUnderscores).not.to.equal(oneUnderscore);
+        expect(twoUnderscores).to.include('__');
+      });
+
+      it('should return a value that is preserved by html-minifier when splitting', () => {
+        const placeholder = defaultStrategy.getPlaceholder(parts);
+        const minHtml = defaultStrategy.minifyHTML(
+          `
+          <style>
+            ${placeholder}
+
+            p {
+              ${placeholder}
+              color: ${placeholder}
+            }
+
+            div {
+              width: ${placeholder}
+            }
+          </style>
+          <p style="color: ${placeholder}">
+            ${placeholder}
+          </p>
+          <div id="${placeholder}" class="with ${placeholder}"></div>
+        `,
+          defaultMinifyOptions
+        );
+
+        console.log(minHtml);
 
+        // 8 placeholders, 9 parts
         expect(
-          defaultStrategy.getPlaceholder([
-            { text: '@TEMPLATE_EXPRESSION();', start: 0, end: 19 },
-            { text: '@TEMPLATE_EXPRESSION_();', start: 19, end: 38 }
-          ])
-        ).to.equal('@TEMPLATE_EXPRESSION__();');
+          defaultStrategy.splitHTMLByPlaceholder(minHtml, placeholder)
+        ).to.have.lengthOf(9);
       });
     });
 
@@ -51,11 +96,12 @@ describe('strategy', () => {
 
     describe('minifyHTML()', () => {
       it('should call minify() with html and options', () => {
+        const placeholder = defaultStrategy.getPlaceholder(parts);
         const html = `
-          <style>@TEMPLATE_EXPRESSION();</style>
-          <h1 class="heading">@TEMPLATE_EXPRESSION();</h1>
+          <style>${placeholder}</style>
+          <h1 class="heading">${placeholder}</h1>
           <ul>
-            <li>@TEMPLATE_EXPRESSION();</li>
+            <li>${placeholder}</li>
           </ul>
         `;