fix(postcss-minify-font-values): fix font declaration output (#1562)

* fix: add additional space before font-family if needed (fixes #1519)

* fix(postcss-minify-font-values): fix wrong output when input is missing spaces

Add the missing spaces before minifying.

Fix #1519, close #1520

test(postcss-minify-font-values): refactor to make tests more readable

Input and output are now directly strings.

---------

Co-authored-by: Sergey Melyukov <s.melukov@gmail.com>

.changeset/cyan-squids-own.md

@@ -0,0 +1,5 @@
+---
+"postcss-minify-font-values": patch
+---
+
+fix(postcss-minify-font-values): emit correct output when spaces are missing between the font family and other values

packages/postcss-minify-font-values/src/index.js

@@ -32,11 +32,7 @@ function transform(prop, value, opts) {
 
     return tree.toString();
   } else if (lowerCasedProp === 'font') {
-    const tree = valueParser(value);
-
-    tree.nodes = minifyFont(tree.nodes, opts);
-
-    return tree.toString();
+    return minifyFont(value, opts);
   }
 
   return value;

packages/postcss-minify-font-values/src/lib/minify-font.js

@@ -1,37 +1,61 @@
 'use strict';
-const { unit } = require('postcss-value-parser');
+const valueParser = require('postcss-value-parser');
 const keywords = require('./keywords');
 const minifyFamily = require('./minify-family');
 const minifyWeight = require('./minify-weight');
 
 /**
+ * Adds missing spaces before strings.
+ *
+ * @param toBeSpliced {Set<number>}
  * @param {import('postcss-value-parser').Node[]} nodes
+ * @return {void}
+ */
+function normalizeNodes(nodes, toBeSpliced) {
+  for (const index of toBeSpliced) {
+    nodes.splice(
+      index,
+      0,
+      /** @type {import('postcss-value-parser').SpaceNode} */ ({
+        type: 'space',
+        value: ' ',
+      })
+    );
+  }
+}
+
+/**
+ * @param {string} unminified
  * @param {import('../index').Options} opts
- * @return {import('postcss-value-parser').Node[]}
+ * @return {string}
  */
-module.exports = function (nodes, opts) {
-  let i, max, node, family;
+module.exports = function (unminified, opts) {
+  const tree = valueParser(unminified);
+  const nodes = tree.nodes;
+
   let familyStart = NaN;
   let hasSize = false;
+  const toBeSpliced = new Set();
 
-  for (i = 0, max = nodes.length; i < max; i += 1) {
-    node = nodes[i];
+  for (const [i, node] of nodes.entries()) {
+    if (node.type === 'string' && i > 0 && nodes[i - 1].type !== 'space') {
+      toBeSpliced.add(i);
+    }
 
     if (node.type === 'word') {
       if (hasSize) {
         continue;
       }
 
       const value = node.value.toLowerCase();
-
       if (
         value === 'normal' ||
         value === 'inherit' ||
         value === 'initial' ||
         value === 'unset'
       ) {
         familyStart = i;
-      } else if (keywords.style.has(value) || unit(value)) {
+      } else if (keywords.style.has(value) || valueParser.unit(value)) {
         familyStart = i;
       } else if (keywords.variant.has(value)) {
         familyStart = i;
@@ -40,7 +64,7 @@ module.exports = function (nodes, opts) {
         familyStart = i;
       } else if (keywords.stretch.has(value)) {
         familyStart = i;
-      } else if (keywords.size.has(value) || unit(value)) {
+      } else if (keywords.size.has(value) || valueParser.unit(value)) {
         familyStart = i;
         hasSize = true;
       }
@@ -56,9 +80,11 @@ module.exports = function (nodes, opts) {
     }
   }
 
+  normalizeNodes(nodes, toBeSpliced);
   familyStart += 2;
 
-  family = minifyFamily(nodes.slice(familyStart), opts);
+  const family = minifyFamily(nodes.slice(familyStart), opts);
 
-  return nodes.slice(0, familyStart).concat(family);
+  tree.nodes = nodes.slice(0, familyStart).concat(family);
+  return tree.toString();
 };

packages/postcss-minify-font-values/test/minify-font.js

@@ -3,39 +3,49 @@ const { test } = require('uvu');
 const assert = require('uvu/assert');
 const minifyFont = require('../src/lib/minify-font.js');
 
-const tests = [
-  {
-    options: {},
-    fixture: [
-      { type: 'word', value: 'bold' },
-      { type: 'space', value: '  ' },
-      { type: 'word', value: 'italic' },
-      { type: 'space', value: ' \t ' },
-      { type: 'word', value: '20px' },
-      { type: 'space', value: ' \n ' },
-      { type: 'word', value: 'Times' },
-      { type: 'space', value: ' ' },
-      { type: 'word', value: 'New' },
-      { type: 'space', value: ' \t ' },
-      { type: 'word', value: 'Roman' },
-      { type: 'div', value: ',', before: '', after: ' ' },
-      { type: 'word', value: 'serif' },
-    ],
-    expected: [
-      { type: 'word', value: '700' },
-      { type: 'space', value: '  ' },
-      { type: 'word', value: 'italic' },
-      { type: 'space', value: ' \t ' },
-      { type: 'word', value: '20px' },
-      { type: 'space', value: ' \n ' },
-      { type: 'word', value: 'Times New Roman,serif' },
-    ],
-  },
-];
+test('.8em "Times New Roman", Arial, Helvetica, sans-serif', () => {
+  assert.equal(
+    minifyFont(
+      '.8em "Times New Roman", Arial, Helvetica, sans-serif',
 
-test('minify-font', () => {
-  tests.forEach(({ fixture, options, expected }) => {
-    assert.equal(minifyFont(fixture, options), expected);
-  });
+      { removeQuotes: true }
+    ),
+    '.8em Times New Roman,Arial,Helvetica,sans-serif'
+  );
 });
+
+test('.8em"Times New Roman", Arial, Helvetica, sans-serif', () => {
+  assert.equal(
+    minifyFont('.8em"Times New Roman", Arial, Helvetica, sans-serif', {
+      removeQuotes: true,
+    }),
+    '.8em Times New Roman,Arial,Helvetica,sans-serif'
+  );
+});
+
+test('ultra-condensed small-caps 1.2em "Fira Sans", sans-serif;', () => {
+  assert.equal(
+    minifyFont('ultra-condensed small-caps 1.2em "Fira Sans", sans-serif;', {
+      removeQuotes: true,
+    }),
+    'ultra-condensed small-caps 1.2em Fira Sans,sans-serif;'
+  );
+});
+
+test('ultra-condensed small-caps 1.2em"Fira Sans", sans-serif;', () => {
+  assert.equal(
+    minifyFont('ultra-condensed small-caps 1.2em"Fira Sans", sans-serif;', {
+      removeQuotes: true,
+    }),
+    'ultra-condensed small-caps 1.2em Fira Sans,sans-serif;'
+  );
+});
+
+test('tabs and newlines', () => {
+  assert.equal(
+    minifyFont('bold italic \t 20px \n Times New\tRoman, serif', {}),
+    '700 italic \t 20px \n Times New Roman,serif'
+  );
+});
+
 test.run();

packages/postcss-minify-font-values/types/lib/minify-font.d.ts

@@ -1,2 +1,2 @@
-declare function _exports(nodes: import('postcss-value-parser').Node[], opts: import('../index').Options): import('postcss-value-parser').Node[];
+declare function _exports(unminified: string, opts: import('../index').Options): string;
 export = _exports;