Update connect implementation to remap state on prop changes. Fixes #120

developit · Jan 4, 2019 · 1028258 · 1028258
1 parent 7e08849
commit 1028258
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 2 deletions.
diff --git a/src/integrations/preact.js b/src/integrations/preact.js
@@ -27,13 +27,16 @@ export function connect(mapStateToProps, actions) {
 				let mapped = mapStateToProps(store ? store.getState() : {}, this.props);
 				for (let i in mapped) if (mapped[i]!==state[i]) {
 					state = mapped;
-					return this.setState(null);
+					this.setState();
+					return;
 				}
 				for (let i in state) if (!(i in mapped)) {
 					state = mapped;
-					return this.setState(null);
+					this.setState();
+					return;
 				}
 			};
+			this.componentWillReceiveProps = update;
 			this.componentDidMount = () => {
 				store.subscribe(update);
 			};

diff --git a/src/integrations/react.js b/src/integrations/react.js
@@ -39,6 +39,7 @@ export function connect(mapStateToProps, actions) {
 					return this.forceUpdate();
 				}
 			};
+			this.componentWillReceiveProps = update;
 			this.componentDidMount = () => {
 				store.subscribe(update);
 			};

diff --git a/test/preact/unistore.test.js b/test/preact/unistore.test.js
@@ -198,4 +198,73 @@ describe('connect()', () => {
 		await sleep(1);
 		expect(Child).not.toHaveBeenCalled();
 	});
+
+	it('should run mapStateToProps and update when outer props change', () => {
+		let state = {};
+		const store = { subscribe: jest.fn(), getState: () => state };
+		const Child = jest.fn().mockName('<Child>');
+		let mappings = 0;
+
+		// Jest mock return values are broken :(
+		const mapStateToProps = jest.fn((state, props) => ({ mappings: ++mappings, ...props }));
+
+		// const mapper = jest.fn();
+		// const mapStateToProps = (state, props) => {
+		// 	mapper(state, props);
+		// 	mappings++;
+		// 	return { mappings, ...props };
+		// };
+
+		const ConnectedChild = connect(mapStateToProps)(Child);
+		let root = render(
+			<Provider store={store}>
+				<ConnectedChild />
+			</Provider>,
+			document.body
+		);
+
+		expect(mapStateToProps).toHaveBeenCalledTimes(1);
+		expect(mapStateToProps).toHaveBeenCalledWith({}, { children: expect.anything() });
+		// first render calls mapStateToProps
+		expect(Child).toHaveBeenCalledWith(
+			{ mappings: 1, store, children: expect.anything() },
+			expect.anything()
+		);
+
+		mapStateToProps.mockClear();
+		Child.mockClear();
+
+		render(
+			<Provider store={store}>
+				<ConnectedChild a="b" />
+			</Provider>,
+			document.body,
+			root
+		);
+
+		expect(mapStateToProps).toHaveBeenCalledTimes(1);
+		expect(mapStateToProps).toHaveBeenCalledWith({}, { children: expect.anything() });
+		// outer props were changed
+		expect(Child).toHaveBeenCalledWith(
+			{ mappings: 2, a: 'b', store, children: expect.anything() },
+			expect.anything()
+		);
+
+		mapStateToProps.mockClear();
+		Child.mockClear();
+
+		render(
+			<Provider store={store}>
+				<ConnectedChild a="b" />
+			</Provider>,
+			document.body,
+			root
+		);
+
+		// re-rendered, but outer props were not changed
+		expect(Child).toHaveBeenCalledWith(
+			{ mappings: 3, a: 'b', store, children: expect.anything() },
+			expect.anything()
+		);
+	});
 });
diff --git a/test/react/unistore.test.js b/test/react/unistore.test.js
@@ -192,4 +192,72 @@ describe('<Provider>', () => {
 		mountedProvider.unmount();
 		expect(store.unsubscribe).toBeCalled();
 	});
+
+	it('should run mapStateToProps and update when outer props change', async () => {
+		let state = {};
+		const store = { subscribe: jest.fn(), getState: () => state };
+		const Child = jest.fn(() => null).mockName('<Child>');
+		let mappings = 0;
+
+		// Jest mock return values are broken :(
+		const mapStateToProps = jest.fn((state, props) => ({ mappings: ++mappings, ...props }));
+
+		let root;
+		class Outer extends Component {
+			constructor() {
+				super();
+				this.state = {};
+				root = this;
+				root.setProps = props => this.setState({ props });
+			}
+			render() {
+				root = this;
+				return (
+					<Provider store={store}>
+						<ConnectedChild {...(this.state.props || this.props)} />
+					</Provider>
+				);
+			}
+		}
+
+		const ConnectedChild = connect(mapStateToProps)(Child);
+		const mountedProvider = mount(<Outer />);
+
+		expect(mapStateToProps).toHaveBeenCalledTimes(1);
+		expect(mapStateToProps).toHaveBeenCalledWith({}, { });
+		// first render calls mapStateToProps
+		expect(Child).toHaveBeenCalledWith(
+			{ mappings: 1, store },
+			expect.anything()
+		);
+
+		mapStateToProps.mockClear();
+		Child.mockClear();
+
+		// root.setState({ a: 'b' });
+		mountedProvider.setProps({ a: 'b' });
+
+		// await sleep(100);
+
+		expect(mapStateToProps).toHaveBeenCalledTimes(1);
+		expect(mapStateToProps).toHaveBeenCalledWith({}, {});
+		// outer props were changed
+		expect(Child).toHaveBeenCalledWith(
+			{ mappings: 2, a: 'b', store },
+			expect.anything()
+		);
+
+		mapStateToProps.mockClear();
+		Child.mockClear();
+
+		mountedProvider.setProps({ });
+
+		await sleep(1);
+
+		// re-rendered, but outer props were not changed
+		expect(Child).toHaveBeenCalledWith(
+			{ mappings: 3, a: 'b', store },
+			expect.anything()
+		);
+	});
 });