Skip to content

Commit 8cf612e

Browse files
TkDodoTkDodo
committed
Merge remote-tracking branch 'react-query/master' into alpha
# Conflicts: # src/core/tests/focusManager.test.tsx # src/core/tests/onlineManager.test.tsx
2 parents c50b630 + 128e578 commit 8cf612e

File tree

8 files changed

+332
-111
lines changed

8 files changed

+332
-111
lines changed

‎.github/ISSUE_TEMPLATE/bug_report.md‎

Lines changed: 0 additions & 38 deletions
This file was deleted.

‎.github/ISSUE_TEMPLATE/bug_report.yml‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
name: '🐛 Bug report'
2+
description: Report a reproducible bug or regression
3+
body:
4+
- type: markdown
5+
attributes:
6+
value: |
7+
Thank you for reporting an issue :pray:.
8+
9+
This issue tracker is for reporting reproducible bugs or regression's found in react-query (https://github.com/tannerlinsley/react-query)
10+
If you have a question about how to achieve something and are struggling, please post a question
11+
inside of react-query's Discussion's tab: https://github.com/tannerlinsley/react-query/discussions
12+
13+
Before submitting a new bug/issue, please check the links below to see if there is a solution or question posted there already:
14+
- react-query's Discussion's tab: https://github.com/tannerlinsley/react-query/discussions
15+
- react-query's Issue's tab: https://github.com/tannerlinsley/react-query/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc
16+
- react-query's Closed issues tab: https://github.com/tannerlinsley/react-query/issues?q=is%3Aissue+sort%3Aupdated-desc+is%3Aclosed
17+
18+
The more information you fill in, the better the community can help you.
19+
- type: textarea
20+
id: description
21+
attributes:
22+
label: Describe the bug
23+
description: Provide a clear and concise description of the challenge you are running into.
24+
validations:
25+
required: true
26+
- type: input
27+
id: link
28+
attributes:
29+
label: Your Example Website or App
30+
description: |
31+
Which website or app were you using when the bug happened?
32+
Note:
33+
- Your bug will may get fixed much faster if we can run your code and it doesn't have dependencies other than React.
34+
- To create a shareable code example you can use Stackblitz (https://stackblitz.com/) or CodeSandbox (https://codesandbox.io/s/new). Please no localhost URLs.
35+
- Please read these tips for providing a minimal example: https://stackoverflow.com/help/mcve.
36+
placeholder: |
37+
e.g. Stackblitz, Code Sandbox app url
38+
validations:
39+
required: true
40+
- type: textarea
41+
id: steps
42+
attributes:
43+
label: Steps to reproduce
44+
description: Describe the steps we have to take to reproduce the behavior.
45+
placeholder: |
46+
1. Go to '...'
47+
2. Click on '....'
48+
3. Scroll down to '....'
49+
4. See error
50+
validations:
51+
required: true
52+
- type: textarea
53+
id: expected
54+
attributes:
55+
label: Expected behavior
56+
description: Provide a clear and concise description of what you expected to happen.
57+
placeholder: |
58+
As a user, I expected ___ behavior but i am seeing ___
59+
validations:
60+
required: true
61+
- type: dropdown
62+
attributes:
63+
label: How often does this bug happen?
64+
description: |
65+
Following the repro steps above, how easily are you able to reproduce this bug?
66+
options:
67+
- Every time
68+
- Often
69+
- Sometimes
70+
- Only once
71+
- type: textarea
72+
id: screenshots_or_videos
73+
attributes:
74+
label: Screenshots or Videos
75+
description: |
76+
If applicable, add screenshots or a video to help explain your problem.
77+
For more information on the supported file image/file types and the file size limits, please refer
78+
to the following link: https://docs.github.com/en/github/writing-on-github/working-with-advanced-formatting/attaching-files
79+
placeholder: |
80+
You can drag your video or image files inside of this editor ↓
81+
- type: textarea
82+
id: platform
83+
attributes:
84+
label: Platform
85+
value: |
86+
- OS: [e.g. macOS, Windows, Linux, iOS, Android]
87+
- Browser: [e.g. Chrome, Safari, Firefox, React Native]
88+
- Version: [e.g. 91.1]
89+
validations:
90+
required: true
91+
- type: textarea
92+
id: additional
93+
attributes:
94+
label: Additional context
95+
description: Add any other context about the problem here.

‎src/core/focusManager.ts‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,51 @@
11
import { Subscribable } from './subscribable'
22
import { isServer } from './utils'
33

4-
class FocusManager extends Subscribable {
4+
type SetupFn = (
5+
setFocused: (focused?: boolean) => void
6+
) => (() => void) | undefined
7+
8+
export class FocusManager extends Subscribable {
59
private focused?: boolean
6-
private removeEventListener?: () => void
10+
private cleanup?: () => void
11+
12+
private setup: SetupFn
13+
14+
constructor() {
15+
super()
16+
this.setup = onFocus => {
17+
if (!isServer && window?.addEventListener) {
18+
const listener = () => onFocus()
19+
// Listen to visibillitychange and focus
20+
window.addEventListener('visibilitychange', listener, false)
21+
window.addEventListener('focus', listener, false)
22+
23+
return () => {
24+
// Be sure to unsubscribe if a new handler is set
25+
window.removeEventListener('visibilitychange', listener)
26+
window.removeEventListener('focus', listener)
27+
}
28+
}
29+
}
30+
}
731

832
protected onSubscribe(): void {
9-
if (!this.removeEventListener) {
10-
this.setDefaultEventListener()
33+
if (!this.cleanup) {
34+
this.setEventListener(this.setup)
1135
}
1236
}
1337

14-
setEventListener(
15-
setup: (setFocused: (focused?: boolean) => void) => () => void
16-
): void {
17-
if (this.removeEventListener) {
18-
this.removeEventListener()
38+
protected onUnsubscribe() {
39+
if (!this.hasListeners()) {
40+
this.cleanup?.()
41+
this.cleanup = undefined
1942
}
20-
this.removeEventListener = setup(focused => {
43+
}
44+
45+
setEventListener(setup: SetupFn): void {
46+
this.setup = setup
47+
this.cleanup?.()
48+
this.cleanup = setup(focused => {
2149
if (typeof focused === 'boolean') {
2250
this.setFocused(focused)
2351
} else {
@@ -54,23 +82,6 @@ class FocusManager extends Subscribable {
5482
document.visibilityState
5583
)
5684
}
57-
58-
private setDefaultEventListener() {
59-
if (!isServer && window?.addEventListener) {
60-
this.setEventListener(onFocus => {
61-
const listener = () => onFocus()
62-
// Listen to visibillitychange and focus
63-
window.addEventListener('visibilitychange', listener, false)
64-
window.addEventListener('focus', listener, false)
65-
66-
return () => {
67-
// Be sure to unsubscribe if a new handler is set
68-
window.removeEventListener('visibilitychange', listener)
69-
window.removeEventListener('focus', listener)
70-
}
71-
})
72-
}
73-
}
7485
}
7586

7687
export const focusManager = new FocusManager()

‎src/core/onlineManager.ts‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,51 @@
11
import { Subscribable } from './subscribable'
22
import { isServer } from './utils'
33

4-
class OnlineManager extends Subscribable {
4+
type SetupFn = (
5+
setOnline: (online?: boolean) => void
6+
) => (() => void) | undefined
7+
8+
export class OnlineManager extends Subscribable {
59
private online?: boolean
6-
private removeEventListener?: () => void
10+
private cleanup?: () => void
11+
12+
private setup: SetupFn
13+
14+
constructor() {
15+
super()
16+
this.setup = onOnline => {
17+
if (!isServer && window?.addEventListener) {
18+
const listener = () => onOnline()
19+
// Listen to online
20+
window.addEventListener('online', listener, false)
21+
window.addEventListener('offline', listener, false)
22+
23+
return () => {
24+
// Be sure to unsubscribe if a new handler is set
25+
window.removeEventListener('online', listener)
26+
window.removeEventListener('offline', listener)
27+
}
28+
}
29+
}
30+
}
731

832
protected onSubscribe(): void {
9-
if (!this.removeEventListener) {
10-
this.setDefaultEventListener()
33+
if (!this.cleanup) {
34+
this.setEventListener(this.setup)
1135
}
1236
}
1337

14-
setEventListener(
15-
setup: (setOnline: (online?: boolean) => void) => () => void
16-
): void {
17-
if (this.removeEventListener) {
18-
this.removeEventListener()
38+
protected onUnsubscribe() {
39+
if (!this.hasListeners()) {
40+
this.cleanup?.()
41+
this.cleanup = undefined
1942
}
20-
this.removeEventListener = setup((online?: boolean) => {
43+
}
44+
45+
setEventListener(setup: SetupFn): void {
46+
this.setup = setup
47+
this.cleanup?.()
48+
this.cleanup = setup((online?: boolean) => {
2149
if (typeof online === 'boolean') {
2250
this.setOnline(online)
2351
} else {
@@ -54,23 +82,6 @@ class OnlineManager extends Subscribable {
5482

5583
return navigator.onLine
5684
}
57-
58-
private setDefaultEventListener() {
59-
if (!isServer && window?.addEventListener) {
60-
this.setEventListener(onOnline => {
61-
const listener = () => onOnline()
62-
// Listen to online
63-
window.addEventListener('online', listener, false)
64-
window.addEventListener('offline', listener, false)
65-
66-
return () => {
67-
// Be sure to unsubscribe if a new handler is set
68-
window.removeEventListener('online', listener)
69-
window.removeEventListener('offline', listener)
70-
}
71-
})
72-
}
73-
}
7485
}
7586

7687
export const onlineManager = new OnlineManager()

‎src/core/queryObserver.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -759,9 +759,7 @@ function shouldFetchOptionally(
759759
return (
760760
options.enabled !== false &&
761761
(query !== prevQuery || prevOptions.enabled === false) &&
762-
(!options.suspense ||
763-
query.state.status !== 'error' ||
764-
prevOptions.enabled === false) &&
762+
(!options.suspense || query.state.status !== 'error') &&
765763
isStale(query, options)
766764
)
767765
}

‎src/core/tests/focusManager.test.tsx‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { sleep } from '../utils'
2-
import { focusManager } from '../focusManager'
2+
import { FocusManager } from '../focusManager'
33

44
describe('focusManager', () => {
5-
afterEach(() => {
6-
// Reset removeEventListener private property to avoid side effects between tests
7-
focusManager['removeEventListener'] = undefined
5+
let focusManager: FocusManager
6+
beforeEach(() => {
7+
focusManager = new FocusManager()
88
})
99

1010
it('should call previous remove handler when replacing an event listener', () => {
@@ -61,16 +61,14 @@ describe('focusManager', () => {
6161
globalThis.document = document
6262
})
6363

64-
it('should not set window listener if window.addEventListener is not defined', async () => {
64+
test('cleanup should still be undefined if window.addEventListener is not defined', async () => {
6565
const { addEventListener } = globalThis.window
6666

6767
// @ts-expect-error
6868
globalThis.window.addEventListener = undefined
6969

70-
const setEventListenerSpy = jest.spyOn(focusManager, 'setEventListener')
71-
7270
const unsubscribe = focusManager.subscribe(() => undefined)
73-
expect(setEventListenerSpy).toHaveBeenCalledTimes(0)
71+
expect(focusManager['cleanup']).toBeUndefined()
7472

7573
unsubscribe()
7674
globalThis.window.addEventListener = addEventListener
@@ -103,4 +101,43 @@ describe('focusManager', () => {
103101
addEventListenerSpy.mockRestore()
104102
removeEventListenerSpy.mockRestore()
105103
})
104+
105+
test('should call removeEventListener when last listener unsubscribes', () => {
106+
const addEventListenerSpy = jest.spyOn(
107+
globalThis.window,
108+
'addEventListener'
109+
)
110+
111+
const removeEventListenerSpy = jest.spyOn(
112+
globalThis.window,
113+
'removeEventListener'
114+
)
115+
116+
const unsubscribe1 = focusManager.subscribe(() => undefined)
117+
const unsubscribe2 = focusManager.subscribe(() => undefined)
118+
expect(addEventListenerSpy).toHaveBeenCalledTimes(2) // visibilitychange + focus
119+
120+
unsubscribe1()
121+
expect(removeEventListenerSpy).toHaveBeenCalledTimes(0)
122+
unsubscribe2()
123+
expect(removeEventListenerSpy).toHaveBeenCalledTimes(2) // visibilitychange + focus
124+
})
125+
126+
test('should keep setup function even if last listener unsubscribes', () => {
127+
const setupSpy = jest.fn().mockImplementation(() => () => undefined)
128+
129+
focusManager.setEventListener(setupSpy)
130+
131+
const unsubscribe1 = focusManager.subscribe(() => undefined)
132+
133+
expect(setupSpy).toHaveBeenCalledTimes(1)
134+
135+
unsubscribe1()
136+
137+
const unsubscribe2 = focusManager.subscribe(() => undefined)
138+
139+
expect(setupSpy).toHaveBeenCalledTimes(2)
140+
141+
unsubscribe2()
142+
})
106143
})

0 commit comments

Comments
 (0)