Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for custom equality checks on unwrap #123

Merged
merged 1 commit into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions packages/@react-facet/core/src/hooks/useFacetUnwrap.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,45 @@ it('does not trigger a re-render when changing a facet from undefined to undefin

expect(renderedMock).toHaveBeenCalledTimes(0)
})

it('supports custom equality checks', () => {
const value = {}
const demoFacet = createFacet({ initialValue: value })

// Dummy equality check that always returns its not equal
const check = jest.fn().mockReturnValue(false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also test for equal cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal of the test was to be able to validate that the check itself was being used, as the actual comparison (or usage of the check) is done within the createFacet function.

const equalityCheck = jest.fn().mockReturnValue(check)

const renderedMock = jest.fn()

const ComponentWithFacetEffect = () => {
useFacetUnwrap(demoFacet, equalityCheck)
renderedMock()
return null
}

render(<ComponentWithFacetEffect />)

// initialize equality checks once
expect(equalityCheck).toHaveBeenCalledTimes(1)

// but check for it twice, once upon initialization, then another on the first observed value
expect(check).toHaveBeenCalledTimes(2)
expect(check).toHaveBeenNthCalledWith(1, value)
expect(check).toHaveBeenNthCalledWith(2, value)

// as the custom equality check always returns false, we render twice on mount
expect(renderedMock).toHaveBeenCalledTimes(2)

jest.clearAllMocks()

// If we update with the same object,
act(() => {
demoFacet.set(value)
})

expect(equalityCheck).toHaveBeenCalledTimes(0) // equality check was already initialized
expect(check).toHaveBeenCalledTimes(1) // but the check should be executed
expect(check).toHaveBeenCalledWith(value) // passing the value (which should be the same)
expect(renderedMock).toHaveBeenCalledTimes(1) // and since the equality check always returns "false", we have a render
})
45 changes: 32 additions & 13 deletions packages/@react-facet/core/src/hooks/useFacetUnwrap.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { useLayoutEffect, useState } from 'react'
import { FacetProp, isFacet, Value, NoValue } from '../types'
import { FacetProp, isFacet, Value, NoValue, EqualityCheck, NO_VALUE } from '../types'
import { defaultEqualityCheck } from '../equalityChecks'

/**
* Hook that allows consuming values from a Facet
* It acts as a regular react state, triggering a re-render of the component
*
* @param facet
*/
export function useFacetUnwrap<T extends Value>(prop: FacetProp<T>): T | NoValue {
export function useFacetUnwrap<T extends Value>(
prop: FacetProp<T>,
equalityCheck: EqualityCheck<T> = defaultEqualityCheck,
): T | NoValue {
const [state, setState] = useState<{ value: T | NoValue }>(() => {
if (!isFacet(prop)) return { value: prop }

Expand All @@ -18,12 +22,17 @@ export function useFacetUnwrap<T extends Value>(prop: FacetProp<T>): T | NoValue

useLayoutEffect(() => {
if (isFacet(prop)) {
// Initialize the equalityCheck
const isEqual = equalityCheck()
const startValue = prop.get()
if (startValue !== NO_VALUE) {
isEqual(startValue)
}

return prop.observe((value) => {
setState((previousState) => {
const { value: previousValue } = previousState

const typeofValue = typeof previousValue

/**
* Performs this equality check locally to prevent triggering two consecutive renderings
* for facets that have immutable values (unfortunately we can't handle mutable values).
Expand All @@ -34,22 +43,32 @@ export function useFacetUnwrap<T extends Value>(prop: FacetProp<T>): T | NoValue
* - Once on initialization of the useState above
* - And another time on this observe initialization
*/
if (
(typeofValue === 'number' ||
typeofValue === 'string' ||
typeofValue === 'boolean' ||
value === undefined ||
value === null) &&
value === previousValue
) {
if (equalityCheck === defaultEqualityCheck) {
const typeofValue = typeof previousValue

if (
(typeofValue === 'number' ||
typeofValue === 'string' ||
typeofValue === 'boolean' ||
value === undefined ||
value === null) &&
value === previousValue
) {
return previousState
}

return { value }
}

if (previousValue !== NO_VALUE && isEqual(previousValue)) {
return previousState
}

return { value }
})
})
}
}, [prop])
}, [prop, equalityCheck])

return isFacet(prop) ? state.value : prop
}