Skip to content

Commit

Permalink
fix(batch): replace unstable batch api with custom queue
Browse files Browse the repository at this point in the history
  • Loading branch information
rolandszoke authored and solkimicreb committed Feb 14, 2020
1 parent a082123 commit df56b5b
Show file tree
Hide file tree
Showing 32 changed files with 417 additions and 317 deletions.
3 changes: 1 addition & 2 deletions __tests__/Clock.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { StrictMode } from 'react'
import { act } from 'react-dom/test-utils'
import { render, cleanup } from '@testing-library/react/pure'
import { render, cleanup, act } from '@testing-library/react/pure'
import sinon from 'sinon'
import App from '../examples/clock/src/App'

Expand Down
4 changes: 1 addition & 3 deletions __tests__/Contacts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ describe('Contacts App', () => {
})

test('should edit contact', () => {
let display,
editor,
editButton
let display, editor, editButton

display = container.querySelector('.contact-display')
editButton = display.querySelector('.zmdi-edit')
Expand Down
128 changes: 94 additions & 34 deletions __tests__/batching.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react'
import { render, cleanup } from '@testing-library/react/pure'
import React, { Component } from 'react'
import { render, cleanup, act } from '@testing-library/react/pure'
import { view, store, batch } from 'react-easy-state'

describe('batching', () => {
Expand All @@ -13,6 +13,31 @@ describe('batching', () => {
return <div>{person.name}</div>
})

const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
act(() =>
batch(() => {
person.name = 'Ann'
person.name = 'Rick'
})
)
expect(container).toHaveTextContent('Rick')
expect(renderCount).toBe(2)
})

test('should batch state changes inside a batch() wrapper in a class component', () => {
let renderCount = 0
const person = store({ name: 'Bob' })
const MyComp = view(
class extends Component {
render () {
renderCount++
return <div>{person.name}</div>
}
}
)

const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
Expand All @@ -24,7 +49,7 @@ describe('batching', () => {
expect(renderCount).toBe(2)
})

test('should work with nested batch() calls', () => {
test('should not batch state changes outside a batch() wrapper', () => {
let renderCount = 0
const person = store({ name: 'Bob' })
const MyComp = view(() => {
Expand All @@ -35,16 +60,39 @@ describe('batching', () => {
const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
batch(() => {
batch(() => {
person.name = 'Rob'
person.name = 'David'
})
expect(container).toHaveTextContent('Bob')
act(() => {
person.name = 'Ann'
})
act(() => {
person.name = 'Rick'
})
expect(container).toHaveTextContent('Rick')
expect(renderCount).toBe(3)
})

test('should work with nested batch() calls', () => {
let renderCount = 0
const person = store({ name: 'Bob' })
const MyComp = view(() => {
renderCount++
return <div>{person.name}</div>
})

const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
act(() =>
batch(() => {
batch(() => {
person.name = 'Rob'
person.name = 'David'
})
expect(container).toHaveTextContent('Bob')
person.name = 'Ann'
person.name = 'Rick'
})
)
expect(container).toHaveTextContent('Rick')
expect(renderCount).toBe(2)
})

Expand All @@ -59,10 +107,10 @@ describe('batching', () => {
const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
const batched = () => {
const batched = act(() => {
person.name = 'Ann'
person.name = 'Rick'
}
})
document.body.addEventListener('click', batched)
document.body.dispatchEvent(new Event('click'))
expect(container).toHaveTextContent('Rick')
Expand All @@ -83,12 +131,17 @@ describe('batching', () => {
const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
await new Promise(resolve =>
setTimeout(() => {
person.name = 'Ann'
person.name = 'Rick'
resolve()
}, 100)
await act(
() =>
new Promise(
resolve =>
setTimeout(() => {
person.name = 'Ann'
person.name = 'Rick'
resolve()
}),
100
)
)
expect(container).toHaveTextContent('Rick')
expect(renderCount).toBe(2)
Expand All @@ -105,13 +158,16 @@ describe('batching', () => {
const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
await new Promise(resolve =>
// eslint-disable-next-line
requestAnimationFrame(() => {
person.name = 'Ann'
person.name = 'Rick'
resolve()
})
await act(
() =>
new Promise(resolve =>
// eslint-disable-next-line
requestAnimationFrame(() => {
person.name = 'Ann'
person.name = 'Rick'
resolve()
})
)
)
expect(container).toHaveTextContent('Rick')
expect(renderCount).toBe(2)
Expand All @@ -128,17 +184,21 @@ describe('batching', () => {
const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
await Promise.resolve().then(() => {
person.name = 'Ann'
person.name = 'Rick'
})
await act(() =>
Promise.resolve().then(() => {
person.name = 'Ann'
person.name = 'Rick'
})
)
expect(container).toHaveTextContent('Rick')
expect(renderCount).toBe(2)

await Promise.reject(new Error()).catch(() => {
person.name = 'Ben'
person.name = 'Morty'
})
await act(() =>
Promise.reject(new Error()).catch(() => {
person.name = 'Ben'
person.name = 'Morty'
})
)
expect(container).toHaveTextContent('Morty')
expect(renderCount).toBe(3)
})
Expand All @@ -154,11 +214,11 @@ describe('batching', () => {
const { container } = render(<MyComp />)
expect(renderCount).toBe(1)
expect(container).toHaveTextContent('Bob')
await Promise.resolve()
await act(() => Promise.resolve())
person.name = 'Ann'
person.name = 'Rick'
// ISSUE -> here it is not yet updated!!! -> the then block is not over I guess
await Promise.resolve()
await act(() => Promise.resolve())
expect(container).toHaveTextContent('Rick')
expect(renderCount).toBe(2)
})
Expand Down
12 changes: 7 additions & 5 deletions __tests__/edgeCases.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { Component, useState } from 'react'
import { render, cleanup, fireEvent } from '@testing-library/react/pure'
import { render, cleanup, fireEvent, act } from '@testing-library/react/pure'
import { view, store, batch } from 'react-easy-state'

describe('edge cases', () => {
Expand Down Expand Up @@ -50,7 +50,8 @@ describe('edge cases', () => {
const MyComp = view(
class extends Component {
state = { counter: 0 };
handleIncrement = () => this.setState({ counter: this.state.counter + 1 });
handleIncrement = () =>
this.setState({ counter: this.state.counter + 1 });

render () {
return <div onClick={this.handleIncrement}>{this.state.counter}</div>
Expand All @@ -68,7 +69,8 @@ describe('edge cases', () => {
const MyComp = view(
class extends Component {
state = { counter: 0 };
handleIncrement = () => this.setState({ counter: this.state.counter + 1 });
handleIncrement = () =>
this.setState({ counter: this.state.counter + 1 });

render () {
return (
Expand Down Expand Up @@ -182,7 +184,7 @@ describe('edge cases', () => {
expect(container).toHaveTextContent('12, 1')
expect(parentCalls).toBe(1)
expect(childCalls).toBe(1)
batch(change)
act(() => batch(change))
expect(container).toHaveTextContent('')
expect(parentCalls).toBe(2)
expect(childCalls).toBe(1)
Expand Down Expand Up @@ -225,7 +227,7 @@ describe('edge cases', () => {
expect(container).toHaveTextContent('12, 1')
expect(parentCalls).toBe(1)
expect(childCalls).toBe(1)
batch(change)
act(() => batch(change))
expect(container).toHaveTextContent('')
expect(parentCalls).toBe(2)
expect(childCalls).toBe(1)
Expand Down
3 changes: 1 addition & 2 deletions __tests__/router.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { Component } from 'react'
import { act } from 'react-dom/test-utils'
import { render, cleanup, fireEvent } from '@testing-library/react/pure'
import { render, cleanup, fireEvent, act } from '@testing-library/react/pure'
import { view, store } from 'react-easy-state'
import {
BrowserRouter as Router,
Expand Down
3 changes: 1 addition & 2 deletions __tests__/styled.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { Component } from 'react'
import { act } from 'react-dom/test-utils'
import { render, cleanup } from '@testing-library/react/pure'
import { render, cleanup, act } from '@testing-library/react/pure'
import { view, store } from 'react-easy-state'
import { withTheme, ThemeProvider } from 'styled-components'

Expand Down
8 changes: 4 additions & 4 deletions examples/beer-finder/src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react'
import NavBar from './NavBar'
import BeerList from './BeerList'
import React from "react";
import NavBar from "./NavBar";
import BeerList from "./BeerList";

// if a component does not use any store, it doesn't have to be wrapped with view()
// it is safer to wrap everything with view() until you get more comfortable with Easy State
Expand All @@ -9,4 +9,4 @@ export default () => (
<NavBar />
<BeerList />
</>
)
);
20 changes: 10 additions & 10 deletions examples/beer-finder/src/Beer.jsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import React from 'react'
import { view, store } from 'react-easy-state'
import Card from '@material-ui/core/Card'
import CardMedia from '@material-ui/core/CardMedia'
import CardContent from '@material-ui/core/CardContent'
import React from "react";
import { view, store } from "react-easy-state";
import Card from "@material-ui/core/Card";
import CardMedia from "@material-ui/core/CardMedia";
import CardContent from "@material-ui/core/CardContent";

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

return (
<Card onClick={() => (beer.details = !beer.details)} className='beer'>
{!beer.details && <CardMedia image={imageUrl} className='media' />}
<Card onClick={() => (beer.details = !beer.details)} className="beer">
{!beer.details && <CardMedia image={imageUrl} className="media" />}
<CardContent>
<h3>{name}</h3>
{beer.details ? (
Expand All @@ -25,6 +25,6 @@ export default view(
)}
</CardContent>
</Card>
)
);
}
)
);
12 changes: 6 additions & 6 deletions examples/beer-finder/src/BeerList.jsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import React from 'react'
import { view } from 'react-easy-state'
import appStore from './appStore'
import Beer from './Beer'
import React from "react";
import { view } from "react-easy-state";
import appStore from "./appStore";
import Beer from "./Beer";

// this is re-rendered whenever the relevant parts of the used data stores change
export default view(() => (
<div className='beerlist'>
<div className="beerlist">
{!appStore.beers.length ? (
<h3>No matching beers found!</h3>
) : (
appStore.beers.map(beer => <Beer key={beer.name} {...beer} />)
)}
</div>
))
));
16 changes: 8 additions & 8 deletions examples/beer-finder/src/NavBar.jsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import React from 'react'
import { view } from 'react-easy-state'
import SearchBar from 'material-ui-search-bar'
import LinearProgress from '@material-ui/core/LinearProgress'
import appStore from './appStore'
import React from "react";
import { view } from "react-easy-state";
import SearchBar from "material-ui-search-bar";
import LinearProgress from "@material-ui/core/LinearProgress";
import appStore from "./appStore";

// this is re-rendered whenever the relevant parts of the used data stores change
export default view(() => (
<div className='searchbar'>
<div className="searchbar">
<SearchBar
onRequestSearch={appStore.fetchBeers}
placeholder='Add some food ...'
placeholder="Add some food ..."
autoFocus
/>
{appStore.isLoading && <LinearProgress />}
</div>
))
));

0 comments on commit df56b5b

Please sign in to comment.