optimising ax and improving its clarity (#1820)

* optimising ax and improving clarity

* adding comment

* adding changeset

* updating comments

* further improving language in comments

* removing misleading comment

* further clarity for jsdoc

* trying out objects

* fixing up logic

* wip

* more optimised string creation

* updating ax

* updating test and bundle size values

* adding more comments and test cases

Author: alexreardon

.changeset/nasty-jars-roll.md

@@ -0,0 +1,9 @@
+---
+'@compiled/react': patch
+---
+
+Reducing bundle size and improving runtime performance of the `ax` runtime function.
+
+```ts
+import { ax } from '@compiled/react/runtime';
+```

package.json

@@ -136,7 +136,7 @@
     },
     {
       "path": "./packages/react/dist/browser/runtime/ax.js",
-      "limit": "195B",
+      "limit": "169B",
       "import": "ax"
     },
     {

packages/react/src/runtime/__perf__/ax.test.ts

@@ -3,42 +3,61 @@ import { runBenchmark } from '@compiled/benchmark';
 import { ax } from '../index';
 
 describe('ax benchmark', () => {
-  const arr = [
-    '_19itglyw',
-    '_2rko1l7b',
-    '_ca0qftgi',
-    '_u5f319bv',
-    '_n3tdftgi',
-    '_19bv19bv',
-    '_bfhk1mzw',
-    '_syazu67f',
-    '_k48p1nn1',
-    '_ect41kw7',
-    '_1wybdlk8',
-    '_irr3mlcl',
-    '_1di6vctu',
-    // `undefined` is an acceptable parameter so we want to include it in the test case.
-    // Example: ax(['aaaabbbb', foo() && "aaaacccc"])
-    undefined,
+  const chunks: string[] = ['aaaa', 'bbbb', 'cccc', 'dddd', 'eeee', 'ffff', 'gggg'];
+  const uniques: string[] = chunks.map((chunk) => `_${chunk}${chunk}`);
+  const withClashes: string[] = [
+    ...Array.from({ length: 4 }, () => `_${chunks[0]}${chunks[0]}`),
+    ...Array.from({ length: 6 }, () => `_${chunks[0]}${chunks[1]}`),
+    ...Array.from({ length: 8 }, () => `_${chunks[0]}${chunks[2]}`),
   ];
 
-  it('completes with ax() string as the fastest', async () => {
-    // Remove undefined and join the strings
-    const str = arr.slice(0, -1).join(' ');
+  const getRandomRules = (() => {
+    function randomChunk() {
+      return chunks[Math.floor(Math.random() * chunks.length)];
+    }
+
+    return function create(): string[] {
+      return Array.from({ length: 20 }, () => `_${randomChunk()}${randomChunk()}`);
+    };
+  })();
 
+  it('completes with ax() string as the fastest', async () => {
     const benchmark = await runBenchmark('ax', [
       {
-        name: 'ax() array',
-        fn: () => ax(arr),
+        name: 'ax() single',
+        fn: () => ax(['_aaaabbbb']),
+      },
+      {
+        name: 'ax() uniques (array)',
+        fn: () => ax(uniques),
+      },
+      {
+        name: 'ax() uniques (as a string)',
+        fn: () => ax([uniques.join(' ')]),
+      },
+      {
+        name: 'ax() clashes',
+        fn: () => ax(withClashes),
+      },
+      {
+        name: 'ax() clashes (as a string)',
+        fn: () => ax([withClashes.join(' ')]),
+      },
+      {
+        name: 'ax() random keys (no clashes)',
+        fn: () => ax(getRandomRules()),
       },
       {
-        name: 'ax() string',
-        fn: () => ax([str, undefined]),
+        name: 'ax() random keys (with clashes)',
+        fn: () => {
+          const random = getRandomRules();
+          ax([...random, ...random, ...random]);
+        },
       },
     ]);
 
     expect(benchmark).toMatchObject({
-      fastest: ['ax() string'],
+      fastest: ['ax() single'],
     });
-  }, 30000);
+  }, 90000);
 });

packages/react/src/runtime/__tests__/ax.test.ts

@@ -6,6 +6,7 @@ describe('ax', () => {
   it.each([
     ['should handle empty array', [], undefined],
     ['should handle array with undefined', [undefined], undefined],
+    ['should handle array with falsy values', [undefined, null, false as const, ''], undefined],
     ['should join single classes together', ['foo', 'bar'], 'foo bar'],
     ['should join multi classes together', ['foo baz', 'bar'], 'foo baz bar'],
     ['should remove undefined', ['foo', 'bar', undefined], 'foo bar'],
@@ -50,7 +51,8 @@ describe('ax', () => {
       ['hello_there', 'hello_world', '_aaaabbbb'],
       'hello_there hello_world _aaaabbbb',
     ],
-  ])('%s', (_, params, result) => {
-    expect(result).toEqual(ax(params));
+    ['should remove duplicate custom class names', ['a', 'a'], 'a'],
+  ])('%s', (_, params, expected) => {
+    expect(ax(params)).toEqual(expected);
   });
 });

packages/react/src/runtime/ax.ts

@@ -1,64 +1,96 @@
-const UNDERSCORE_UNICODE = 95;
-
 /**
  * This length includes the underscore,
  * e.g. `"_1s4A"` would be a valid atomic group hash.
  */
 const ATOMIC_GROUP_LENGTH = 5;
 
 /**
- * Joins classes together and ensures atomic declarations of a single group exist.
- * Atomic declarations take the form of `_{group}{value}` (always prefixed with an underscore),
- * where both `group` and `value` are hashes **four characters long**.
- * Class names can be of any length,
- * this function can take both atomic declarations and class names.
- *
- * Input:
+ * Create a single string containing all the classnames provided, separated by a space (`" "`).
+ * The result will only contain the _last_ atomic style classname for each atomic `group`.
  *
- * ```
- * ax(['_aaaabbbb', '_aaaacccc'])
+ * ```ts
+ * ax(['_aaaabbbb', '_aaaacccc']);
+ * // output
+ * '_aaaacccc'
  * ```
  *
- * Output:
+ * Format of Atomic style classnames: `_{group}{value}` (`_\w{4}\w{4}`)
  *
- * ```
- * '_aaaacccc'
- * ```
+ * `ax` will preserve any non atomic style classnames (eg `"border-red"`)
  *
- * @param classes
+ * ```ts
+ * ax(['_aaaabbbb', '_aaaacccc', 'border-red']);
+ * // output
+ * '_aaaacccc border-red'
+ * ```
  */
 export default function ax(classNames: (string | undefined | null | false)[]): string | undefined {
-  if (classNames.length <= 1 && (!classNames[0] || classNames[0].indexOf(' ') === -1)) {
-    // short circuit if there's no custom class names.
-    return classNames[0] || undefined;
+  // Shortcut: nothing to do
+  if (!classNames.length) {
+    return;
   }
 
-  const atomicGroups: Record<string, string> = {};
+  // Shortcut: don't need to do anything if we only have a single classname
+  if (
+    classNames.length === 1 &&
+    classNames[0] &&
+    // checking to see if `classNames[0]` is a string that contains other classnames
+    !classNames[0].includes(' ')
+  ) {
+    return classNames[0];
+  }
 
-  for (let i = 0; i < classNames.length; i++) {
-    const cls = classNames[i];
-    if (!cls) {
+  // Using an object rather than a `Map` as it performed better in our benchmarks.
+  // Would be happy to move to `Map` if it proved to be better under real conditions.
+  const map: Record<string, string> = {};
+
+  // Note: using loops to minimize iterations over the collection
+  for (const value of classNames) {
+    // Exclude all falsy values, which leaves us with populated strings
+    if (!value) {
       continue;
     }
 
-    const groups = cls.split(' ');
+    // a `value` can contain multiple classnames
+    const list = value.split(' ');
 
-    for (let x = 0; x < groups.length; x++) {
-      const atomic = groups[x];
-      const atomicGroupName = atomic.slice(
-        0,
-        atomic.charCodeAt(0) === UNDERSCORE_UNICODE ? ATOMIC_GROUP_LENGTH : undefined
-      );
-      atomicGroups[atomicGroupName] = atomic;
+    for (const className of list) {
+      /**
+       * For atomic style classnames: the `key` is the `group`
+       *
+       * - Later atomic classnames with the same `group` will override earlier ones
+       *   (which is what we want).
+       * - Assumes atomic classnames are the only things that start with `_`
+       * - Could use a regex to ensure that atomic classnames are structured how we expect,
+       *   but did not add that for now as it did slow things down a bit.
+       *
+       * For other classnames: the `key` is the whole classname
+       * - Okay to remove duplicates as doing so does not impact specificity
+       *
+       * */
+      const key = className.startsWith('_') ? className.slice(0, ATOMIC_GROUP_LENGTH) : className;
+      map[key] = className;
     }
   }
 
-  let str = '';
+  /**
+   * We are converting the `map` into a string.
+   *
+   * The simple way to do this would be `Object.values(map).join(' ')`.
+   * However, the approach below performs 10%-20% better in benchmarks.
+   *
+   * For `ax()` it feels right to squeeze as much runtime performance out as we can.
+   */
+  let result: string = '';
+  for (const key in map) {
+    result += map[key] + ' ';
+  }
 
-  for (const key in atomicGroups) {
-    const value = atomicGroups[key];
-    str += value + ' ';
+  // If we have an empty string, then our `map` was empty.
+  if (!result) {
+    return;
   }
 
-  return str.slice(0, -1);
+  // remove last " " from the result (we added " " at the end of every value)
+  return result.trimEnd();
 }