Refactor expand flex shorthands to handle the CSS spec correctly (#1205)

* Refactor expand flex shorthands to handle the CSS spec correctly

* Add more width units to utils

.changeset/red-donkeys-decide.md

@@ -0,0 +1,5 @@
+---
+'@compiled/css': patch
+---
+
+Fix flex shorthand expansion not following CSS specification

packages/css/src/plugins/expand-shorthands/__tests__/flex.test.ts

@@ -18,9 +18,9 @@ describe('flex property expander', () => {
 
     expect(result).toMatchInlineSnapshot(`
       "
-            flex-grow: auto;
-            flex-shrink: initial;
-            flex-basis: none;
+            flex-grow: 0;
+            flex-shrink: 0;
+            flex-basis: auto;
           "
     `);
   });
@@ -81,6 +81,34 @@ describe('flex property expander', () => {
     `);
   });
 
+  it('should expand flex auto', () => {
+    const result = transform`
+      flex: 3 2 auto;
+    `;
+
+    expect(result).toMatchInlineSnapshot(`
+      "
+            flex-grow: 3;
+            flex-shrink: 2;
+            flex-basis: auto;
+          "
+    `);
+  });
+
+  it('should expand flex function call', () => {
+    const result = transform`
+      flex: 3 2 calc(50px + 50px);
+    `;
+
+    expect(result).toMatchInlineSnapshot(`
+      "
+            flex-grow: 3;
+            flex-shrink: 2;
+            flex-basis: calc(50px + 50px);
+          "
+    `);
+  });
+
   it('should remove decls for invalid single', () => {
     const result = transform`
       flex: asd;
@@ -131,8 +159,8 @@ describe('flex property expander', () => {
     `;
 
     expect(result).toMatchInlineSnapshot(`
-    "
-        "
-  `);
+          "
+              "
+      `);
   });
 });

packages/css/src/plugins/expand-shorthands/__tests__/utils.test.ts

@@ -48,6 +48,22 @@ describe('expand utils', () => {
     expect(actual).toBe(false);
   });
 
+  it('should return true for fit-content', () => {
+    const value = parse(`fit-content`);
+
+    const actual = isWidth(value.nodes[0]);
+
+    expect(actual).toBe(true);
+  });
+
+  it('should return true for fit-content(5em)', () => {
+    const value = parse(`fit-content(5em)`);
+
+    const actual = isWidth(value.nodes[0]);
+
+    expect(actual).toBe(true);
+  });
+
   it('should return true for a valid color word', () => {
     const value = parse('red');
 
@@ -95,4 +111,12 @@ describe('expand utils', () => {
 
     expect(actual).toBe(true);
   });
+
+  it('should return true for a valid color hsla', () => {
+    const value = parse('hsla(188, 97%, 28%, .3)');
+
+    const actual = isColor(value.nodes[0]);
+
+    expect(actual).toBe(true);
+  });
 });

packages/css/src/plugins/expand-shorthands/flex.ts

@@ -1,93 +1,79 @@
+import type { ChildNode, Numeric, Word, Func } from 'postcss-values-parser';
+
 import type { ConversionFunction } from './types';
 import { getWidth, isWidth } from './utils';
 
+const isFlexNumber = (node: ChildNode): node is Numeric => node.type === 'numeric' && !node.unit;
+const isFlexBasis = (node: ChildNode): node is Numeric | Word | Func =>
+  (node.type === 'word' && node.value === 'content') || isWidth(node);
+
 /**
- * https://developer.mozilla.org/en-US/docs/Web/CSS/flex
+ * https://drafts.csswg.org/css-flexbox-1/#flex-property
  */
 export const flex: ConversionFunction = (value) => {
   const [left, middle, right] = value.nodes;
-  let grow: number | string = 'auto';
-  let shrink: number | string = 'initial';
-  let basis: number | string = 'none';
 
   switch (value.nodes.length) {
     case 1: {
-      if (left.type === 'numeric' && !left.unit) {
-        grow = left.value;
-        shrink = 1;
-        basis = 0;
+      if (left.type === 'word' && left.value == 'none') {
+        // none is equivalent to 0 0 auto
+        return [
+          { prop: 'flex-grow', value: 0 },
+          { prop: 'flex-shrink', value: 0 },
+          { prop: 'flex-basis', value: 'auto' },
+        ];
+      } else if (isFlexNumber(left)) {
+        // flex grow
+        return [
+          { prop: 'flex-grow', value: left.value },
+          { prop: 'flex-shrink', value: 1 },
+          { prop: 'flex-basis', value: 0 },
+        ];
+      } else if (isFlexBasis(left)) {
+        // flex basis
+        return [
+          { prop: 'flex-grow', value: 1 },
+          { prop: 'flex-shrink', value: 1 },
+          { prop: 'flex-basis', value: getWidth(left) },
+        ];
       }
-      if (left.type === 'word' && left.value !== 'none') {
-        // Invalid
-        return [];
-      }
-
       break;
     }
 
     case 2: {
-      if (left.type === 'numeric' && !left.unit) {
-        grow = left.value;
-      } else {
-        return [];
-      }
-
-      if (middle.type === 'numeric' && !middle.unit) {
-        shrink = middle.value;
-        basis = 0;
-      } else if (isWidth(middle)) {
-        shrink = 1;
-        const value = getWidth(middle);
-        if (value) {
-          basis = value;
-        } else {
-          // Invalid
-          return [];
+      if (isFlexNumber(left)) {
+        if (isFlexNumber(middle)) {
+          // flex grow and flex shrink
+          return [
+            { prop: 'flex-grow', value: left.value },
+            { prop: 'flex-shrink', value: middle.value },
+            { prop: 'flex-basis', value: 0 },
+          ];
+        } else if (isFlexBasis(middle)) {
+          // flex grow and flex basis
+          return [
+            { prop: 'flex-grow', value: left.value },
+            { prop: 'flex-shrink', value: 1 },
+            { prop: 'flex-basis', value: getWidth(middle) },
+          ];
         }
-      } else {
-        // Invalid
-        return [];
       }
-
       break;
     }
 
     case 3: {
-      if (left.type === 'numeric' && !left.unit) {
-        grow = left.value;
-      } else {
-        return [];
+      if (isFlexNumber(left) && isFlexNumber(middle) && isFlexBasis(right)) {
+        // flex grow, flex shrink, and flex basis
+        return [
+          { prop: 'flex-grow', value: left.value },
+          { prop: 'flex-shrink', value: middle.value },
+          { prop: 'flex-basis', value: getWidth(right) },
+        ];
       }
-
-      if (middle.type === 'numeric' && !middle.unit) {
-        shrink = middle.value;
-        basis = 0;
-      }
-
-      if (isWidth(right)) {
-        const value = getWidth(right);
-        if (value) {
-          basis = value;
-        } else {
-          // Invalid
-          return [];
-        }
-      } else {
-        // Invalid
-        return [];
-      }
-
       break;
     }
-
-    default:
-      // Invalid
-      return [];
   }
 
-  return [
-    { prop: 'flex-grow', value: grow },
-    { prop: 'flex-shrink', value: shrink },
-    { prop: 'flex-basis', value: basis },
-  ];
+  // Invalid CSS
+  return [];
 };

packages/css/src/plugins/expand-shorthands/utils.ts

@@ -1,9 +1,9 @@
-import type { Node } from 'postcss-values-parser';
+import type { Node, Numeric, Word, Func } from 'postcss-values-parser';
 
 /**
  * Common global values
  */
-export const globalValues = ['inherit', 'initial', 'unset'];
+export const globalValues = ['inherit', 'initial', 'unset', 'revert', 'revert-layer'];
 
 /**
  * Returns `true` if the node is a color,
@@ -15,38 +15,53 @@ export const isColor = (node: Node): boolean => {
   return (node.type === 'word' || node.type === 'func') && node.isColor;
 };
 
+const widthUnits = new Set([
+  '%',
+  'cap',
+  'ch',
+  'cm',
+  'em',
+  'ex',
+  'fr',
+  'ic',
+  'in',
+  'lh',
+  'mm',
+  'pc',
+  'pt',
+  'px',
+  'Q',
+  'rem',
+  'rlh',
+  'vb',
+  'vh',
+  'vi',
+  'vmax',
+  'vmin',
+  'vw',
+]);
+
 /**
  * Returns `true` if the node is a width,
  * else `false`.
  *
  * @param node
  */
 export const isWidth = (node: Node): boolean => {
-  if (node.type === 'numeric') {
-    if (
-      [
-        'px',
-        'rem',
-        'em',
-        '%',
-        'pt',
-        'cm',
-        'mm',
-        'Q',
-        'in',
-        'pc',
-        'ex',
-        'ch',
-        'lh',
-        'vw',
-        'vh',
-        'vmin',
-        'vmax',
-        'fr',
-      ].includes(node.unit)
-    ) {
-      return true;
-    }
+  if (node.type === 'numeric' && widthUnits.has(node.unit)) {
+    return true;
+  }
+
+  if (
+    node.type === 'word' &&
+    [...globalValues, 'auto', 'min-content', 'max-content', 'fit-content'].includes(node.value)
+  ) {
+    return true;
+  }
+
+  if (node.type === 'func') {
+    // We don't want to be strict about functions, as we don't know the return type
+    return true;
   }
 
   return false;
@@ -57,14 +72,14 @@ export const isWidth = (node: Node): boolean => {
  *
  * @param node
  */
-export const getWidth = (node: Node): string | undefined => {
+export const getWidth = (node: Numeric | Word | Func): string => {
   if (node.type === 'numeric') {
     return `${node.value}${node.unit}`;
   }
 
-  if (node.type === 'word') {
-    return node.value;
+  if (node.type === 'func') {
+    return `${node.name}${node.params}`;
   }
 
-  return undefined;
+  return node.value;
 };