fix: Prevent memory leak.

h2oai · Aug 16, 2023 · 9a98b0e · 9a98b0e
1 parent 36b1ca4
commit 9a98b0e
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 30 deletions.
diff --git a/ui/src/nav.test.tsx b/ui/src/nav.test.tsx
@@ -168,31 +168,31 @@ describe('Nav.tsx', () => {
     it('Selects nav item when value is updated to the same value twice', () => {
       const
         props: T.Model<State> = { ...navProps, state: { items } },
-        checkIfFirstItemIsSelected = () => {
+        expectFirstSelected = () => {
           expect(getByTitle('Nav 1').parentElement).toHaveClass('is-selected')
           expect(getByTitle('Nav 2').parentElement).not.toHaveClass('is-selected')
         },
-        checkIfSecondItemIsSelected = () => {
+        expectSecondSelected = () => {
           expect(getByTitle('Nav 1').parentElement).not.toHaveClass('is-selected')
           expect(getByTitle('Nav 2').parentElement).toHaveClass('is-selected')
         },
         { rerender, getByTitle } = render(<View {...props} />)
 
-      checkIfFirstItemIsSelected()
+      expectFirstSelected()
 
       props.state.value = 'nav2'
       rerender(<View {...props} />)
       expect(wave.args['nav2']).toBe(true)
-      checkIfSecondItemIsSelected()
+      expectSecondSelected()
 
       fireEvent.click(getByTitle('Nav 1'))
       expect(wave.args['nav1']).toBe(true)
-      checkIfFirstItemIsSelected()
+      expectFirstSelected()
 
       props.state.value = 'nav2'
       rerender(<View {...props} />)
       expect(wave.args['nav2']).toBe(true)
-      checkIfSecondItemIsSelected()
+      expectSecondSelected()
     })
 
     it('Does not set args on value update when name starts with hash', () => {

diff --git a/ui/src/nav.tsx b/ui/src/nav.tsx
@@ -147,38 +147,39 @@ export const
     return <Fluent.Nav groups={groups} selectedKey={valueB() || groups[0].links[0].key} styles={{ groupContent: { marginBottom: 0 } }} />
   },
   View = bond(({ name, state, changed }: Model<State>) => {
-    const valueB = box<S | undefined>(state.value)
-    const render = () => {
-      const { title, subtitle, icon, color = 'card', icon_color = color === 'primary' ? '$card' : '$text', image, persona, secondary_items } = state
-      return (
-        <div data-test={name} className={clas(getEffectClass(toCardEffect(color)), css.card)} style={{ background: cssVar(`$${color}`) }}>
-          <div className={css.header}>
-            {(image || icon) && (
-              <div className={css.brand}>
-                {image && <img src={image} className={css.img} />}
-                {icon && !image && <Fluent.Icon iconName={icon} className={css.icon} style={{ color: cssVar(icon_color) }} />}
-              </div>
-            )}
-            {title && <div className={clas('wave-s24 wave-w6', color === 'card' ? 'wave-p9' : 'wave-c9')}>{title}</div>}
-            {subtitle && <div className={clas('wave-s13', color === 'card' ? 'wave-t8' : 'wave-c8')}>{subtitle}</div>}
-            {!image && !icon && persona?.persona && <div className={css.persona}><XPersona model={persona.persona} /></div>}
-          </div>
-          <XNav {...state} linksOnly={!image && !icon && !title && !subtitle && !persona} valueB={valueB} />
-          {secondary_items && <div className={css.secondaryItems} style={{ marginTop: state.items.length ? 'auto' : 'initial' }}><XComponents items={secondary_items} /></div>}
-        </div>)
-    },
+    const
+      valueB = box<S | undefined>(state.value),
+      valueWatcher = on(valueB, val => state.value = val),
+      render = () => {
+        const { title, subtitle, icon, color = 'card', icon_color = color === 'primary' ? '$card' : '$text', image, persona, secondary_items } = state
+        return (
+          <div data-test={name} className={clas(getEffectClass(toCardEffect(color)), css.card)} style={{ background: cssVar(`$${color}`) }}>
+            <div className={css.header}>
+              {(image || icon) && (
+                <div className={css.brand}>
+                  {image && <img src={image} className={css.img} />}
+                  {icon && !image && <Fluent.Icon iconName={icon} className={css.icon} style={{ color: cssVar(icon_color) }} />}
+                </div>
+              )}
+              {title && <div className={clas('wave-s24 wave-w6', color === 'card' ? 'wave-p9' : 'wave-c9')}>{title}</div>}
+              {subtitle && <div className={clas('wave-s13', color === 'card' ? 'wave-t8' : 'wave-c8')}>{subtitle}</div>}
+              {!image && !icon && persona?.persona && <div className={css.persona}><XPersona model={persona.persona} /></div>}
+            </div>
+            <XNav {...state} linksOnly={!image && !icon && !title && !subtitle && !persona} valueB={valueB} />
+            {secondary_items && <div className={css.secondaryItems} style={{ marginTop: state.items.length ? 'auto' : 'initial' }}><XComponents items={secondary_items} /></div>}
+          </div>)
+      },
       update = (prevProps: Model<State>) => {
         if (prevProps.state.value === valueB()) return
         valueB(prevProps.state.value)
         const name = prevProps.state.value || prevProps.state.items[0].items[0].name
 
         if (name.startsWith('#')) window.location.hash = name.substring(1)
         else wave.args[name] = true
-      }
-
-    on(valueB, val => state.value = val)
+      },
+      dispose = () => valueWatcher.dispose()
 
-    return { render, changed, update, valueB }
+    return { render, changed, update, valueB, dispose }
   })
 
 cards.register('nav', View, { effect: CardEffect.Flat, marginless: true })