Skip to content

Commit 97cdedc

Browse files
authored
Add support for multiple facets in useFacetCallback and useFacetEffect (#14)
This was been motivated by internal use-cases that relied on adding extra `useFacetMaps` to achieve the same goal. This should have a lower overhead with the added benefit that now all of our hooks share the same API (facets are arrays). Also updates the `useRemoteFacetPropSetter` to accept a `remoteFacet` instead of a facet, making the API more consistent. I did this update as I needed to touch the implementation.
1 parent 6df2b6c commit 97cdedc

18 files changed

+441
-88
lines changed

examples/benchmarking/src/listMemoFacet.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,31 +52,31 @@ const ListItem = ({ item }: { item: Facet<Data> }) => {
5252
randomWork(health)
5353
},
5454
[],
55-
health,
55+
[health],
5656
)
5757

5858
useFacetEffect(
5959
(name) => {
6060
randomWork(name)
6161
},
6262
[],
63-
name,
63+
[name],
6464
)
6565

6666
useFacetEffect(
6767
(name) => {
6868
randomWork(name)
6969
},
7070
[],
71-
name,
71+
[name],
7272
)
7373

7474
useFacetEffect(
7575
(name) => {
7676
randomWork(name)
7777
},
7878
[],
79-
name,
79+
[name],
8080
)
8181

8282
return null

examples/benchmarking/src/overheadFacet.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ function Performance() {
2525
// and effect that does nothing
2626
},
2727
[],
28-
value,
28+
[value],
2929
)
3030

3131
return null

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ it('updates only items that have changed', () => {
102102
const mock = jest.fn()
103103

104104
const ExampleContent = ({ item }: { item: Facet<Input> }) => {
105-
useFacetEffect(mock, [], item)
105+
useFacetEffect(mock, [], [item])
106106
return null
107107
}
108108

packages/@react-facet/core/src/hooks/useFacetCallback.spec.tsx

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React, { useEffect, useRef } from 'react'
2-
import { render } from '@react-facet/dom-fiber-testing-library'
2+
import { render, act } from '@react-facet/dom-fiber-testing-library'
33
import { useFacetCallback } from './useFacetCallback'
44
import { useFacetEffect } from './useFacetEffect'
55
import { useFacetMap } from './useFacetMap'
@@ -19,11 +19,11 @@ it('captures the current value of the facet in a function that can be used as ha
1919

2020
const ComponentWithFacetCallback = ({ cb, dependency }: ComponentWithFacetCallbackProps) => {
2121
const handler = useFacetCallback(
22-
(value) => (event) => {
22+
(value) => (event: string) => {
2323
cb(value, dependency, event)
2424
},
2525
[dependency, cb],
26-
demoFacet,
26+
[demoFacet],
2727
)
2828

2929
useEffect(() => {
@@ -60,7 +60,7 @@ it('properly memoizes the returned facet', () => {
6060

6161
const TestComponent = () => {
6262
const previousCallbackRef = useRef<() => void | NoValue>()
63-
const callback = useFacetCallback(() => () => {}, [], demoFacet)
63+
const callback = useFacetCallback(() => () => {}, [], [demoFacet])
6464

6565
// Check if it is a second render
6666
if (previousCallbackRef.current) {
@@ -98,15 +98,15 @@ it('should work with uninitialized values', () => {
9898
cb(value)
9999
},
100100
[cb],
101-
internalDemoFacet,
101+
[internalDemoFacet],
102102
)
103103

104104
useFacetEffect(
105105
() => {
106106
handler()
107107
},
108108
[handler],
109-
internalDemoFacet,
109+
[internalDemoFacet],
110110
)
111111

112112
return null
@@ -120,3 +120,61 @@ it('should work with uninitialized values', () => {
120120
expect(callback).toHaveBeenCalledTimes(1)
121121
expect(callback).toHaveBeenCalledWith('valuevalue')
122122
})
123+
124+
it('supports multiple facets', () => {
125+
const facetA = createFacet({ initialValue: 'a' })
126+
const facetB = createFacet({ initialValue: 123 })
127+
128+
const callback = jest.fn()
129+
let handler: (event: string) => void
130+
131+
const TestComponent = ({ dependency }: { dependency: string }) => {
132+
handler = useFacetCallback(
133+
(valueA, valueB) => (event: string) => {
134+
callback(valueA, valueB, dependency, event)
135+
},
136+
[dependency],
137+
[facetA, facetB],
138+
)
139+
140+
return null
141+
}
142+
143+
render(<TestComponent dependency="dependency" />)
144+
145+
act(() => {
146+
handler('event')
147+
})
148+
149+
expect(callback).toHaveBeenCalledWith('a', 123, 'dependency', 'event')
150+
})
151+
152+
it('returns NO_VALUE if any facet has NO_VALUE and skip calling the callback', () => {
153+
const facetA = createFacet({ initialValue: 'a' })
154+
const facetB = createFacet<number>({ initialValue: NO_VALUE })
155+
156+
const callback = jest.fn()
157+
let handler: (event: string) => void
158+
159+
const TestComponent = ({ dependency }: { dependency: string }) => {
160+
handler = useFacetCallback(
161+
(valueA, valueB) => (event: string) => {
162+
callback(valueA, valueB, dependency, event)
163+
},
164+
[dependency],
165+
[facetA, facetB],
166+
)
167+
168+
return null
169+
}
170+
171+
render(<TestComponent dependency="dependency" />)
172+
173+
act(() => {
174+
const result = handler('event')
175+
// verifies that calling the callback returns NO_VALUE
176+
expect(result).toBe(NO_VALUE)
177+
})
178+
179+
expect(callback).not.toHaveBeenCalledWith()
180+
})
Lines changed: 137 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,157 @@
1-
import { useCallback, useRef } from 'react'
1+
import { useCallback, useLayoutEffect, useRef } from 'react'
22
import { NoValue } from '..'
33
import { Facet, NO_VALUE, Option } from '../types'
4-
import { useFacetEffect } from './useFacetEffect'
4+
5+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
6+
export function useFacetCallback<M, V, C extends (...args: any[]) => M | NoValue>(
7+
callback: (v: V) => C,
8+
dependencies: unknown[],
9+
facet: [Facet<V>],
10+
): C
11+
12+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
13+
export function useFacetCallback<M, V, V1, C extends (...args: any[]) => M | NoValue>(
14+
callback: (v: V, v1: V1) => C,
15+
dependencies: unknown[],
16+
facet: [Facet<V>, Facet<V1>],
17+
): C
18+
19+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
20+
export function useFacetCallback<M, V, V1, V2, C extends (...args: any[]) => M | NoValue>(
21+
callback: (v: V, v1: V1, v2: V2) => C,
22+
dependencies: unknown[],
23+
facet: [Facet<V>, Facet<V1>, Facet<V2>],
24+
): C
25+
26+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
27+
export function useFacetCallback<M, V, V1, V2, V3, C extends (...args: any[]) => M | NoValue>(
28+
callback: (v: V, v1: V1, v2: V2, v3: V3) => C,
29+
dependencies: unknown[],
30+
facet: [Facet<V>, Facet<V1>, Facet<V2>, Facet<V3>],
31+
): C
32+
33+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
34+
export function useFacetCallback<M, V, V1, V2, V3, V4, C extends (...args: any[]) => M | NoValue>(
35+
callback: (v: V, v1: V1, v2: V2, v3: V3, v4: V4) => C,
36+
dependencies: unknown[],
37+
facet: [Facet<V>, Facet<V1>, Facet<V2>, Facet<V3>, Facet<V4>],
38+
): C
39+
40+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
41+
export function useFacetCallback<M, V, V1, V2, V3, V4, V5, C extends (...args: any[]) => M | NoValue>(
42+
callback: (v: V, v1: V1, v2: V2, v3: V3, v4: V4, v5: V5) => C,
43+
dependencies: unknown[],
44+
facet: [Facet<V>, Facet<V1>, Facet<V2>, Facet<V3>, Facet<V4>, Facet<V5>],
45+
): C
46+
47+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
48+
export function useFacetCallback<M, V, V1, V2, V3, V4, V5, V6, C extends (...args: any[]) => M | NoValue>(
49+
callback: (v: V, v1: V1, v2: V2, v3: V3, v4: V4, v5: V5, v6: V6) => C,
50+
dependencies: unknown[],
51+
facet: [Facet<V>, Facet<V1>, Facet<V2>, Facet<V3>, Facet<V4>, Facet<V5>, Facet<V6>],
52+
): C
53+
54+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
55+
export function useFacetCallback<M, V, V1, V2, V3, V4, V5, V6, V7, C extends (...args: any[]) => M | NoValue>(
56+
callback: (v: V, v1: V1, v2: V2, v3: V3, v4: V4, v5: V5, v6: V6, v7: V7) => C,
57+
dependencies: unknown[],
58+
facet: [Facet<V>, Facet<V1>, Facet<V2>, Facet<V3>, Facet<V4>, Facet<V5>, Facet<V6>, Facet<V7>],
59+
): C
60+
61+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
62+
export function useFacetCallback<M, V, V1, V2, V3, V4, V5, V6, V7, V8, C extends (...args: any[]) => M | NoValue>(
63+
callback: (v: V, v1: V1, v2: V2, v3: V3, v4: V4, v5: V5, v6: V6, v7: V7, v8: V8) => C,
64+
dependencies: unknown[],
65+
facet: [Facet<V>, Facet<V1>, Facet<V2>, Facet<V3>, Facet<V4>, Facet<V5>, Facet<V6>, Facet<V7>, Facet<V8>],
66+
): C
67+
68+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
69+
export function useFacetCallback<M, V, V1, V2, V3, V4, V5, V6, V7, V8, V9, C extends (...args: any[]) => M | NoValue>(
70+
callback: (v: V, v1: V1, v2: V2, v3: V3, v4: V4, v5: V5, v6: V6, v7: V7, v8: V8, v9: V9) => C,
71+
dependencies: unknown[],
72+
facet: [Facet<V>, Facet<V1>, Facet<V2>, Facet<V3>, Facet<V4>, Facet<V5>, Facet<V6>, Facet<V7>, Facet<V8>, Facet<V9>],
73+
): C
74+
75+
export function useFacetCallback<
76+
M,
77+
V,
78+
V1,
79+
V2,
80+
V3,
81+
V4,
82+
V5,
83+
V6,
84+
V7,
85+
V8,
86+
V9,
87+
V10,
88+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
89+
C extends (...args: any[]) => M | NoValue,
90+
>(
91+
callback: (v: V, v1: V1, v2: V2, v3: V3, v4: V4, v5: V5, v6: V6, v7: V7, v8: V8, v9: V9, v10: V10) => C,
92+
dependencies: unknown[],
93+
facet: [
94+
Facet<V>,
95+
Facet<V1>,
96+
Facet<V2>,
97+
Facet<V3>,
98+
Facet<V4>,
99+
Facet<V5>,
100+
Facet<V6>,
101+
Facet<V7>,
102+
Facet<V8>,
103+
Facet<V9>,
104+
Facet<V10>,
105+
],
106+
): C
5107

6108
/**
7109
* Creates a callback that depends on the value of a facet.
8110
* Very similar to `useCallback` from `React`
9111
*
10-
* @param callback function callback that receives the current facet value and the arguments passed to the callback
11-
* @param dependencies variable used by the map that are available in scope (similar as dependencies of useCallback)
12-
* @param facet facet that the callback listens to
112+
* @param callback function callback that receives the current facet values and the arguments passed to the callback
113+
* @param dependencies variable used by the callback that are available in scope (similar as dependencies of useCallback)
114+
* @param facets facets that the callback listens to
13115
*
14116
* We pass the dependencies of the callback as the second argument so we can leverage the eslint-plugin-react-hooks option for additionalHooks.
15117
* Having this as the second argument allows the linter to work.
16118
*/
17-
export function useFacetCallback<V, T>(
18-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
19-
callback: (value: V) => (...args: any[]) => T | NoValue,
20-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
21-
dependencies: any[],
22-
facet: Facet<V>,
23-
) {
24-
const facetRef = useRef<Option<V>>(facet.get())
25-
26-
useFacetEffect(
27-
(value) => {
28-
facetRef.current = value
29-
},
30-
[],
31-
facet,
32-
)
119+
export function useFacetCallback<M>(
120+
callback: (...args: unknown[]) => (...args: unknown[]) => M | NoValue,
121+
dependencies: unknown[],
122+
facets: Facet<unknown>[],
123+
): (...args: unknown[]) => M | NoValue {
124+
const facetsRef = useRef<Option<unknown>[]>(facets.map(() => NO_VALUE))
125+
126+
useLayoutEffect(() => {
127+
const unsubscribes = facets.map((facet, index) => {
128+
return facet.observe((value) => {
129+
facetsRef.current[index] = value
130+
})
131+
})
132+
133+
return () => {
134+
unsubscribes.forEach((unsubscribe) => unsubscribe())
135+
}
136+
// We care about each individual facet and if any is a different reference
137+
// eslint-disable-next-line react-hooks/exhaustive-deps
138+
}, facets)
139+
140+
// We care about each individual dependency and if any is a different reference
33141
// eslint-disable-next-line react-hooks/exhaustive-deps
34142
const callbackMemoized = useCallback(callback, dependencies)
35143

144+
// eslint-disable-next-line react-hooks/exhaustive-deps
36145
return useCallback(
37-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
38-
(...args: any[]): T | NoValue => {
39-
const value = facetRef.current
146+
(...args: unknown[]) => {
147+
const values = facetsRef.current
40148

41-
if (value === NO_VALUE) return NO_VALUE
149+
for (const value of values) {
150+
if (value === NO_VALUE) return NO_VALUE
151+
}
42152

43-
return callbackMemoized(value)(...args)
153+
return callbackMemoized(...values)(...args)
44154
},
45-
[callbackMemoized, facetRef],
155+
[callbackMemoized, facetsRef],
46156
)
47157
}

0 commit comments

Comments
 (0)