fix(perf): release references to node styles

chakra-ui · Nov 20, 2024 · c794e08 · c794e08
1 parent 9054ca4
commit c794e08
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 13 deletions.
diff --git a/.changeset/old-flowers-destroy.md b/.changeset/old-flowers-destroy.md
@@ -0,0 +1,6 @@
+---
+"@zag-js/presence": patch
+"@zag-js/collapsible": patch
+---
+
+Fix memory leak where machine might hold on to the element and it's styles after element is unmounted
diff --git a/.xstate/collapsible.js b/.xstate/collapsible.js
@@ -18,7 +18,7 @@ const fetchMachine = createMachine({
     "isOpenControlled": false,
     "isOpenControlled": false
   },
-  exit: ["clearInitial"],
+  exit: ["clearInitial", "cleanupNode"],
   on: {
     UPDATE_CONTEXT: {
       actions: "updateContext"
@@ -74,7 +74,10 @@ const fetchMachine = createMachine({
         }, {
           target: "closing",
           actions: ["setInitial", "computeSize", "invokeOnClose"]
-        }]
+        }],
+        "SIZE.MEASURE": {
+          actions: ["measureSize"]
+        }
       }
     }
   }

diff --git a/.xstate/presence.js b/.xstate/presence.js
@@ -12,7 +12,7 @@ const {
 const fetchMachine = createMachine({
   initial: ctx.present ? "mounted" : "unmounted",
   context: {},
-  exit: ["clearInitial"],
+  exit: ["clearInitial", "cleanupNode"],
   on: {
     "NODE.SET": {
       actions: ["setNode", "setStyles"]

diff --git a/examples/next-ts/components/toolbar.tsx b/examples/next-ts/components/toolbar.tsx
@@ -1,19 +1,20 @@
 import { dataAttr } from "@zag-js/dom-query"
 import { ReactNode, useState } from "react"
 
-type ToolbarProps = {
+type ToolbarProps = React.HTMLAttributes<HTMLDivElement> & {
   controls?: null | (() => JSX.Element)
   children: ReactNode
   viz?: boolean
 }
 
 export function Toolbar(props: ToolbarProps) {
-  const [active, setActive] = useState(props.viz ? 1 : !props.controls ? 1 : 0)
+  const { controls, children, viz, ...rest } = props
+  const [active, setActive] = useState(viz ? 1 : !controls ? 1 : 0)
 
   return (
-    <div className="toolbar">
+    <div className="toolbar" {...rest}>
       <nav>
-        {props.controls && (
+        {controls && (
           <button data-active={dataAttr(active === 0)} onClick={() => setActive(0)}>
             Controls
           </button>
@@ -23,13 +24,13 @@ export function Toolbar(props: ToolbarProps) {
         </button>
       </nav>
       <div>
-        {props.controls && (
+        {controls && (
           <div data-content data-active={dataAttr(active === 0)}>
-            <props.controls />
+            {controls()}
           </div>
         )}
         <div data-content data-active={dataAttr(active === 1)}>
-          {props.children}
+          {children}
         </div>
       </div>
     </div>

diff --git a/examples/next-ts/pages/compositions b/examples/next-ts/pages/compositions
diff --git a/packages/machines/collapsible/src/collapsible.connect.ts b/packages/machines/collapsible/src/collapsible.connect.ts
@@ -18,6 +18,9 @@ export function connect<T extends PropTypes>(state: State, send: Send, normalize
     disabled,
     visible,
     open,
+    measureSize() {
+      send("SIZE.MEASURE")
+    },
     setOpen(nextOpen) {
       if (nextOpen === open) return
       send(nextOpen ? "OPEN" : "CLOSE")
@@ -35,6 +38,7 @@ export function connect<T extends PropTypes>(state: State, send: Send, normalize
     getContentProps() {
       return normalize.element({
         ...parts.content.attrs,
+        "data-collapsible": "",
         "data-state": skip ? undefined : open ? "open" : "closed",
         id: dom.getContentId(state.context),
         "data-disabled": dataAttr(disabled),

diff --git a/packages/machines/collapsible/src/collapsible.machine.ts b/packages/machines/collapsible/src/collapsible.machine.ts
@@ -24,7 +24,7 @@ export function machine(userContext: UserDefinedContext) {
         open: ["setInitial", "computeSize", "toggleVisibility"],
       },
 
-      exit: ["clearInitial"],
+      exit: ["clearInitial", "cleanupNode"],
 
       states: {
         closed: {
@@ -91,6 +91,9 @@ export function machine(userContext: UserDefinedContext) {
                 actions: ["setInitial", "computeSize", "invokeOnClose"],
               },
             ],
+            "SIZE.MEASURE": {
+              actions: ["measureSize"],
+            },
           },
         },
       },
@@ -145,6 +148,16 @@ export function machine(userContext: UserDefinedContext) {
         clearInitial(ctx) {
           ctx.initial = false
         },
+        cleanupNode(ctx) {
+          ctx.stylesRef = null
+        },
+        measureSize(ctx) {
+          const contentEl = dom.getContentEl(ctx)
+          if (!contentEl) return
+          const { height, width } = contentEl.getBoundingClientRect()
+          ctx.height = height
+          ctx.width = width
+        },
         computeSize(ctx, evt) {
           ctx._rafCleanup?.()
 

diff --git a/packages/machines/collapsible/src/collapsible.types.ts b/packages/machines/collapsible/src/collapsible.types.ts
@@ -116,6 +116,10 @@ export interface MachineApi<T extends PropTypes = PropTypes> {
    * Function to open or close the collapsible.
    */
   setOpen(open: boolean): void
+  /**
+   * Function to measure the size of the content.
+   */
+  measureSize(): void
 
   getRootProps(): T["element"]
   getTriggerProps(): T["button"]

diff --git a/packages/machines/presence/src/presence.machine.ts b/packages/machines/presence/src/presence.machine.ts
@@ -16,7 +16,7 @@ export function machine(ctx: Partial<UserDefinedContext>) {
         ...ctx,
       },
 
-      exit: ["clearInitial"],
+      exit: ["clearInitial", "cleanupNode"],
 
       watch: {
         present: ["setInitial", "syncPresence"],
@@ -83,6 +83,10 @@ export function machine(ctx: Partial<UserDefinedContext>) {
         clearInitial(ctx) {
           ctx.initial = false
         },
+        cleanupNode(ctx) {
+          ctx.node = null
+          ctx.styles = null
+        },
         invokeOnExitComplete(ctx) {
           ctx.onExitComplete?.()
         },