Skip shorthand property expansion if CSS variable given as value (#1467)

* Skip shorthand property expansion if CSS variable given as value

* Update tests

* Remove useless line from test case

* Add changeset

Author: dddlr

.changeset/big-walls-boil.md

@@ -0,0 +1,6 @@
+---
+'@compiled/css': minor
+'@compiled/babel-plugin': patch
+---
+
+Skip expansion of shorthand properties (e.g. padding, margin) if they have dynamic values (e.g. CSS variables, ternary expressions, arrow functions)

packages/babel-plugin/src/css-prop/__tests__/object-literal.test.ts

@@ -606,10 +606,7 @@ describe('css prop object literal', () => {
        }}>hello world</div>
     `);
 
-    expect(actual).toInclude('{padding-top:0}');
-    expect(actual).toInclude('{padding-right:var(--_1xlms2h)}');
-    expect(actual).toInclude('{padding-bottom:0}');
-    expect(actual).toInclude('{padding-left:var(--_1xlms2h)}');
+    expect(actual).toInclude('{padding:0 var(--_1xlms2h)}');
   });
 
   it('should parse an inline string interpolation delimited by multiple spaces', () => {
@@ -625,10 +622,7 @@ describe('css prop object literal', () => {
        }}>hello world</div>
     `);
 
-    expect(actual).toInclude('{padding-top:0}');
-    expect(actual).toInclude('{padding-right:var(--_1xlms2h)}');
-    expect(actual).toInclude('{padding-bottom:0}');
-    expect(actual).toInclude('{padding-left:0}');
+    expect(actual).toInclude('{padding:0 var(--_1xlms2h) 0 0}');
   });
 
   it('should parse an inline string interpolation delimited by multiple spaces and suffix', () => {
@@ -784,4 +778,27 @@ describe('css prop object literal', () => {
 
     expect(actual).toInclude('{display:grid}');
   });
+
+  it('should correctly expand shorthand property with ternary expression', () => {
+    const actual = transform(`
+    import { css } from '@compiled/react';
+
+    const morePadding = true;
+
+    <div css={{
+        padding: morePadding ? "10px 20px 30px 40px" : "4px 8px",
+    }}>
+      Hello world
+    </div>
+    `);
+
+    expect(actual).toMatchInlineSnapshot(`
+      "import*as React from'react';import{ax,ix,CC,CS}from"@compiled/react/runtime";const _4="._19bv1ylp{padding-left:40px}";const _3="._n3td1ul9{padding-bottom:30px}";const _2="._u5f3gktf{padding-right:20px}";const _="._ca0q19bv{padding-top:10px}";const morePadding=true;<CC>
+          <CS>{[_,_2,_3,_4]}</CS>
+          {<div className={ax(["_ca0q19bv _u5f3gktf _n3td1ul9 _19bv1ylp"])}>
+            Hello world
+          </div>}
+        </CC>;"
+    `);
+  });
 });

packages/babel-plugin/src/styled/__tests__/call-expression.test.ts

@@ -641,4 +641,82 @@ describe('styled object call expression', () => {
       "
     `);
   });
+
+  it('should refuse to expand shorthand property when value is unknown at build time (arrow function)', () => {
+    const actual = transform(`
+    import { styled } from '@compiled/react';
+
+    const Container = styled.div({
+      padding: ({ customPadding }) => customPadding,
+    });
+    `);
+
+    expect(actual).toMatchInlineSnapshot(`
+      "const _ = "._1yt414tu{padding:var(--_1hhnq9y)}";
+      const Container = forwardRef(
+        ({ as: C = "div", style: __cmpls, ...__cmplp }, __cmplr) => {
+          const { customPadding, ...__cmpldp } = __cmplp;
+          return (
+            <CC>
+              <CS>{[_]}</CS>
+              <C
+                {...__cmpldp}
+                style={{
+                  ...__cmpls,
+                  "--_1hhnq9y": ix(__cmplp.customPadding),
+                }}
+                ref={__cmplr}
+                className={ax(["_1yt414tu", __cmplp.className])}
+              />
+            </CC>
+          );
+        }
+      );
+      "
+    `);
+  });
+
+  it('should refuse to expand shorthand property when value is unknown at build time (ternary expression)', () => {
+    const actual = transform(`
+    import { styled } from '@compiled/react';
+
+    const Container = styled.div({
+      padding: ({ morePadding }) => morePadding ? morePadding : '4px 8px',
+    });
+    `);
+
+    expect(actual).toMatchInlineSnapshot(`
+      "const _5 = "._19bvftgi{padding-left:8px}";
+      const _4 = "._n3td1y44{padding-bottom:4px}";
+      const _3 = "._u5f3ftgi{padding-right:8px}";
+      const _2 = "._ca0q1y44{padding-top:4px}";
+      const _ = "._1yt41v0o{padding:var(--_1dm0vu2)}";
+      const Container = forwardRef(
+        ({ as: C = "div", style: __cmpls, ...__cmplp }, __cmplr) => {
+          const { morePadding, ...__cmpldp } = __cmplp;
+          return (
+            <CC>
+              <CS>{[_, _2, _3, _4, _5]}</CS>
+              <C
+                {...__cmpldp}
+                style={{
+                  ...__cmpls,
+                  "--_1dm0vu2": ix(__cmplp.morePadding),
+                }}
+                ref={__cmplr}
+                className={ax([
+                  "",
+                  __cmplp.morePadding
+                    ? "_1yt41v0o"
+                    : "_ca0q1y44 _u5f3ftgi _n3td1y44 _19bvftgi",
+                  __cmplp.className,
+                ])}
+              />
+            </CC>
+          );
+        }
+      );
+      "
+    `);
+  });
 });

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

@@ -1,5 +1,5 @@
 import type { Plugin } from 'postcss';
-import { parse } from 'postcss-values-parser';
+import { parse, type ChildNode } from 'postcss-values-parser';
 
 import { background } from './background';
 import { flex } from './flex';
@@ -56,6 +56,15 @@ const shorthands: Record<string, ConversionFunction> = {
    */
 };
 
+const valueIsNotSafeToExpand = (node: ChildNode): boolean => {
+  // This is the case where a CSS variable is given as the value, e.g.
+  // `padding: var(--_fl6vf6)`. Value of _fl6vf6 is unknown, so this
+  // cannot be expanded safely.
+  //
+  // https://github.com/atlassian-labs/compiled/issues/1331
+  return node.type === 'func' && node.isVar;
+};
+
 /**
  * PostCSS plugin that expands shortform properties to their longform equivalents.
  */
@@ -68,12 +77,17 @@ export const expandShorthands = (): Plugin => {
       if (!expand) {
         return;
       }
+
       const valueNode = parse(decl.value);
+      if (valueNode.nodes.some(valueIsNotSafeToExpand)) {
+        return;
+      }
 
       const longforms = expand(valueNode);
       if (!longforms) {
         throw new Error('Longform properties were not returned!');
       }
+
       /** Return early if not replacing a node */
       if (longforms.length === 1 && longforms[0].prop === undefined) {
         return;