@samitouri / QOS-React / commits / 40fa616053

Subscriptions shouldn't call setState after unmount even for Promises (#12425)

Brian Vaughn committed Mar 22, 2018 at 08:54 UTC 40fa6160534e2bc67b0c43f0b32dcdd1d214068b
2 files changed +41
packages/create-subscription/src/__tests__/createSubscription-test.internal.js
+32
@@ -189,6 +189,38 @@ describe('createSubscription', () => {
189 // Ensure that only Promise B causes an update
190 expect(ReactNoop.flush()).toEqual([123]);
191 });
192 +
193 + it('should not call setState for a Promise that resolves after unmount', async () => {
194 + const Subscription = createSubscription({
195 + getCurrentValue: source => undefined,
196 + subscribe: (source, callback) => {
197 + source.then(value => callback(value), value => callback(value));
198 + // (Can't unsubscribe from a Promise)
199 + return () => {};
200 + },
201 + });
202 +
203 + function render(hasLoaded) {
204 + ReactNoop.yield('rendered');
205 + return null;
206 + }
207 +
208 + let resolvePromise;
209 + const promise = new Promise((resolve, reject) => {
210 + resolvePromise = resolve;
211 + });
212 +
213 + ReactNoop.render(<Subscription source={promise}>{render}</Subscription>);
214 + expect(ReactNoop.flush()).toEqual(['rendered']);
215 +
216 + // Unmount
217 + ReactNoop.render(null);
218 + ReactNoop.flush();
219 +
220 + // Resolve Promise should not trigger a setState warning
221 + resolvePromise(true);
222 + await promise;
223 + });
224 });
225
226 it('should unsubscribe from old subscribables and subscribe to new subscribables when props change', () => {
packages/create-subscription/src/createSubscription.js
+9
@@ -64,6 +64,7 @@ export function createSubscription<Property, Value>(
64 : undefined,
65 };
66
67 + _hasUnmounted: boolean = false;
68 _unsubscribe: Unsubscribe | null = null;
69
70 static getDerivedStateFromProps(nextProps, prevState) {
@@ -93,6 +94,10 @@ export function createSubscription<Property, Value>(
94
95 componentWillUnmount() {
96 this.unsubscribe(this.state);
97 +
98 + // Track mounted to avoid calling setState after unmounting
99 + // For source like Promises that can't be unsubscribed from.
100 + this._hasUnmounted = true;
101 }
102
103 render() {
@@ -103,6 +108,10 @@ export function createSubscription<Property, Value>(
108 const {source} = this.state;
109 if (source != null) {
110 const callback = (value: Value | void) => {
111 + if (this._hasUnmounted) {
112 + return;
113 + }
114 +
115 this.setState(state => {
116 // If the value is the same, skip the unnecessary state update.
117 if (value === state.value) {