perf: reduce accesses to reactive variables

swordev · Apr 23, 2023 · f7dc98e · f7dc98e
1 parent 99a2ff1
commit f7dc98e
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 81 deletions.
diff --git a/.changeset/loud-months-camp.md b/.changeset/loud-months-camp.md
@@ -0,0 +1,7 @@
+---
+"@suid/styled-engine": patch
+"@suid/system": patch
+"@suid/utils": patch
+---
+
+Improve performance in styled components
diff --git a/packages/styled-engine/src/createStyle.tsx b/packages/styled-engine/src/createStyle.tsx
@@ -26,14 +26,14 @@ type StyleProps =
   | Record<string, any>
   | (Record<string, any> | undefined)[];
 
-function normalizeStyleProps(props: StyleProps) {
-  if (!props) return [];
-  return (
-    (Array.isArray(props) ? props : [props])
-      // https://github.com/microsoft/TypeScript/issues/44408
-      .flat(Infinity as 1)
-      .filter((v) => !!v) as Record<string, any>[]
-  );
+function normalizeStyleProps(props: StyleProps): Record<string, any>[] {
+  if (!props) {
+    return [];
+  } else if (Array.isArray(props)) {
+    return props.flat(Infinity as 1).filter((v) => !!v);
+  } else {
+    return [props];
+  }
 }
 
 function createStyleId() {

diff --git a/packages/system/src/Box/Box.test.tsx b/packages/system/src/Box/Box.test.tsx
@@ -23,7 +23,7 @@ describe("Box", () => {
       <Box data-testid="e" sx={{ color: "red" }} component="a" as="p" />
     ));
     const e = screen.getByTestId("e");
-    expect(e.getAttributeNames().join(",")).toBe("data-testid,class");
+    expect(e.getAttributeNames().join(",")).toBe("class,data-testid");
     unmount();
   });
   it("uses host component", () => {
@@ -45,7 +45,7 @@ describe("Box", () => {
     ));
     const e = screen.getByTestId("e");
     expect(e.nodeName).toBe("A");
-    expect(e.getAttributeNames().join(",")).toBe("data-testid,href,class");
+    expect(e.getAttributeNames().join(",")).toBe("class,data-testid,href");
     expect(e.getAttribute("href")).toBe("/");
     expect(window.getComputedStyle(e).color).toBe("red");
     unmount();
@@ -73,7 +73,7 @@ describe("Box", () => {
       <Box data-testid="e" sx={{ color: "red" }} as="a" href="/" />
     ));
     const e = screen.getByTestId("e");
-    expect(e.getAttributeNames().join(",")).toBe("data-testid,href,class");
+    expect(e.getAttributeNames().join(",")).toBe("class,data-testid,href");
     expect(e.nodeName).toBe("A");
     expect(e.getAttribute("href")).toBe("/");
     expect(window.getComputedStyle(e).color).toBe("red");

diff --git a/packages/system/src/Dynamic/Dynamic.tsx b/packages/system/src/Dynamic/Dynamic.tsx
@@ -23,42 +23,42 @@ function createElement(
 }
 
 // https://github.com/solidjs/solid/blob/cafc51f6fc2e3ddf24b40cae00b77cf879051c9e/packages/solid/web/server/index.ts#L25
-function ServerDynamic<T>(
+function createServerDynamicComponent<T>(
+  component: () => Function | string,
   props: T & {
     children?: any;
-    $component?: Component<T> | string | keyof JSX.IntrinsicElements;
   }
 ) {
-  const [p, others] = splitProps(props, ["$component"]);
-  const comp = p.$component,
+  const comp = component(),
     t = typeof comp;
 
   if (comp) {
-    if (t === "function") return (comp as Function)(others);
+    if (t === "function") return (comp as Function)(props);
     else if (t === "string") {
-      return ssrElement(comp as string, others, undefined, true);
+      return ssrElement(comp as string, props, undefined, true);
     }
   }
 }
 
-// https://github.com/solidjs/solid/blob/12c0dbbbf9f9fdf798c6682e57aee8ea763cf1ba/packages/solid/web/src/index.ts#L114
-export function Dynamic(props: any): JSX.Element {
-  if (isServer) return ServerDynamic(props);
-  const [p, others] = splitProps(props, ["$component"]);
-  const cached = createMemo<Function | string>(() => p.$component);
+export function createDynamicComponent(
+  component: () => Function | string,
+  props: any
+): JSX.Element {
+  if (isServer) return createServerDynamicComponent(component, props);
+  const cached = createMemo<Function | string>(component);
   return createMemo(() => {
     const component = cached();
     switch (typeof component) {
       case "function":
         if ("_DX_DEV_") Object.assign(component, { [$DEVCOMP]: true });
-        return untrack(() => component(others));
+        return untrack(() => component(props));
 
       case "string":
         const isSvg = web.SVGElements.has(component);
         const el = sharedConfig.context
           ? web.getNextElement()
           : createElement(component, isSvg);
-        spread(el, others, isSvg);
+        spread(el, props, isSvg);
         return el;
 
       default:
@@ -67,4 +67,11 @@ export function Dynamic(props: any): JSX.Element {
   }) as unknown as JSX.Element;
 }
 
+// https://github.com/solidjs/solid/blob/12c0dbbbf9f9fdf798c6682e57aee8ea763cf1ba/packages/solid/web/src/index.ts#L114
+export function Dynamic(props: any): JSX.Element {
+  const [p, others] = splitProps(props, ["$component"]);
+  if (isServer) return createServerDynamicComponent(() => p.$component, others);
+  return createDynamicComponent(() => p.$component, others);
+}
+
 export default Dynamic;
diff --git a/packages/system/src/createStyled.tsx b/packages/system/src/createStyled.tsx
@@ -1,4 +1,4 @@
-import Dynamic from "./Dynamic";
+import { createDynamicComponent } from "./Dynamic";
 import createStyle from "./createStyle";
 import type { Theme } from "./createTheme/createTheme";
 import resolveStyledProps from "./resolveStyledProps";
@@ -14,7 +14,7 @@ import {
 } from "@suid/types";
 import { randomString } from "@suid/utils";
 import clsx from "clsx";
-import { Component, createMemo } from "solid-js";
+import { Component, createMemo, mergeProps } from "solid-js";
 import { ComponentProps as _ComponentProps, JSX, splitProps } from "solid-js";
 
 export interface ComponentProps<T, O> {
@@ -67,14 +67,19 @@ function resolveStyles<T extends Theme<any>, P, O>(
 ) {
   return createMemo(() => {
     const theme = useTheme();
+    const ownerState = inProps.ownerState;
     return styles.reduce((result, style) => {
       let styledProps: StyledProps | false | undefined;
       if (typeof style === "function") {
         styledProps = style({
-          ownerState: inProps.ownerState,
+          ownerState,
           theme,
-          sx: inProps.sx,
-          as: inProps.as,
+          get sx() {
+            return inProps.sx;
+          },
+          get as() {
+            return inProps.as;
+          },
           props: inProps as never as P,
         });
       } else if (style) {
@@ -112,6 +117,7 @@ function createStyled<
     } else {
       className = `styled-${randomString()}`;
     }
+    const isComponentStyled = isStyledComponent(Component);
 
     return function <
       O = N extends keyof CM
@@ -157,63 +163,64 @@ function createStyled<
           inProps
         );
 
-        const inSx: () => SxPropsObject[] = createMemo(() =>
-          !options.skipSx && inProps.sx
-            ? Array.isArray(inProps.sx)
-              ? inProps.sx
-              : [inProps.sx]
-            : []
-        );
+        const inSx = () => {
+          const sx = inProps.sx;
+          return (sx ? (Array.isArray(sx) ? sx : [sx]) : []) as SxPropsObject[];
+        };
+
+        const $component = () => {
+          if (isComponentStyled) return Component;
+          const as = inProps.as;
+          return as ? as : Component;
+        };
+
+        const is$ComponentStyled = isComponentStyled
+          ? () => true
+          : createMemo(() => isStyledComponent($component()));
 
         const sx = () => {
           const theme = $useTheme();
           return [
             ...inStyles().map((v) => ({ ...v, __resolved: true })),
-            ...inSx().map((sx) =>
-              (sx as any).__resolved ? sx : resolveSxProps(sx, theme)
-            ),
+            ...(options.skipSx
+              ? []
+              : inSx().map((sx) =>
+                  (sx as any).__resolved ? sx : resolveSxProps(sx, theme)
+                )),
           ];
         };
 
-        const styledComponent = createMemo(() => isStyledComponent(Component));
-
-        const $component = () =>
-          inProps.as && !styledComponent() ? inProps.as : Component;
-
-        const styled$Component = createMemo(() =>
-          isStyledComponent($component())
+        const styleClassName = createStyle(() =>
+          is$ComponentStyled() ? undefined : sx()
         );
 
-        const as = () =>
-          inProps.as && styledComponent() ? inProps.as : undefined;
-
-        const styledProps = () =>
-          styled$Component() && {
-            ownerState: inProps.ownerState,
+        return createDynamicComponent(
+          $component,
+          mergeProps(otherProps, {
+            get children() {
+              return (otherProps as any).children;
+            },
+            // [review] This property must be omitted on each component individually.
+            get component() {
+              return is$ComponentStyled()
+                ? (otherProps as any).component
+                : null;
+            },
+            get as() {
+              return isComponentStyled ? inProps.as : undefined;
+            },
             get sx() {
-              return sx();
+              return is$ComponentStyled() ? sx() : undefined;
             },
-          };
-
-        const styleClassName = createStyle(() =>
-          styled$Component() ? undefined : sx()
-        );
-
-        // [review] This property must be omitted on each component individually.
-        const component = () =>
-          styled$Component() ? (otherProps as any).component : null;
-
-        return (
-          <Dynamic
-            {...otherProps}
-            $component={$component()}
-            component={component()}
-            as={as()}
-            {...styledProps()}
-            class={clsx([
-              ...new Set([inProps.class, className, styleClassName()]),
-            ])}
-          />
+            get ownerState() {
+              return is$ComponentStyled() ? inProps.ownerState : undefined;
+            },
+            get class() {
+              return clsx([
+                ...new Set([inProps.class, className, styleClassName()]),
+              ]);
+            },
+          })
         );
       }
 

diff --git a/packages/utils/src/deepmerge.ts b/packages/utils/src/deepmerge.ts
@@ -25,6 +25,7 @@ export default function deepmerge<T>(
   options: DeepmergeOptions = { clone: true }
 ): T {
   const output = options.clone ? { ...target } : target;
+  const sourceKeys: string[] | undefined = options.sortKeys ? [] : undefined;
 
   if (isPlainObject(target) && isPlainObject(source)) {
     Object.keys(source).forEach((key) => {
@@ -33,24 +34,28 @@ export default function deepmerge<T>(
         return;
       }
 
+      if (sourceKeys) sourceKeys.push(key);
+
+      let sourceValue: any;
+      let targetValue: any;
+
       if (
-        isPlainObject(source[key]) &&
+        isPlainObject((sourceValue = source[key])) &&
         key in target &&
-        isPlainObject(target[key])
+        isPlainObject((targetValue = target[key]))
       ) {
         // Since `output` is a clone of `target` and we have narrowed `target` in this block we can cast to the same type.
         (output as Record<keyof any, unknown>)[key] = deepmerge(
-          target[key],
-          source[key],
+          targetValue,
+          sourceValue,
           options
         );
       } else {
-        (output as Record<keyof any, unknown>)[key] = source[key];
+        (output as Record<keyof any, unknown>)[key] = sourceValue;
       }
     });
 
-    if (options.sortKeys)
-      sortKeys(output as Record<string, any>, Object.keys(source) as string[]);
+    if (sourceKeys) sortKeys(output as Record<string, any>, sourceKeys);
   }
 
   return output;