Update sorting logic for a lot more shorthands and attempt runtime sorting as well.

Concerns:
 - Runtime sorting does not work properly as we'd also need per-bucket nested sorting where `@media, :focus, border` needs to be sorted.
 - `shorthandFor` needs 40 more shorthand properties defined…

Author: kylorhall-atlassian

packages/css/src/plugins/__tests__/sort-shorthand-declarations.test.ts

@@ -5,7 +5,7 @@ import { sortAtomicStyleSheet } from '../sort-atomic-style-sheet';
 
 const transform = (css: string, enabled = true) => {
   const result = postcss([
-    sortAtomicStyleSheet({ sortAtRulesEnabled: false, sortShorthandEnabled: enabled }),
+    sortAtomicStyleSheet({ sortAtRulesEnabled: true, sortShorthandEnabled: enabled }),
   ]).process(css, {
     from: undefined,
   });
@@ -184,6 +184,164 @@ describe('sort shorthand vs. longhand declarations', () => {
     `);
   });
 
+  it("sorts 4 layers deep of shorthands from 'all' to 'border-block-start'", () => {
+    const actual = transform(outdent`
+      @media all {
+        .f {
+          display: block;
+        }
+        .e {
+          border-block-start-color: transparent;
+        }
+        .d {
+          border-block-start: none;
+        }
+        .c {
+          border-top: none;
+        }
+        .b {
+          border: none;
+        }
+        .a {
+          all: unset;
+        }
+      }
+
+      .f:focus {
+        display: block;
+      }
+      .e:hover {
+        border-block-start-color: transparent;
+      }
+      .d:active {
+        border-block-start: none;
+      }
+      .c[data-foo='bar'] {
+        border-top: none;
+      }
+      .b[disabled] {
+        border: none;
+      }
+      .a > .external {
+        all: unset;
+      }
+
+      .f {
+        display: block;
+      }
+      .e {
+        border-block-start-color: transparent;
+      }
+      .d {
+        border-block-start: none;
+      }
+      .c {
+        border-top: none;
+      }
+      .b {
+        border: none;
+      }
+      .a {
+        all: unset;
+      }
+    `);
+
+    expect(actual).toMatchInlineSnapshot(`
+      "
+      .a {
+        all: unset;
+      }
+      .a > .external {
+        all: unset;
+      }
+      .b[disabled] {
+        border: none;
+      }
+      .c[data-foo='bar'] {
+        border-top: none;
+      }
+
+      .f {
+        display: block;
+      }
+      .b {
+        border: none;
+      }
+      .c {
+        border-top: none;
+      }
+      .d {
+        border-block-start: none;
+      }
+      .d:active {
+        border-block-start: none;
+      }
+      .e {
+        border-block-start-color: transparent;
+      }
+
+      .f:focus {
+        display: block;
+      }
+      .e:hover {
+        border-block-start-color: transparent;
+      }@media all {
+        .a {
+          all: unset;
+        }
+        .f {
+          display: block;
+        }
+        .b {
+          border: none;
+        }
+        .c {
+          border-top: none;
+        }
+        .d {
+          border-block-start: none;
+        }
+        .e {
+          border-block-start-color: transparent;
+        }
+      }"
+    `);
+  });
+
+  it('sorts non-atomic classes inline, but only singular declaration rules against each other', () => {
+    const actual = transform(outdent`
+      .e { border-top: none; }
+      .a {
+        border-block-start: 1px solid;
+        border-top: red;
+        all: reset;
+        border-block-start-color: transparent;
+        border: 2px dashed;
+      }
+      .f { border-block-start-color: transparent; }
+      .d { border: none; }
+      .c { all: unset; }
+      .b { all: unset; }
+    `);
+
+    // WARNING: This may be wrong as `.a { … }` is not sorted as we expect, it _should_ be 'abcdef' not 'eabcdf'.
+    // Is this a real scenario—multiple variables in a singular class?
+    expect(actual).toMatchInlineSnapshot(`
+      ".e { border-top: none; }
+      .a {
+        all: reset;
+        border: 2px dashed;
+        border-top: red;
+        border-block-start: 1px solid;
+        border-block-start-color: transparent;
+      }
+      .b { all: unset; }
+      .c { all: unset; }
+      .d { border: none; }
+      .f { border-block-start-color: transparent; }"
+    `);
+  });
+
   describe('when disabled', () => {
     it('does nothing when crossover detected', () => {
       const actual = transformWithoutSorting(outdent`

packages/css/src/plugins/sort-shorthand-declarations.ts

@@ -1,18 +1,51 @@
-import type { ChildNode } from 'postcss';
-
-// TODO: Would need a full list from https://developer.mozilla.org/en-US/docs/Web/CSS/Shorthand_properties
-// We _kind_ of have some of this in `expand-shorthands`, but only partially.
-const shorthandFor: { [key: string]: string[] } = {
-  font: [
-    'font-style',
-    'font-variant',
-    'font-weight',
-    'font-stretch',
-    'font-size',
-    'line-height',
-    'font-family',
-  ],
-  outline: ['outline-color', 'outline-style', 'outline-width'],
+import { shorthandFor } from '@compiled/utils';
+import type { ChildNode, Declaration } from 'postcss';
+
+const nodeIsDeclaration = (node: ChildNode): node is Declaration => node.type === 'decl';
+
+const findDeclaration = (node: ChildNode): Declaration | Declaration[] | undefined => {
+  if (nodeIsDeclaration(node)) {
+    return node;
+  }
+
+  if ('nodes' in node) {
+    if (node.nodes.length === 1 && nodeIsDeclaration(node.nodes[0])) {
+      return node.nodes[0];
+    }
+
+    const declarations = node.nodes.map(findDeclaration).filter(Boolean) as Declaration[];
+
+    if (declarations.length === 1) {
+      return declarations[0];
+    }
+
+    return declarations;
+  }
+};
+
+const sortNodes = (a: ChildNode, b: ChildNode): number => {
+  const aDecl = findDeclaration(a);
+  const bDecl = findDeclaration(b);
+
+  // Don't worry about any array of declarations, this would  be something like a group of AtRule versus a regular Rule
+  // Those are sorted elsewhere…
+  if (Array.isArray(aDecl) || Array.isArray(bDecl)) return 0;
+
+  if (!aDecl?.prop || !bDecl?.prop) return 0;
+
+  console.log('@@vs', aDecl.prop + ':' + aDecl.value, bDecl.prop + ':' + bDecl.value);
+
+  const aShorthand = shorthandFor[aDecl.prop];
+  if (aShorthand === true || aShorthand?.includes(bDecl.prop)) {
+    return -1;
+  }
+
+  const bShorthand = shorthandFor[bDecl.prop];
+  if (bShorthand === true || bShorthand?.includes(aDecl.prop)) {
+    return 1;
+  }
+
+  return 0;
 };
 
 export const sortShorthandDeclarations = (nodes: ChildNode[]): void => {
@@ -25,18 +58,5 @@ export const sortShorthandDeclarations = (nodes: ChildNode[]): void => {
     }
   });
 
-  nodes.sort((a, b) => {
-    if (a.type !== 'decl' || b.type !== 'decl') return 0;
-    if (!a.prop || !b.prop) return 0;
-
-    if (shorthandFor[a.prop]?.includes(b.prop)) {
-      return -1;
-    }
-
-    if (shorthandFor[b.prop]?.includes(a.prop)) {
-      return 1;
-    }
-
-    return 0;
-  });
+  nodes.sort(sortNodes);
 };

packages/react/package.json

@@ -72,6 +72,7 @@
     "jsx-dev-runtime"
   ],
   "dependencies": {
+    "@compiled/utils": "^0.11.1",
     "csstype": "^3.1.2"
   },
   "devDependencies": {

packages/react/src/runtime/__tests__/style-ssr.test.tsx

@@ -43,12 +43,19 @@ describe('<Style />', () => {
           `._g1234567:visited{ color: grey; }`,
           `._h1234567:focus-visible{ color: white; }`,
           `._i1234567:focus-within{ color: black; }`,
+          `._j1234567{ border: none; }`,
+          `._j1234567{ border-top: 1px solid transparent; }`,
+          `._j1234567{ border-top-color: red; }`,
+          `._j1234567:focus{ border-top-color: transparent; }`,
+          `._j1234567:focus{ border-color: blue; }`,
+          `._j1234567:focus{ border: 1px solid; }`,
         ]}
       </Style>
     );
 
+    // WARNING: This is wrong, the `focus` border properties are different than without focus, borders would be indeterministic.
     expect(result.split('</style>').join('</style>\n')).toMatchInlineSnapshot(`
-      "<style data-cmpld="true">._c1234567{ display: block; }._d1234567:link{ color: green; }._g1234567:visited{ color: grey; }._i1234567:focus-within{ color: black; }._f1234567:focus{ color: pink; }._h1234567:focus-visible{ color: white; }._a1234567:hover{ color: red; }._b1234567:active{ color: blue; }@media (max-width: 800px){ ._e1234567{ color: yellow; } }</style>
+      "<style data-cmpld="true">._j1234567{ border: none; }._j1234567{ border-top: 1px solid transparent; }._c1234567{ display: block; }._j1234567{ border-top-color: red; }._d1234567:link{ color: green; }._g1234567:visited{ color: grey; }._i1234567:focus-within{ color: black; }._f1234567:focus{ color: pink; }._j1234567:focus{ border-top-color: transparent; }._j1234567:focus{ border-color: blue; }._j1234567:focus{ border: 1px solid; }._h1234567:focus-visible{ color: white; }._a1234567:hover{ color: red; }._b1234567:active{ color: blue; }@media (max-width: 800px){ ._e1234567{ color: yellow; } }</style>
       "
     `);
   });

packages/react/src/runtime/sheet.ts

@@ -1,3 +1,5 @@
+import { getShorthandDepth } from '@compiled/utils';
+
 import { isCacheDisabled } from './cache';
 import type { Bucket, StyleSheetOpts } from './types';
 
@@ -6,6 +8,11 @@ import type { Bucket, StyleSheetOpts } from './types';
  * If changes are needed make sure that it aligns with the definition in `sort-at-rule-pseudos.tsx`.
  */
 export const styleBucketOrdering: Bucket[] = [
+  // shorthand properties
+  's-root',
+  's-1',
+  's-2',
+  's-3',
   // catch-all
   '',
   // link
@@ -114,15 +121,26 @@ export const getStyleBucketName = (sheet: string): Bucket => {
     return 'm';
   }
 
+  const firstBracket = sheet.indexOf('{');
+
   /**
    * We assume that classname will always be 9 character long,
-   * using this the 10th character could be a pseudo declaration.
+   * using this the 10th characters could be a pseudo declaration.
    */
   if (sheet.charCodeAt(10) === 58 /* ":" */) {
     // We send through a subset of the string instead of the full pseudo name.
     // For example `"focus-visible"` name would instead of `"us-visible"`.
     // Return a mapped pseudo else the default catch all bucket.
-    return pseudosMap[sheet.slice(14, sheet.indexOf('{'))] || '';
+    const mapped = pseudosMap[sheet.slice(14, firstBracket)];
+    if (mapped) return mapped;
+  }
+
+  const property = sheet.slice(firstBracket + 1, sheet.indexOf(':', firstBracket)).trim();
+
+  const shorthandDepth = getShorthandDepth(property);
+  if (shorthandDepth) {
+    // NOTE: This doesn't actually work fully because there's various _layers_ of shorthands — up to 4 deep, I believe…
+    return `s-${shorthandDepth}` as const;
   }
 
   // Return default catch all bucket

packages/react/src/runtime/types.ts

@@ -11,6 +11,11 @@ export interface StyleSheetOpts {
  * Buckets under which we will group our stylesheets
  */
 export type Bucket =
+  // shorthand properties
+  | 's-root'
+  | 's-1'
+  | 's-2'
+  | 's-3'
   // catch-all
   | ''
   // link

packages/react/tsconfig.browser.json

@@ -12,5 +12,5 @@
     ],
     "tsBuildInfoFile": "tsconfig.browser.tsbuildinfo"
   },
-  "references": [{ "path": "../benchmark" }, { "path": "../jest" }]
+  "references": [{ "path": "../benchmark" }, { "path": "../jest" }, { "path": "../utils" }]
 }

packages/react/tsconfig.cjs.json

@@ -5,5 +5,5 @@
     "outDir": "dist/cjs",
     "tsBuildInfoFile": "tsconfig.cjs.tsbuildinfo"
   },
-  "references": [{ "path": "../benchmark" }, { "path": "../jest" }]
+  "references": [{ "path": "../benchmark" }, { "path": "../jest" }, { "path": "../utils" }]
 }

packages/react/tsconfig.json

@@ -4,5 +4,5 @@
     "outDir": "dist/esm",
     "tsBuildInfoFile": "tsconfig.esm.tsbuildinfo"
   },
-  "references": [{ "path": "../benchmark" }, { "path": "../jest" }]
+  "references": [{ "path": "../benchmark" }, { "path": "../jest" }, { "path": "../utils" }]
 }

packages/utils/src/index.ts

@@ -8,3 +8,4 @@ export { createError } from './error';
 export { preserveLeadingComments } from './preserve-leading-comments';
 export { INCREASE_SPECIFICITY_SELECTOR } from './increase-specificity';
 export { DEFAULT_PARSER_BABEL_PLUGINS } from './default-parser-babel-plugins';
+export { shorthandFor, getShorthandDepth } from './shorthand';