Skip to content

Commit 1b04e63

Browse files
committed
fix(useObserver): do not initialize observer if element or observer is not defined
1 parent af25eaa commit 1b04e63

File tree

2 files changed

+31
-12
lines changed

2 files changed

+31
-12
lines changed

src/useObserver.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ export default function useObserver(Observer, { observerOptions, subscribeOption
1616
}, [])
1717

1818
useDeepCompareEffect(() => {
19+
if (!element || !Observer) {
20+
return undefined;
21+
}
22+
1923
if (!publishers) {
2024
publishers = new Map()
2125
}
@@ -34,10 +38,6 @@ export default function useObserver(Observer, { observerOptions, subscribeOption
3438
options.set(optionId, publisher)
3539
}
3640

37-
if (!element) {
38-
return undefined
39-
}
40-
4141
const { subscribe, unsubscribe } = publisher
4242
subscribe({ element, options: subscribeOptions }, newEntry => setEntry(newEntry))
4343

src/useObserver.test.js

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import React, { useEffect, useState } from 'react'
2-
import { renderHook } from 'react-hooks-testing-library'
32
import { render, act } from 'react-testing-library'
43

54
const mockObserve = jest.fn()
@@ -23,19 +22,39 @@ beforeEach(() => {
2322
})
2423
})
2524

25+
it('should not initialize observer if element ref is not provided', () => {
26+
// eslint-disable-next-line react/prop-types
27+
const ObservedComponent = ({ observer, options }) => {
28+
useObserver(observer, options);
29+
30+
return <div />
31+
};
32+
33+
render(<ObservedComponent observer={MockObserver} />);
34+
35+
expect(MockObserver).not.toBeCalled();
36+
});
37+
2638
it('should create only one observer instance for same Observer and Options for all uses and rerenders', () => {
2739
const loopArr = [...Array(10)]
2840

41+
// eslint-disable-next-line react/prop-types
42+
const ObservedComponent = ({ observer, options }) => {
43+
const [ref] = useObserver(observer, options);
44+
45+
return <div ref={ref} />
46+
};
47+
2948
loopArr.forEach(() => {
30-
renderHook(() => useObserver(MockObserver))
31-
renderHook(() => useObserver(MockObserver, { observerOptions: { test: 'test' } }))
32-
renderHook(() => useObserver(MockObserver, { observerOptions: { test: 'test2' } }))
33-
renderHook(() => useObserver(MockObserver, { observerOptions: { test: 'test' }, subscribeOptions: { test: 'test' } }))
49+
render(<ObservedComponent observer={MockObserver} />)
50+
render(<ObservedComponent observer={MockObserver} options={{ observerOptions: { test: 'test' } }} />)
51+
render(<ObservedComponent observer={MockObserver} options={{ observerOptions: { test: 'test2' } }} />)
52+
render(<ObservedComponent observer={MockObserver} options={{ observerOptions: { test: 'test' }, subscribeOptions: { test: 'test' } }} />)
3453
})
3554

36-
const { rerender } = renderHook(() => useObserver(MockObserver))
37-
const { rerender: rerender2 } = renderHook(() => useObserver(MockObserver, { observerOptions: { test: 'test' } }))
38-
const { rerender: rerender3 } = renderHook(() => useObserver(MockObserver, { observerOptions: { test: 'test2' } }))
55+
const { rerender } = render(<ObservedComponent observer={MockObserver} />);
56+
const { rerender: rerender2 } = render(<ObservedComponent observer={MockObserver} options={{ observerOptions: { test: 'test' } }} />)
57+
const { rerender: rerender3 } = render(<ObservedComponent observer={MockObserver} options={{ observerOptions: { test: 'test2' } }} />)
3958

4059
loopArr.forEach(() => {
4160
rerender()

0 commit comments

Comments
 (0)