Fix <With /> bug where children are not unmounted when given data is null | undefined. (#131)

Author: marlonicus

packages/@react-facet/core/src/components/With.spec.tsx

@@ -1,9 +1,9 @@
 import React from 'react'
-import { render } from '@react-facet/dom-fiber-testing-library'
+import { act, render } from '@react-facet/dom-fiber-testing-library'
 import { With } from '.'
 import { createFacet } from '../facet'
 import { Facet, NO_VALUE } from '../types'
-import { useFacetMap } from '../hooks'
+import { useFacetEffect, useFacetMap } from '../hooks'
 
 it('renders when not null, passing down the information', () => {
   const userFacet = createFacet({ initialValue: { user: 'Zelda' } })
@@ -65,3 +65,34 @@ it('does not render facet has no value', () => {
 
   expect(rendered).not.toHaveBeenCalled()
 })
+
+it('correctly handles unmounting', () => {
+  const mockFacet = createFacet<string | null>({ initialValue: 'abc' })
+
+  const Content = ({ data }: { data: Facet<string> }) => {
+    useFacetEffect(
+      (data) => {
+        if (data === null || data === undefined) {
+          throw new Error('data should not be null')
+        }
+      },
+      [],
+      [data],
+    )
+
+    return <>mounted</>
+  }
+
+  const Example = () => {
+    return <With data={mockFacet}>{(mock) => <Content data={mock} />}</With>
+  }
+  const scenario = <Example />
+
+  const result = render(scenario)
+
+  expect(result.container).toHaveTextContent('mounted')
+
+  act(() => mockFacet.set(null))
+
+  expect(result.container).not.toHaveTextContent('mounted')
+})

packages/@react-facet/core/src/components/With.tsx

@@ -1,19 +1,23 @@
 import { ReactElement } from 'react'
+import { useFacetMemo } from '../hooks/useFacetMemo'
 import { useFacetUnwrap } from '../hooks/useFacetUnwrap'
 import { useFacetMap } from '../hooks/useFacetMap'
-import { Facet, NoValue } from '../types'
+import { Facet, NO_VALUE } from '../types'
 
 type WithProps<T> = {
   data: Facet<T | null | undefined>
   children: (data: Facet<T>) => ReactElement | null
 }
 
-const hasData = <T,>(_: Facet<T | null | undefined>, shouldRender: boolean | NoValue): _ is Facet<T> => {
-  return shouldRender === true
-}
-
+/**
+ * Conditionally renders a child if a given facet value is not null or undefined
+ *
+ * @param data facet value which can be null or undefined
+ * @param children render prop which receives the transformed facet
+ */
 export const With = <T,>({ data, children }: WithProps<T>) => {
   const shouldRenderFacet = useFacetMap((data) => data !== null && data !== undefined, [], [data])
   const shouldRender = useFacetUnwrap(shouldRenderFacet)
-  return hasData(data, shouldRender) ? children(data) : null
+  const nonNullData = useFacetMemo((data) => (data !== null && data !== undefined ? data : NO_VALUE), [], [data])
+  return shouldRender === true ? children(nonNullData) : null
 }