-
Notifications
You must be signed in to change notification settings - Fork 18
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Move responsibility of cleaning up effects from facets to `useEffect(…
…)` / `useFacetEffect()` (#96) * - Remove cleanup logic in createFacet.ts - Re-implement cleanup logic in useFacetEffect() - Create a factory for creating useFacetEffect() and useFacetLayoutEffect() - Exclude /dist folder when running unit tests * Update the description of the integration unit test * Use workspace ranges for dependencies. See this for more info: https://yarnpkg.com/features/workspaces#workspace-ranges-workspace * Revert "Use workspace ranges for dependencies." This reverts commit 5b76def. * Update docs for contributing * Use effectMemoized and make the function returned from createUseFacetEffect unnamed. * Minor adjustment to contributions docs
- Loading branch information
1 parent
2b19d17
commit ec2ea3b
Showing
12 changed files
with
182 additions
and
359 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
108 changes: 63 additions & 45 deletions
108
packages/@react-facet/core/src/hooks/useFacetEffect.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,57 +1,75 @@ | ||
import { useCallback, useEffect } from 'react' | ||
import { useCallback, useEffect, useLayoutEffect } from 'react' | ||
import { Facet, Unsubscribe, Cleanup, NO_VALUE, ExtractFacetValues } from '../types' | ||
|
||
/** | ||
* Allows running an effect based on facet updates. Similar to React's useEffect. | ||
* | ||
* @param effect function that will do the side-effect (ex: update the DOM) | ||
* @param dependencies variable used by the map that are available in scope (similar as dependencies of useEffect) | ||
* @param facets facets that the effect listens to | ||
* | ||
* We pass the dependencies of the callback as the second argument so we can leverage the eslint-plugin-react-hooks option for additionalHooks. | ||
* Having this as the second argument allows the linter to work. | ||
*/ | ||
export function useFacetEffect<Y extends Facet<unknown>[], T extends [...Y]>( | ||
effect: (...args: ExtractFacetValues<T>) => void | Cleanup, | ||
dependencies: unknown[], | ||
facets: T, | ||
) { | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
const effectMemoized = useCallback(effect as (...args: unknown[]) => ReturnType<typeof effect>, dependencies) | ||
|
||
useEffect(() => { | ||
if (facets.length === 1) { | ||
return facets[0].observe(effectMemoized) | ||
} | ||
|
||
let cleanup: void | Cleanup | ||
let hasAllDependencies = false | ||
const unsubscribes: Unsubscribe[] = [] | ||
const values: unknown[] = facets.map(() => NO_VALUE) | ||
|
||
facets.forEach((facet, index) => { | ||
unsubscribes[index] = facet.observe((value) => { | ||
values[index] = value | ||
hasAllDependencies = hasAllDependencies || values.every((value) => value != NO_VALUE) | ||
|
||
if (hasAllDependencies) { | ||
export const createUseFacetEffect = (useHook: typeof useEffect | typeof useLayoutEffect) => { | ||
return function <Y extends Facet<unknown>[], T extends [...Y]>( | ||
effect: (...args: ExtractFacetValues<T>) => void | Cleanup, | ||
dependencies: unknown[], | ||
facets: T, | ||
) { | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
const effectMemoized = useCallback(effect as (...args: unknown[]) => ReturnType<typeof effect>, dependencies) | ||
|
||
useHook(() => { | ||
let cleanup: void | Cleanup | ||
|
||
if (facets.length === 1) { | ||
const unsubscribe = facets[0].observe((value) => { | ||
if (cleanup != null) { | ||
cleanup() | ||
} | ||
|
||
cleanup = effect(...(values as ExtractFacetValues<T>)) | ||
cleanup = effectMemoized(value) | ||
}) | ||
|
||
return () => { | ||
unsubscribe() | ||
if (cleanup != null) { | ||
cleanup() | ||
} | ||
} | ||
} | ||
|
||
let hasAllDependencies = false | ||
const unsubscribes: Unsubscribe[] = [] | ||
const values: unknown[] = facets.map(() => NO_VALUE) | ||
|
||
facets.forEach((facet, index) => { | ||
unsubscribes[index] = facet.observe((value) => { | ||
values[index] = value | ||
hasAllDependencies = hasAllDependencies || values.every((value) => value != NO_VALUE) | ||
|
||
if (hasAllDependencies) { | ||
if (cleanup != null) { | ||
cleanup() | ||
} | ||
|
||
cleanup = effectMemoized(...(values as ExtractFacetValues<T>)) | ||
} | ||
}) | ||
}) | ||
}) | ||
|
||
return () => { | ||
unsubscribes.forEach((unsubscribe) => unsubscribe()) | ||
if (cleanup != null) { | ||
cleanup() | ||
return () => { | ||
unsubscribes.forEach((unsubscribe) => unsubscribe()) | ||
if (cleanup != null) { | ||
cleanup() | ||
} | ||
} | ||
} | ||
|
||
// We care about each individual facet and if any is a different reference | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, [effectMemoized, ...facets]) | ||
// We care about each individual facet and if any is a different reference | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, [effectMemoized, ...facets]) | ||
} | ||
} | ||
|
||
/** | ||
* Allows running an effect based on facet updates. Similar to React's useEffect. | ||
* | ||
* @param effect function that will do the side-effect (ex: update the DOM) | ||
* @param dependencies variable used by the map that are available in scope (similar as dependencies of useEffect) | ||
* @param facets facets that the effect listens to | ||
* | ||
* We pass the dependencies of the callback as the second argument so we can leverage the eslint-plugin-react-hooks option for additionalHooks. | ||
* Having this as the second argument allows the linter to work. | ||
*/ | ||
export const useFacetEffect = createUseFacetEffect(useEffect) |
Oops, something went wrong.