Skip to content

Commit df56b5b

Browse files
rolandszokesolkimicreb
rolandszoke
authored and
solkimicreb
committed
fix(batch): replace unstable batch api with custom queue
1 parent a082123 commit df56b5b

32 files changed

+417
-317
lines changed

‎__tests__/Clock.test.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import React, { StrictMode } from 'react'
2-
import { act } from 'react-dom/test-utils'
3-
import { render, cleanup } from '@testing-library/react/pure'
2+
import { render, cleanup, act } from '@testing-library/react/pure'
43
import sinon from 'sinon'
54
import App from '../examples/clock/src/App'
65

‎__tests__/Contacts.test.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ describe('Contacts App', () => {
4141
})
4242

4343
test('should edit contact', () => {
44-
let display,
45-
editor,
46-
editButton
44+
let display, editor, editButton
4745

4846
display = container.querySelector('.contact-display')
4947
editButton = display.querySelector('.zmdi-edit')

‎__tests__/batching.test.js

Lines changed: 94 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import React from 'react'
2-
import { render, cleanup } from '@testing-library/react/pure'
1+
import React, { Component } from 'react'
2+
import { render, cleanup, act } from '@testing-library/react/pure'
33
import { view, store, batch } from 'react-easy-state'
44

55
describe('batching', () => {
@@ -13,6 +13,31 @@ describe('batching', () => {
1313
return <div>{person.name}</div>
1414
})
1515

16+
const { container } = render(<MyComp />)
17+
expect(renderCount).toBe(1)
18+
expect(container).toHaveTextContent('Bob')
19+
act(() =>
20+
batch(() => {
21+
person.name = 'Ann'
22+
person.name = 'Rick'
23+
})
24+
)
25+
expect(container).toHaveTextContent('Rick')
26+
expect(renderCount).toBe(2)
27+
})
28+
29+
test('should batch state changes inside a batch() wrapper in a class component', () => {
30+
let renderCount = 0
31+
const person = store({ name: 'Bob' })
32+
const MyComp = view(
33+
class extends Component {
34+
render () {
35+
renderCount++
36+
return <div>{person.name}</div>
37+
}
38+
}
39+
)
40+
1641
const { container } = render(<MyComp />)
1742
expect(renderCount).toBe(1)
1843
expect(container).toHaveTextContent('Bob')
@@ -24,7 +49,7 @@ describe('batching', () => {
2449
expect(renderCount).toBe(2)
2550
})
2651

27-
test('should work with nested batch() calls', () => {
52+
test('should not batch state changes outside a batch() wrapper', () => {
2853
let renderCount = 0
2954
const person = store({ name: 'Bob' })
3055
const MyComp = view(() => {
@@ -35,16 +60,39 @@ describe('batching', () => {
3560
const { container } = render(<MyComp />)
3661
expect(renderCount).toBe(1)
3762
expect(container).toHaveTextContent('Bob')
38-
batch(() => {
39-
batch(() => {
40-
person.name = 'Rob'
41-
person.name = 'David'
42-
})
43-
expect(container).toHaveTextContent('Bob')
63+
act(() => {
4464
person.name = 'Ann'
65+
})
66+
act(() => {
4567
person.name = 'Rick'
4668
})
4769
expect(container).toHaveTextContent('Rick')
70+
expect(renderCount).toBe(3)
71+
})
72+
73+
test('should work with nested batch() calls', () => {
74+
let renderCount = 0
75+
const person = store({ name: 'Bob' })
76+
const MyComp = view(() => {
77+
renderCount++
78+
return <div>{person.name}</div>
79+
})
80+
81+
const { container } = render(<MyComp />)
82+
expect(renderCount).toBe(1)
83+
expect(container).toHaveTextContent('Bob')
84+
act(() =>
85+
batch(() => {
86+
batch(() => {
87+
person.name = 'Rob'
88+
person.name = 'David'
89+
})
90+
expect(container).toHaveTextContent('Bob')
91+
person.name = 'Ann'
92+
person.name = 'Rick'
93+
})
94+
)
95+
expect(container).toHaveTextContent('Rick')
4896
expect(renderCount).toBe(2)
4997
})
5098

@@ -59,10 +107,10 @@ describe('batching', () => {
59107
const { container } = render(<MyComp />)
60108
expect(renderCount).toBe(1)
61109
expect(container).toHaveTextContent('Bob')
62-
const batched = () => {
110+
const batched = act(() => {
63111
person.name = 'Ann'
64112
person.name = 'Rick'
65-
}
113+
})
66114
document.body.addEventListener('click', batched)
67115
document.body.dispatchEvent(new Event('click'))
68116
expect(container).toHaveTextContent('Rick')
@@ -83,12 +131,17 @@ describe('batching', () => {
83131
const { container } = render(<MyComp />)
84132
expect(renderCount).toBe(1)
85133
expect(container).toHaveTextContent('Bob')
86-
await new Promise(resolve =>
87-
setTimeout(() => {
88-
person.name = 'Ann'
89-
person.name = 'Rick'
90-
resolve()
91-
}, 100)
134+
await act(
135+
() =>
136+
new Promise(
137+
resolve =>
138+
setTimeout(() => {
139+
person.name = 'Ann'
140+
person.name = 'Rick'
141+
resolve()
142+
}),
143+
100
144+
)
92145
)
93146
expect(container).toHaveTextContent('Rick')
94147
expect(renderCount).toBe(2)
@@ -105,13 +158,16 @@ describe('batching', () => {
105158
const { container } = render(<MyComp />)
106159
expect(renderCount).toBe(1)
107160
expect(container).toHaveTextContent('Bob')
108-
await new Promise(resolve =>
109-
// eslint-disable-next-line
110-
requestAnimationFrame(() => {
111-
person.name = 'Ann'
112-
person.name = 'Rick'
113-
resolve()
114-
})
161+
await act(
162+
() =>
163+
new Promise(resolve =>
164+
// eslint-disable-next-line
165+
requestAnimationFrame(() => {
166+
person.name = 'Ann'
167+
person.name = 'Rick'
168+
resolve()
169+
})
170+
)
115171
)
116172
expect(container).toHaveTextContent('Rick')
117173
expect(renderCount).toBe(2)
@@ -128,17 +184,21 @@ describe('batching', () => {
128184
const { container } = render(<MyComp />)
129185
expect(renderCount).toBe(1)
130186
expect(container).toHaveTextContent('Bob')
131-
await Promise.resolve().then(() => {
132-
person.name = 'Ann'
133-
person.name = 'Rick'
134-
})
187+
await act(() =>
188+
Promise.resolve().then(() => {
189+
person.name = 'Ann'
190+
person.name = 'Rick'
191+
})
192+
)
135193
expect(container).toHaveTextContent('Rick')
136194
expect(renderCount).toBe(2)
137195

138-
await Promise.reject(new Error()).catch(() => {
139-
person.name = 'Ben'
140-
person.name = 'Morty'
141-
})
196+
await act(() =>
197+
Promise.reject(new Error()).catch(() => {
198+
person.name = 'Ben'
199+
person.name = 'Morty'
200+
})
201+
)
142202
expect(container).toHaveTextContent('Morty')
143203
expect(renderCount).toBe(3)
144204
})
@@ -154,11 +214,11 @@ describe('batching', () => {
154214
const { container } = render(<MyComp />)
155215
expect(renderCount).toBe(1)
156216
expect(container).toHaveTextContent('Bob')
157-
await Promise.resolve()
217+
await act(() => Promise.resolve())
158218
person.name = 'Ann'
159219
person.name = 'Rick'
160220
// ISSUE -> here it is not yet updated!!! -> the then block is not over I guess
161-
await Promise.resolve()
221+
await act(() => Promise.resolve())
162222
expect(container).toHaveTextContent('Rick')
163223
expect(renderCount).toBe(2)
164224
})

‎__tests__/edgeCases.test.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React, { Component, useState } from 'react'
2-
import { render, cleanup, fireEvent } from '@testing-library/react/pure'
2+
import { render, cleanup, fireEvent, act } from '@testing-library/react/pure'
33
import { view, store, batch } from 'react-easy-state'
44

55
describe('edge cases', () => {
@@ -50,7 +50,8 @@ describe('edge cases', () => {
5050
const MyComp = view(
5151
class extends Component {
5252
state = { counter: 0 };
53-
handleIncrement = () => this.setState({ counter: this.state.counter + 1 });
53+
handleIncrement = () =>
54+
this.setState({ counter: this.state.counter + 1 });
5455

5556
render () {
5657
return <div onClick={this.handleIncrement}>{this.state.counter}</div>
@@ -68,7 +69,8 @@ describe('edge cases', () => {
6869
const MyComp = view(
6970
class extends Component {
7071
state = { counter: 0 };
71-
handleIncrement = () => this.setState({ counter: this.state.counter + 1 });
72+
handleIncrement = () =>
73+
this.setState({ counter: this.state.counter + 1 });
7274

7375
render () {
7476
return (
@@ -182,7 +184,7 @@ describe('edge cases', () => {
182184
expect(container).toHaveTextContent('12, 1')
183185
expect(parentCalls).toBe(1)
184186
expect(childCalls).toBe(1)
185-
batch(change)
187+
act(() => batch(change))
186188
expect(container).toHaveTextContent('')
187189
expect(parentCalls).toBe(2)
188190
expect(childCalls).toBe(1)
@@ -225,7 +227,7 @@ describe('edge cases', () => {
225227
expect(container).toHaveTextContent('12, 1')
226228
expect(parentCalls).toBe(1)
227229
expect(childCalls).toBe(1)
228-
batch(change)
230+
act(() => batch(change))
229231
expect(container).toHaveTextContent('')
230232
expect(parentCalls).toBe(2)
231233
expect(childCalls).toBe(1)

‎__tests__/router.test.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import React, { Component } from 'react'
2-
import { act } from 'react-dom/test-utils'
3-
import { render, cleanup, fireEvent } from '@testing-library/react/pure'
2+
import { render, cleanup, fireEvent, act } from '@testing-library/react/pure'
43
import { view, store } from 'react-easy-state'
54
import {
65
BrowserRouter as Router,

‎__tests__/styled.test.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import React, { Component } from 'react'
2-
import { act } from 'react-dom/test-utils'
3-
import { render, cleanup } from '@testing-library/react/pure'
2+
import { render, cleanup, act } from '@testing-library/react/pure'
43
import { view, store } from 'react-easy-state'
54
import { withTheme, ThemeProvider } from 'styled-components'
65

‎examples/beer-finder/src/App.jsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import React from 'react'
2-
import NavBar from './NavBar'
3-
import BeerList from './BeerList'
1+
import React from "react";
2+
import NavBar from "./NavBar";
3+
import BeerList from "./BeerList";
44

55
// if a component does not use any store, it doesn't have to be wrapped with view()
66
// it is safer to wrap everything with view() until you get more comfortable with Easy State
@@ -9,4 +9,4 @@ export default () => (
99
<NavBar />
1010
<BeerList />
1111
</>
12-
)
12+
);

‎examples/beer-finder/src/Beer.jsx

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1-
import React from 'react'
2-
import { view, store } from 'react-easy-state'
3-
import Card from '@material-ui/core/Card'
4-
import CardMedia from '@material-ui/core/CardMedia'
5-
import CardContent from '@material-ui/core/CardContent'
1+
import React from "react";
2+
import { view, store } from "react-easy-state";
3+
import Card from "@material-ui/core/Card";
4+
import CardMedia from "@material-ui/core/CardMedia";
5+
import CardContent from "@material-ui/core/CardContent";
66

77
// this is re-rendered whenever the relevant parts of the used data stores change
88
export default view(
99
({ name, description, image_url: imageUrl, food_pairing: foodPairing }) => {
10-
const beer = store({ details: false })
10+
const beer = store({ details: false });
1111

1212
return (
13-
<Card onClick={() => (beer.details = !beer.details)} className='beer'>
14-
{!beer.details && <CardMedia image={imageUrl} className='media' />}
13+
<Card onClick={() => (beer.details = !beer.details)} className="beer">
14+
{!beer.details && <CardMedia image={imageUrl} className="media" />}
1515
<CardContent>
1616
<h3>{name}</h3>
1717
{beer.details ? (
@@ -25,6 +25,6 @@ export default view(
2525
)}
2626
</CardContent>
2727
</Card>
28-
)
28+
);
2929
}
30-
)
30+
);

‎examples/beer-finder/src/BeerList.jsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
import React from 'react'
2-
import { view } from 'react-easy-state'
3-
import appStore from './appStore'
4-
import Beer from './Beer'
1+
import React from "react";
2+
import { view } from "react-easy-state";
3+
import appStore from "./appStore";
4+
import Beer from "./Beer";
55

66
// this is re-rendered whenever the relevant parts of the used data stores change
77
export default view(() => (
8-
<div className='beerlist'>
8+
<div className="beerlist">
99
{!appStore.beers.length ? (
1010
<h3>No matching beers found!</h3>
1111
) : (
1212
appStore.beers.map(beer => <Beer key={beer.name} {...beer} />)
1313
)}
1414
</div>
15-
))
15+
));

‎examples/beer-finder/src/NavBar.jsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1-
import React from 'react'
2-
import { view } from 'react-easy-state'
3-
import SearchBar from 'material-ui-search-bar'
4-
import LinearProgress from '@material-ui/core/LinearProgress'
5-
import appStore from './appStore'
1+
import React from "react";
2+
import { view } from "react-easy-state";
3+
import SearchBar from "material-ui-search-bar";
4+
import LinearProgress from "@material-ui/core/LinearProgress";
5+
import appStore from "./appStore";
66

77
// this is re-rendered whenever the relevant parts of the used data stores change
88
export default view(() => (
9-
<div className='searchbar'>
9+
<div className="searchbar">
1010
<SearchBar
1111
onRequestSearch={appStore.fetchBeers}
12-
placeholder='Add some food ...'
12+
placeholder="Add some food ..."
1313
autoFocus
1414
/>
1515
{appStore.isLoading && <LinearProgress />}
1616
</div>
17-
))
17+
));

0 commit comments

Comments
 (0)