From ec2ea3b0ad8575c9cff86831cab805fb7ae9347e Mon Sep 17 00:00:00 2001 From: Adam Ramberg Date: Mon, 24 Oct 2022 08:48:37 +0200 Subject: [PATCH] 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 5b76defce67442614efea321eca9f79d4708e616. * Update docs for contributing * Use effectMemoized and make the function returned from createUseFacetEffect unnamed. * Minor adjustment to contributions docs --- CONTRIBUTING.md | 17 +- jest.base.config.js | 1 + .../core/src/facet/createFacet.spec.ts | 6 +- .../core/src/facet/createFacet.ts | 46 +---- .../core/src/hooks/useFacetEffect.spec.tsx | 50 ++--- .../core/src/hooks/useFacetEffect.tsx | 108 +++++----- .../src/hooks/useFacetLayoutEffect.spec.tsx | 185 ------------------ .../core/src/hooks/useFacetLayoutEffect.ts | 49 +---- .../src/{index.spec.ts => index.spec.tsx} | 65 ++++++ .../core/src/mapFacets/mapIntoObserveArray.ts | 6 +- .../src/mapFacets/mapIntoObserveSingle.ts | 6 +- packages/@react-facet/core/src/types.ts | 2 +- 12 files changed, 182 insertions(+), 359 deletions(-) delete mode 100644 packages/@react-facet/core/src/hooks/useFacetLayoutEffect.spec.tsx rename packages/@react-facet/core/src/{index.spec.ts => index.spec.tsx} (52%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0fee24cb..0a70bf6e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,7 +1,8 @@ -# Bumping versions +# Creating a new release Currently the process of updating the version of the packages is mostly manual. Before you start, first you need to make sure you have the permissions to publish the packages. +- If there is a release branch (see __Release candidate__ below) that hasn't been merged to `main` yet now is the time to do so. - Make sure that you are logged in into your `npm` account. Use the command `yarn npm login` on the project folder to do this. - While on the `main` branch. - Perform a search an replace on all "package.json" files from the old version to the new. (ex `0.1.4` to `0.2.0`). @@ -12,4 +13,16 @@ Currently the process of updating the version of the packages is mostly manual. - Publish the packages by running `yarn publish` (it will also build the packages). - Then head to GitHub to draft a new release https://github.com/Mojang/ore-ui/releases/new - Choose the tag and click the button "Auto-generate release notes" -- Click "publish release" +- Click "publish release" 🚀 + +# Release candidate + +- Make sure that you are logged in into your `npm` account. Use the command `yarn npm login` on the project folder to do this. +- Create a branch including the changes for the release candidate and name the branch to the same thing as the upcoming release, eg: `v0.4`. +- Perform a search an replace on all "package.json" files from the old version to the new. (ex `0.3.12` to `0.4.0-rc.0`). +- Run `yarn` to update the lockfile. +- Commit the changes with a message containing the new version (ex: `0.4.0-rc.0`). +- Create an annotated git tag by running `git tag -a v0.4.0-rc.0` (replace with the version). The tag message can be the version again. +- Push commit and the tag `git push --follow-tags`. +- Publish the packages by running `yarn publish --tag rc` (it will also build the packages). +- We are currently not doing release notes for release candidates so you are all done! 🎉 \ No newline at end of file diff --git a/jest.base.config.js b/jest.base.config.js index 849170c6..1a4ae0c7 100644 --- a/jest.base.config.js +++ b/jest.base.config.js @@ -11,4 +11,5 @@ module.exports = { '^.+\\.jsx?$': 'babel-jest', '^.+\\.tsx?$': 'babel-jest', }, + testPathIgnorePatterns: ['/node_modules/', '/dist/'], } diff --git a/packages/@react-facet/core/src/facet/createFacet.spec.ts b/packages/@react-facet/core/src/facet/createFacet.spec.ts index 57ef809b..441da769 100644 --- a/packages/@react-facet/core/src/facet/createFacet.spec.ts +++ b/packages/@react-facet/core/src/facet/createFacet.spec.ts @@ -219,23 +219,19 @@ describe('cleanup', () => { describe('setWithCallback', () => { it('prevents calling listeners if a setter returns NO_VALUE', () => { const facet = createFacet({ initialValue: 10 }) - const cleanupMock = jest.fn() - const listenerMock = jest.fn().mockReturnValue(cleanupMock) + const listenerMock = jest.fn() facet.observe(listenerMock) // after observing it, the listener is called once with the initial value (but not the cleanup) expect(listenerMock).toHaveBeenCalledTimes(1) expect(listenerMock).toHaveBeenCalledWith(10) - expect(cleanupMock).not.toHaveBeenCalled() listenerMock.mockClear() listenerMock.mockClear() - cleanupMock.mockClear() facet.setWithCallback(() => NO_VALUE) // after using a setter callback to return NO_VALUE, the previous cleanup should be called, but not the listener again expect(listenerMock).not.toHaveBeenCalled() - expect(cleanupMock).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/@react-facet/core/src/facet/createFacet.ts b/packages/@react-facet/core/src/facet/createFacet.ts index c3355afb..8792f851 100644 --- a/packages/@react-facet/core/src/facet/createFacet.ts +++ b/packages/@react-facet/core/src/facet/createFacet.ts @@ -1,12 +1,6 @@ import { defaultEqualityCheck } from '../equalityChecks' import { Cleanup, EqualityCheck, Listener, WritableFacet, StartSubscription, Option, NO_VALUE } from '../types' -interface ListenerCleanupEntry { - cleanup: Cleanup - // eslint-disable-next-line @typescript-eslint/no-explicit-any - listener: Listener -} - export interface FacetOptions { initialValue: Option startSubscription?: StartSubscription @@ -24,7 +18,6 @@ export function createFacet({ let currentValue = initialValue let cleanupSubscription: Cleanup | undefined - let listenerCleanups: ListenerCleanupEntry[] = [] const checker = equalityCheck?.() const update = (newValue: V) => { @@ -51,22 +44,8 @@ export function createFacet({ currentValue = newValue - if (listenerCleanups.length !== 0) { - for (let index = 0; index < listenerCleanups.length; index++) { - listenerCleanups[index].cleanup() - } - - // start with a new array - listenerCleanups = [] - } - for (const listener of listeners) { - const cleanup = listener(currentValue) - - // if the listener returns a cleanup function, we store it to call latter - if (cleanup != null) { - listenerCleanups.push({ cleanup, listener }) - } + listener(currentValue) } } @@ -76,15 +55,6 @@ export function createFacet({ */ const updateToNoValue = () => { currentValue = NO_VALUE - - if (listenerCleanups.length !== 0) { - for (let index = 0; index < listenerCleanups.length; index++) { - listenerCleanups[index].cleanup() - } - - // start with a new array - listenerCleanups = [] - } } return { @@ -105,12 +75,7 @@ export function createFacet({ listeners.add(listener) if (currentValue !== NO_VALUE) { - const cleanup = listener(currentValue) - - // if the listener returns a cleanup function, we store it to call latter - if (cleanup != null) { - listenerCleanups.push({ cleanup, listener }) - } + listener(currentValue) } // This is the first subscription, so we start subscribing to dependencies @@ -119,13 +84,6 @@ export function createFacet({ } return () => { - // check if this listener has any cleanup that we need to call - const cleanupIndex = listenerCleanups.findIndex((entry) => entry.listener === listener) - if (cleanupIndex !== -1) { - listenerCleanups[cleanupIndex].cleanup() - listenerCleanups.splice(cleanupIndex, 1) - } - listeners.delete(listener) // if this was the last to unsubscribe, we unsubscribe from our dependencies diff --git a/packages/@react-facet/core/src/hooks/useFacetEffect.spec.tsx b/packages/@react-facet/core/src/hooks/useFacetEffect.spec.tsx index 436e6abc..625ee08b 100644 --- a/packages/@react-facet/core/src/hooks/useFacetEffect.spec.tsx +++ b/packages/@react-facet/core/src/hooks/useFacetEffect.spec.tsx @@ -10,13 +10,7 @@ it('triggers the effect on mount with the initial value and on any update of the const callback = jest.fn() const ComponentWithFacetEffect = () => { - useFacetEffect( - (value) => { - callback(value) - }, - [], - [demoFacet], - ) + useFacetEffect(callback, [], [demoFacet]) return null } @@ -75,15 +69,12 @@ describe('cleanup', () => { const demoFacet = createFacet({ initialValue: 'initial value' }) const cleanup = jest.fn() + const effect = jest.fn((...args) => { + return () => cleanup(...args) + }) const ComponentWithFacetEffect = () => { - useFacetEffect( - () => { - return cleanup - }, - [], - [demoFacet], - ) + useFacetEffect(effect, [], [demoFacet]) return null } @@ -95,21 +86,36 @@ describe('cleanup', () => { // cleanup is not called immediately expect(cleanup).not.toHaveBeenCalled() + // ...but the effect is + expect(effect).toHaveBeenCalledTimes(1) + expect(effect).toHaveBeenCalledWith('initial value') + + effect.mockClear() + act(() => { demoFacet.set('new value') }) // once a new value is triggered we expect that the previous cleanup was called - expect(cleanup).toHaveBeenCalled() + expect(cleanup).toHaveBeenCalledTimes(1) + expect(cleanup).toHaveBeenCalledWith('initial value') + + // ...and the effect is called again with the new value + expect(effect).toHaveBeenCalledTimes(1) + expect(effect).toHaveBeenLastCalledWith('new value') - // clear any recorded calls ahead of next check cleanup.mockClear() + effect.mockClear() // unmount the component to check if the cleanup is also called rerender(<>) - // once a new value is triggered we expect that the previous cleanup was called - expect(cleanup).toHaveBeenCalled() + // when the component unmounts we expect that we cleanup the last called effect + expect(cleanup).toHaveBeenCalledTimes(1) + expect(cleanup).toHaveBeenCalledWith('new value') + + // the effect shouldn't be called again on unmount + expect(effect).not.toHaveBeenCalled() }) }) @@ -121,13 +127,7 @@ it('supports multiple facets, only triggering the effect once all facets have a const effect = jest.fn().mockReturnValue(cleanup) const ComponentWithFacetEffect = () => { - useFacetEffect( - (valueA, valueB) => { - return effect(valueA, valueB) - }, - [], - [facetA, facetB], - ) + useFacetEffect(effect, [], [facetA, facetB]) return null } diff --git a/packages/@react-facet/core/src/hooks/useFacetEffect.tsx b/packages/@react-facet/core/src/hooks/useFacetEffect.tsx index 4fc389fb..0decd08a 100644 --- a/packages/@react-facet/core/src/hooks/useFacetEffect.tsx +++ b/packages/@react-facet/core/src/hooks/useFacetEffect.tsx @@ -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[], T extends [...Y]>( - effect: (...args: ExtractFacetValues) => void | Cleanup, - dependencies: unknown[], - facets: T, -) { - // eslint-disable-next-line react-hooks/exhaustive-deps - const effectMemoized = useCallback(effect as (...args: unknown[]) => ReturnType, 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 [], T extends [...Y]>( + effect: (...args: ExtractFacetValues) => void | Cleanup, + dependencies: unknown[], + facets: T, + ) { + // eslint-disable-next-line react-hooks/exhaustive-deps + const effectMemoized = useCallback(effect as (...args: unknown[]) => ReturnType, 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)) + 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)) + } + }) }) - }) - 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) diff --git a/packages/@react-facet/core/src/hooks/useFacetLayoutEffect.spec.tsx b/packages/@react-facet/core/src/hooks/useFacetLayoutEffect.spec.tsx deleted file mode 100644 index 96e4fa7a..00000000 --- a/packages/@react-facet/core/src/hooks/useFacetLayoutEffect.spec.tsx +++ /dev/null @@ -1,185 +0,0 @@ -import React from 'react' -import { act, render } from '@react-facet/dom-fiber-testing-library' -import { useFacetLayoutEffect } from './useFacetLayoutEffect' -import { createFacet } from '../facet' -import { NO_VALUE } from '../types' - -it('triggers the effect on mount with the initial value and on any update of the facet', () => { - const demoFacet = createFacet({ initialValue: 'initial value' }) - - const callback = jest.fn() - - const ComponentWithFacetEffect = () => { - useFacetLayoutEffect( - (value) => { - callback(value) - }, - [], - [demoFacet], - ) - - return null - } - - const scenario = - - render(scenario) - - expect(callback).toHaveBeenCalledWith('initial value') - - // prepare mock for next check - callback.mockClear() - - // change the facet - act(() => { - demoFacet.set('new value') - }) - - // verify that it was called again, but with the new value - expect(callback).toHaveBeenCalledWith('new value') -}) - -it('triggers the effect when a dependency changes', () => { - const demoFacet = createFacet({ initialValue: 'initial value' }) - - const callback = jest.fn() - - const ComponentWithFacetEffect = ({ dependency }: { dependency: number }) => { - useFacetLayoutEffect( - (value) => { - callback(`${value} ${dependency}`) - }, - [dependency], - [demoFacet], - ) - - return null - } - - const { rerender } = render() - - expect(callback).toHaveBeenCalledWith('initial value 0') - - // clear the mock, since it was called on the mount - callback.mockClear() - - // change the dependency - rerender() - - // verify that the effect was called - expect(callback).toHaveBeenCalledWith('initial value 1') -}) - -describe('cleanup', () => { - it('handles cleanup the effect on changing the value and unmounting', () => { - const demoFacet = createFacet({ initialValue: 'initial value' }) - - const cleanup = jest.fn() - - const ComponentWithFacetEffect = () => { - useFacetLayoutEffect( - () => { - return cleanup - }, - [], - [demoFacet], - ) - - return null - } - - const scenario = - - const { rerender } = render(scenario) - - // cleanup is not called immediately - expect(cleanup).not.toHaveBeenCalled() - - act(() => { - demoFacet.set('new value') - }) - - // once a new value is triggered we expect that the previous cleanup was called - expect(cleanup).toHaveBeenCalled() - - // clear any recorded calls ahead of next check - cleanup.mockClear() - - // unmount the component to check if the cleanup is also called - rerender(<>) - - // once a new value is triggered we expect that the previous cleanup was called - expect(cleanup).toHaveBeenCalled() - }) -}) - -it('supports multiple facets, only triggering the effect once all facets have a value', () => { - const facetA = createFacet({ initialValue: 'initial value' }) - const facetB = createFacet({ initialValue: NO_VALUE }) - - const cleanup = jest.fn() - const effect = jest.fn().mockReturnValue(cleanup) - - const ComponentWithFacetEffect = () => { - useFacetLayoutEffect( - (valueA, valueB) => { - return effect(valueA, valueB) - }, - [], - [facetA, facetB], - ) - - return null - } - - const scenario = - - const { rerender } = render(scenario) - - expect(cleanup).not.toHaveBeenCalled() - expect(effect).not.toHaveBeenCalled() - effect.mockClear() - cleanup.mockClear() - - // change only the initialized facet - act(() => { - facetA.set('new value') - }) - - // expect that the effect was not called - expect(cleanup).not.toHaveBeenCalled() - expect(effect).not.toHaveBeenCalled() - effect.mockClear() - cleanup.mockClear() - - // set a value for the second facet - act(() => { - facetB.set(123) - }) - - // verify that the effect was finally called with both values - expect(cleanup).not.toHaveBeenCalled() - expect(effect).toHaveBeenCalledWith('new value', 123) - expect(effect).toHaveBeenCalledTimes(1) - effect.mockClear() - cleanup.mockClear() - - // update the first facet again - act(() => { - facetA.set('one more update') - }) - - // verify that the effect was called again, and now we verify that the previous cleanup was called - expect(cleanup).toHaveBeenCalledTimes(1) - expect(effect).toHaveBeenCalledWith('one more update', 123) - expect(effect).toHaveBeenCalledTimes(1) - effect.mockClear() - cleanup.mockClear() - - // and finally we unmount the component - rerender(<>) - - // then we get a final cleanup, without the effect being fired - expect(cleanup).toHaveBeenCalledTimes(1) - expect(effect).not.toHaveBeenCalled() -}) diff --git a/packages/@react-facet/core/src/hooks/useFacetLayoutEffect.ts b/packages/@react-facet/core/src/hooks/useFacetLayoutEffect.ts index 19e19943..dbb661bd 100644 --- a/packages/@react-facet/core/src/hooks/useFacetLayoutEffect.ts +++ b/packages/@react-facet/core/src/hooks/useFacetLayoutEffect.ts @@ -1,5 +1,5 @@ -import { useCallback, useLayoutEffect } from 'react' -import { Facet, Unsubscribe, Cleanup, NO_VALUE, ExtractFacetValues } from '../types' +import { useLayoutEffect } from 'react' +import { createUseFacetEffect } from './useFacetEffect' /** * Allows running an effect based on facet updates. Similar to React's useLayoutEffect. @@ -11,47 +11,4 @@ import { Facet, Unsubscribe, Cleanup, NO_VALUE, ExtractFacetValues } from '../ty * 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 useFacetLayoutEffect[], T extends [...Y]>( - effect: (...args: ExtractFacetValues) => void | Cleanup, - dependencies: unknown[], - facets: T, -) { - // eslint-disable-next-line react-hooks/exhaustive-deps - const effectMemoized = useCallback(effect as (...args: unknown[]) => ReturnType, dependencies) - - useLayoutEffect(() => { - 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) { - if (cleanup != null) { - cleanup() - } - - cleanup = effect(...(values as ExtractFacetValues)) - } - }) - }) - - 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]) -} +export const useFacetLayoutEffect = createUseFacetEffect(useLayoutEffect) diff --git a/packages/@react-facet/core/src/index.spec.ts b/packages/@react-facet/core/src/index.spec.tsx similarity index 52% rename from packages/@react-facet/core/src/index.spec.ts rename to packages/@react-facet/core/src/index.spec.tsx index 7fd51ef1..aaf882fc 100644 --- a/packages/@react-facet/core/src/index.spec.ts +++ b/packages/@react-facet/core/src/index.spec.tsx @@ -1,5 +1,70 @@ // Import from the index to check for sure that APIs are correctly exposed +import React from 'react' import * as facet from '.' +import { createFacet, useFacetEffect, useFacetMap } from '.' +import { act, render } from '@react-facet/dom-fiber-testing-library' + +describe('integration testing', () => { + it('calls effects and corresponding clean up handlers properly when depending on a useFacetMap facet', () => { + const demoFacet = createFacet({ initialValue: { valueA: 'initialA', valueB: 'initialB' } }) + + const cleanup = jest.fn() + const effect = jest.fn((...args) => { + return () => cleanup(...args) + }) + + const Scenario = () => { + const valueAFacet = useFacetMap(({ valueA }) => valueA, [], [demoFacet]) + useFacetEffect(effect, [], [valueAFacet]) + + return null + } + + const { rerender } = render() + + // cleanup is not called immediately + expect(cleanup).not.toHaveBeenCalled() + + // ...but the effect is + expect(effect).toHaveBeenCalledTimes(1) + expect(effect).toHaveBeenCalledWith('initialA') + + effect.mockClear() + + act(() => { + demoFacet.set({ valueA: 'initialA', valueB: 'newB' }) + }) + + // nothing should be called + expect(cleanup).not.toHaveBeenCalled() + expect(effect).not.toHaveBeenCalled() + + act(() => { + demoFacet.set({ valueA: 'newA', valueB: 'newB' }) + }) + + // once a new value is triggered we expect that the previous cleanup was called + expect(cleanup).toHaveBeenCalledTimes(1) + expect(cleanup).toHaveBeenCalledWith('initialA') + + // ...and the effect is called again with the new value + expect(effect).toHaveBeenCalledTimes(1) + expect(effect).toHaveBeenLastCalledWith('newA') + + cleanup.mockClear() + effect.mockClear() + + // unmount the component to check if the cleanup is also called + rerender(<>) + + // when the component unmounts we expect that we cleanup the last called effect + expect(cleanup).toHaveBeenCalledTimes(1) + expect(cleanup).toHaveBeenCalledWith('newA') + + // the effect shouldn't be called again on unmount + expect(effect).not.toHaveBeenCalled() + }) +}) describe('regression testing preventing accidental removal of APIs', () => { it('exposes the components', () => { diff --git a/packages/@react-facet/core/src/mapFacets/mapIntoObserveArray.ts b/packages/@react-facet/core/src/mapFacets/mapIntoObserveArray.ts index 76eeeb10..1bbe3cc5 100644 --- a/packages/@react-facet/core/src/mapFacets/mapIntoObserveArray.ts +++ b/packages/@react-facet/core/src/mapFacets/mapIntoObserveArray.ts @@ -26,7 +26,7 @@ export function mapIntoObserveArray( const result = fn(...dependencyValues) if (result === NO_VALUE) return - return listener(result) + listener(result) } }) } @@ -57,7 +57,7 @@ export function mapIntoObserveArray( currentValue = result - return listener(result) + listener(result) } }) } @@ -76,7 +76,7 @@ export function mapIntoObserveArray( if (result === NO_VALUE) return if (checker(result)) return - return listener(result) + listener(result) } }) }) diff --git a/packages/@react-facet/core/src/mapFacets/mapIntoObserveSingle.ts b/packages/@react-facet/core/src/mapFacets/mapIntoObserveSingle.ts index ed395e26..28db13cb 100644 --- a/packages/@react-facet/core/src/mapFacets/mapIntoObserveSingle.ts +++ b/packages/@react-facet/core/src/mapFacets/mapIntoObserveSingle.ts @@ -13,7 +13,7 @@ export function mapIntoObserveSingle( const result = fn(value) if (result === NO_VALUE) return - return listener(result) + listener(result) }) } } @@ -42,7 +42,7 @@ export function mapIntoObserveSingle( currentValue = result - return listener(result) + listener(result) }) } } @@ -56,7 +56,7 @@ export function mapIntoObserveSingle( if (result === NO_VALUE) return if (checker(result)) return - return listener(result) + listener(result) }) } } diff --git a/packages/@react-facet/core/src/types.ts b/packages/@react-facet/core/src/types.ts index e13880fa..edd046ff 100644 --- a/packages/@react-facet/core/src/types.ts +++ b/packages/@react-facet/core/src/types.ts @@ -28,7 +28,7 @@ export interface FacetFactory { export type ExcludeFacetFactory = T extends FacetFactory ? never : T export interface Listener { - (value: T): void | Cleanup + (value: T): void } export type Unsubscribe = () => void