forked from reduxjs/react-redux
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add connectAdvanced() tests (reduxjs#1079)
Since the connectAdvanced() is a public api I think it should have some tests. Unfortunately these tests are intentionally failing because I'm not sure how connectAdvanved() should really work. This is just my best guess. For example the behaviour is very different between 5.x and 6 beta. Different tests fail between those and in common failures the errors are different. I have created [redux-render-prop](https://github.com/epeli/redux-render-prop) module which relies on the connectAdvanced() primitive and I noticed that all the important performance related [tests started failing](https://travis-ci.com/epeli/redux-render-prop/jobs/157787740) when I upgraded to 6 beta. Most importantly [`unrelated state updates don't cause render`](https://github.com/epeli/redux-render-prop/blob/aa2aa9b299e5859f7a79bc1c926ed75907f63dcb/__tests__/redux-render-prop.test.tsx#L284-L349). I've added matching test in this PR called `should not render when the returned reference does not change`. I've also observed that in 5.x the state mapping function gets called twice on component mount for which I had to create [a very ugly workaround](https://github.com/epeli/redux-render-prop/blob/aa2aa9b299e5859f7a79bc1c926ed75907f63dcb/src/redux-render-prop.ts#L188-L195) in redux-render-prop to avoid unnecessary rendering. I've added test for that case too (`should map state and render once on mount`). This seems to be fixed in the 6 beta. I hope we can have more stable behaviour for the connectAdvanced() in future.
- Loading branch information
1 parent
1dc56ca
commit dba598b
Showing
2 changed files
with
184 additions
and
5 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
import React, { Component } from 'react' | ||
import * as rtl from 'react-testing-library' | ||
import { Provider as ProviderMock, connectAdvanced } from '../../src/index.js' | ||
import { createStore } from 'redux' | ||
import 'jest-dom/extend-expect' | ||
|
||
describe('React', () => { | ||
describe('connectAdvanced', () => { | ||
it('should map state and render once on mount', () => { | ||
const initialState = { | ||
foo: 'bar' | ||
} | ||
|
||
let mapCount = 0 | ||
let renderCount = 0 | ||
|
||
const store = createStore(() => initialState) | ||
|
||
function Inner(props) { | ||
renderCount++ | ||
return <div data-testid="foo">{JSON.stringify(props)}</div> | ||
} | ||
|
||
const Container = connectAdvanced(() => { | ||
return state => { | ||
mapCount++ | ||
return state | ||
} | ||
})(Inner) | ||
|
||
const tester = rtl.render( | ||
<ProviderMock store={store}> | ||
<Container /> | ||
</ProviderMock> | ||
) | ||
|
||
expect(tester.getByTestId('foo')).toHaveTextContent('bar') | ||
|
||
expect(mapCount).toEqual(1) | ||
expect(renderCount).toEqual(1) | ||
}) | ||
|
||
it('should render on reference change', () => { | ||
let mapCount = 0 | ||
let renderCount = 0 | ||
|
||
// force new reference on each dispatch | ||
const store = createStore(() => ({ | ||
foo: 'bar' | ||
})) | ||
|
||
function Inner(props) { | ||
renderCount++ | ||
return <div data-testid="foo">{JSON.stringify(props)}</div> | ||
} | ||
|
||
const Container = connectAdvanced(() => { | ||
return state => { | ||
mapCount++ | ||
return state | ||
} | ||
})(Inner) | ||
|
||
rtl.render( | ||
<ProviderMock store={store}> | ||
<Container /> | ||
</ProviderMock> | ||
) | ||
|
||
store.dispatch({ type: 'NEW_REFERENCE' }) | ||
|
||
// Should have mapped the state on mount and on the dispatch | ||
expect(mapCount).toEqual(2) | ||
|
||
// Should have rendered on mount and after the dispatch bacause the map | ||
// state returned new reference | ||
expect(renderCount).toEqual(2) | ||
}) | ||
|
||
it('should not render when the returned reference does not change', () => { | ||
const staticReference = { | ||
foo: 'bar' | ||
} | ||
|
||
let mapCount = 0 | ||
let renderCount = 0 | ||
|
||
// force new reference on each dispatch | ||
const store = createStore(() => ({ | ||
foo: 'bar' | ||
})) | ||
|
||
function Inner(props) { | ||
renderCount++ | ||
return <div data-testid="foo">{JSON.stringify(props)}</div> | ||
} | ||
|
||
const Container = connectAdvanced(() => { | ||
return () => { | ||
mapCount++ | ||
// but return static reference | ||
return staticReference | ||
} | ||
})(Inner) | ||
|
||
const tester = rtl.render( | ||
<ProviderMock store={store}> | ||
<Container /> | ||
</ProviderMock> | ||
) | ||
|
||
store.dispatch({ type: 'NEW_REFERENCE' }) | ||
|
||
expect(tester.getByTestId('foo')).toHaveTextContent('bar') | ||
|
||
// The state should have been mapped twice: on mount and on the dispatch | ||
expect(mapCount).toEqual(2) | ||
|
||
// But the render should have been called only on mount since the map state | ||
// did not return a new reference | ||
expect(renderCount).toEqual(1) | ||
}) | ||
|
||
it('should map state on own props change but not render when the reference does not change', () => { | ||
const staticReference = { | ||
foo: 'bar' | ||
} | ||
|
||
let mapCount = 0 | ||
let renderCount = 0 | ||
|
||
const store = createStore(() => staticReference) | ||
|
||
function Inner(props) { | ||
renderCount++ | ||
return <div data-testid="foo">{JSON.stringify(props)}</div> | ||
} | ||
|
||
const Container = connectAdvanced(() => { | ||
return () => { | ||
mapCount++ | ||
// return the static reference | ||
return staticReference | ||
} | ||
})(Inner) | ||
|
||
class OuterComponent extends Component { | ||
constructor() { | ||
super() | ||
this.state = { foo: 'FOO' } | ||
} | ||
|
||
setFoo(foo) { | ||
this.setState({ foo }) | ||
} | ||
|
||
render() { | ||
return ( | ||
<div> | ||
<Container {...this.state} /> | ||
</div> | ||
) | ||
} | ||
} | ||
|
||
let outerComponent | ||
rtl.render( | ||
<ProviderMock store={store}> | ||
<OuterComponent ref={c => (outerComponent = c)} /> | ||
</ProviderMock> | ||
) | ||
|
||
outerComponent.setFoo('BAR') | ||
|
||
// map on mount and on prop change | ||
expect(mapCount).toEqual(2) | ||
|
||
// render only on mount but skip on prop change because no new | ||
// reference was returned | ||
expect(renderCount).toEqual(1) | ||
}) | ||
}) | ||
}) |