fix: do not crash on var(--font-stack, family1, family2, etc) (#436)

Author: bartveneman

src/index.js

@@ -444,6 +444,7 @@ export function analyze(css, options = {}) {
         let { property, important } = declaration
         let complexity = 1
 
+        // i.e. `background-image: -webkit-linear-gradient()`
         if (isValuePrefixed(node)) {
           vendorPrefixedValues.p(stringifyNode(node), node.loc)
           complexity++
@@ -484,9 +485,11 @@ export function analyze(css, options = {}) {
           if (font_family) {
             fontFamilies.p(font_family, loc)
           }
+
           if (font_size) {
             fontSizes.p(font_size, loc)
           }
+
           if (line_height) {
             lineHeights.p(line_height, loc)
           }

src/values/destructure-font-shorthand.js

@@ -41,9 +41,12 @@ export function isSystemFont(node) {
  */
 export function destructure(value, stringifyNode, cb) {
 	let font_family = Array.from({ length: 2 })
+	/** @type {string | undefined} */
 	let font_size
+	/** @type {string | undefined} */
 	let line_height
 
+	// FIXME: in case of a var(--my-stack, fam1, fam2, fam3) this forEach also loops over all children of the var()
 	value.children.forEach(function (node, item) {
 		let prev = item.prev ? item.prev.data : undefined
 		let next = item.next ? item.next.data : undefined
@@ -76,7 +79,7 @@ export function destructure(value, stringifyNode, cb) {
 		) {
 			font_family[0] = node
 
-			if (!font_size && prev !== null) {
+			if (!font_size && prev) {
 				font_size = stringifyNode(prev)
 			}
 
@@ -112,21 +115,22 @@ export function destructure(value, stringifyNode, cb) {
 		}
 	})
 
+	let family = (font_family[0] || font_family[1])
+		? stringifyNode({
+			loc: {
+				start: {
+					offset: (font_family[0] || font_family[1]).loc.start.offset,
+				},
+				end: {
+					offset: font_family[1].loc.end.offset,
+				},
+			},
+		})
+		: null
+
 	return {
 		font_size,
 		line_height,
-		font_family:
-			font_family[0] || font_family[1]
-				? stringifyNode({
-					loc: {
-						start: {
-							offset: (font_family[0] || font_family[1]).loc.start.offset,
-						},
-						end: {
-							offset: font_family[1].loc.end.offset,
-						},
-					},
-				})
-				: null,
+		font_family: family,
 	}
 }

src/values/font-families.test.js

@@ -53,29 +53,44 @@ FontFamilies('extracts the `font` shorthand', () => {
       font: 11px Consolas, "Liberation Mono", Menlo, Courier, monospace;
       font: 0/0 a; /* As generated by some css minifiers */
       font: 1.2em/1.2em; /* Found in indiatimes.com */
+      font: 12px var(--fontStack-monospace, ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace); /* From github.com */
 
       /* Unrelated */
       color: brown;
-      font-size: 12px;
+      font-size: 123456789px;
     }
   `
   const actual = analyze(fixture).values.fontFamilies
   const expected = {
-    total: 10,
-    totalUnique: 6,
+    total: 12,
+    totalUnique: 8,
     unique: {
       [`'Noto Sans'`]: 1,
       '"Source Sans Pro", serif': 1,
       'serif': 5,
       '-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol"': 1,
       'Consolas, "Liberation Mono", Menlo, Courier, monospace': 1,
       'a': 1,
+      'var(--fontStack-monospace, ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace)': 1,
+      // This entry exists due to a ??bug?? in CSSTree, but it's better than not counting the value above this as well
+      'ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace': 1,
     },
-    uniquenessRatio: 6 / 10
+    uniquenessRatio: 8 / 12
   }
   assert.equal(actual, expected)
 })
 
+FontFamilies('does not crash on `12px var(...)', () => {
+  const fixture = `
+    test {
+      font: 12px var(--fontStack-monospace, ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace);
+    }
+  `
+  assert.not.throws(() => {
+    analyze(fixture).values.fontFamilies
+  })
+})
+
 FontFamilies('handles system fonts', () => {
   // Source: https://drafts.csswg.org/css-fonts-3/#font-prop
   const fixture = `

src/values/font-sizes.test.js

@@ -48,6 +48,7 @@ FontSizes('extracts the `font` shorthand', () => {
       font: 11px Consolas, "Liberation Mono", Menlo, Courier, monospace;
       font: 0/0 a; /* As generated by some css minifiers */
       font: 10PX sans-serif;
+      font: 12px var(--fontStack-monospace, ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace); /* from github.com */
 
       /* Unrelated */
       color: brown;
@@ -56,8 +57,8 @@ FontSizes('extracts the `font` shorthand', () => {
   `
   const actual = analyze(fixture).values.fontSizes
   const expected = {
-    total: 12,
-    totalUnique: 8,
+    total: 13,
+    totalUnique: 9,
     unique: {
       '0': 1,
       'large': 1,
@@ -67,8 +68,9 @@ FontSizes('extracts the `font` shorthand', () => {
       '2em': 1,
       '11px': 2,
       '10PX': 1,
+      '12px': 1,
     },
-    uniquenessRatio: 8 / 12
+    uniquenessRatio: 9 / 13
   }
   assert.equal(actual, expected)
 })