Skip to content

Commit 69e7270

Browse files
committed
fixed shouldComponentUpdate bug in createInject where changing props did not trigger a re-render
1 parent 0cdce42 commit 69e7270

File tree

2 files changed

+96
-1
lines changed

2 files changed

+96
-1
lines changed

src/components/createInject.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ export default function createInject(React) {
3939
};
4040

4141
shouldComponentUpdate(nextProps, nextState, nextContext) {
42-
return !shallowEqual(this.state.provided, nextContext.provided);
42+
return !shallowEqual(this.state.provided, nextState.provided) ||
43+
!shallowEqual(this.props, nextProps);
4344
}
4445

4546
constructor(props, context) {

test/components/inject.spec.js

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,5 +176,99 @@ describe('React', () => {
176176
expect(child.props.saying).toBe('goodbye');
177177

178178
});
179+
180+
it('should rerender component if props change even if provided does not.', () => {
181+
const spy = expect.createSpy(() => ({}));
182+
function render({ saying, changingProp }) {
183+
spy();
184+
return <div saying={saying} changingProp={changingProp} />;
185+
}
186+
187+
const InjectedChild = inject()(Child);
188+
189+
@inject()
190+
class InjectedContainer extends Component {
191+
render() {
192+
return render(this.props);
193+
}
194+
}
195+
196+
class ProviderContainer extends Component {
197+
constructor(props, context) {
198+
super(props, context);
199+
this.state = {
200+
saying: "hello",
201+
otherProp: "A",
202+
};
203+
}
204+
render() {
205+
return (
206+
<SimpleProvider saying={this.state.saying}>
207+
{() => <InjectedContainer changingProp={this.state.otherProp} />}
208+
</SimpleProvider>
209+
);
210+
}
211+
}
212+
213+
const tree = TestUtils.renderIntoDocument(<ProviderContainer />);
214+
215+
const child = TestUtils.findRenderedDOMComponentWithTag(tree, 'div');
216+
expect(spy.calls.length).toBe(1);
217+
expect(child.props.saying).toBe('hello');
218+
expect(child.props.changingProp).toBe('A');
219+
220+
tree.setState({otherProp: "B"});
221+
222+
expect(spy.calls.length).toBe(2);
223+
expect(child.props.saying).toBe('hello');
224+
expect(child.props.changingProp).toBe('B');
225+
226+
});
227+
228+
it('should not rerender if injected props remain the same.', () => {
229+
const spy = expect.createSpy(() => ({}));
230+
function render({ otherProp }) {
231+
spy();
232+
return <div otherProp={otherProp} />;
233+
}
234+
235+
const InjectedChild = inject()(Child);
236+
237+
@inject(provided => ({otherProp: provided.otherProp}))
238+
class InjectedContainer extends Component {
239+
render() {
240+
return render(this.props);
241+
}
242+
}
243+
244+
class ProviderContainer extends Component {
245+
constructor(props, context) {
246+
super(props, context);
247+
this.state = {
248+
saying: "hello",
249+
otherProp: "A",
250+
};
251+
}
252+
render() {
253+
return (
254+
<SimpleProvider {...this.state}>
255+
{() => <InjectedContainer />}
256+
</SimpleProvider>
257+
);
258+
}
259+
}
260+
261+
const tree = TestUtils.renderIntoDocument(<ProviderContainer />);
262+
263+
const child = TestUtils.findRenderedDOMComponentWithTag(tree, 'div');
264+
expect(spy.calls.length).toBe(1);
265+
expect(child.props.otherProp).toBe('A');
266+
267+
tree.setState({saying: "goodbye"});
268+
269+
expect(spy.calls.length).toBe(1);
270+
expect(child.props.otherProp).toBe('A');
271+
272+
});
179273
});
180274
});

0 commit comments

Comments
 (0)