Skip to content

Commit 64f0b44

Browse files
author
Brian Vaughn
committed
Refactored useSubscription to remove explicit source param and rely on useMemo
1 parent fef75b3 commit 64f0b44

File tree

2 files changed

+167
-119
lines changed

2 files changed

+167
-119
lines changed

packages/react-hooks/src/__tests__/useSubscription-test.internal.js

+138-94
Original file line numberDiff line numberDiff line change
@@ -49,29 +49,27 @@ describe('useSubscription', () => {
4949
return replaySubject;
5050
}
5151

52-
// Mimic createSubscription API to simplify testing
53-
function createSubscription({getCurrentValue, subscribe}) {
54-
function DefaultChild({value = 'default'}) {
52+
it('supports basic subscription pattern', () => {
53+
function Child({value = 'default'}) {
5554
Scheduler.yieldValue(value);
5655
return null;
5756
}
5857

59-
return function Subscription({children, source}) {
58+
function Subscription({source}) {
6059
const value = useSubscription(
61-
React.useMemo(() => ({source, getCurrentValue, subscribe}), [source]),
60+
React.useMemo(
61+
() => ({
62+
getCurrentValue: () => source.getValue(),
63+
subscribe: callback => {
64+
const subscription = source.subscribe(callback);
65+
return () => subscription.unsubscribe();
66+
},
67+
}),
68+
[source],
69+
),
6270
);
63-
return React.createElement(children || DefaultChild, {value});
64-
};
65-
}
66-
67-
it('supports basic subscription pattern', () => {
68-
const Subscription = createSubscription({
69-
getCurrentValue: source => source.getValue(),
70-
subscribe: (source, callback) => {
71-
const subscription = source.subscribe(callback);
72-
return () => subscription.unsubscribe();
73-
},
74-
});
71+
return <Child value={value} />;
72+
}
7573

7674
const observable = createBehaviorSubject();
7775
const renderer = ReactTestRenderer.create(
@@ -93,21 +91,34 @@ describe('useSubscription', () => {
9391
});
9492

9593
it('should support observable types like RxJS ReplaySubject', () => {
96-
const Subscription = createSubscription({
97-
getCurrentValue: source => {
98-
let currentValue;
99-
source
100-
.subscribe(value => {
101-
currentValue = value;
102-
})
103-
.unsubscribe();
104-
return currentValue;
105-
},
106-
subscribe: (source, callback) => {
107-
const subscription = source.subscribe(callback);
108-
return () => subscription.unsubscribe;
109-
},
110-
});
94+
function Child({value = 'default'}) {
95+
Scheduler.yieldValue(value);
96+
return null;
97+
}
98+
99+
function Subscription({source}) {
100+
const value = useSubscription(
101+
React.useMemo(
102+
() => ({
103+
getCurrentValue: () => {
104+
let currentValue;
105+
source
106+
.subscribe(tempValue => {
107+
currentValue = tempValue;
108+
})
109+
.unsubscribe();
110+
return currentValue;
111+
},
112+
subscribe: callback => {
113+
const subscription = source.subscribe(callback);
114+
return () => subscription.unsubscribe();
115+
},
116+
}),
117+
[source],
118+
),
119+
);
120+
return <Child value={value} />;
121+
}
111122

112123
let observable = createReplaySubject('initial');
113124
const renderer = ReactTestRenderer.create(
@@ -127,13 +138,26 @@ describe('useSubscription', () => {
127138
});
128139

129140
it('should unsubscribe from old subscribables and subscribe to new subscribables when props change', () => {
130-
const Subscription = createSubscription({
131-
getCurrentValue: source => source.getValue(),
132-
subscribe: (source, callback) => {
133-
const subscription = source.subscribe(callback);
134-
return () => subscription.unsubscribe();
135-
},
136-
});
141+
function Child({value = 'default'}) {
142+
Scheduler.yieldValue(value);
143+
return null;
144+
}
145+
146+
function Subscription({source}) {
147+
const value = useSubscription(
148+
React.useMemo(
149+
() => ({
150+
getCurrentValue: () => source.getValue(),
151+
subscribe: callback => {
152+
const subscription = source.subscribe(callback);
153+
return () => subscription.unsubscribe();
154+
},
155+
}),
156+
[source],
157+
),
158+
);
159+
return <Child value={value} />;
160+
}
137161

138162
const observableA = createBehaviorSubject('a-0');
139163
const observableB = createBehaviorSubject('b-0');
@@ -162,18 +186,31 @@ describe('useSubscription', () => {
162186
it('should ignore values emitted by a new subscribable until the commit phase', () => {
163187
const log = [];
164188

165-
function Child({value}) {
166-
Scheduler.yieldValue('Child: ' + value);
189+
function Grandchild({value}) {
190+
Scheduler.yieldValue('Grandchild: ' + value);
167191
return null;
168192
}
169193

170-
const Subscription = createSubscription({
171-
getCurrentValue: source => source.getValue(),
172-
subscribe: (source, callback) => {
173-
const subscription = source.subscribe(callback);
174-
return () => subscription.unsubscribe();
175-
},
176-
});
194+
function Child({value = 'default'}) {
195+
Scheduler.yieldValue('Child: ' + value);
196+
return <Grandchild value={value} />;
197+
}
198+
199+
function Subscription({source}) {
200+
const value = useSubscription(
201+
React.useMemo(
202+
() => ({
203+
getCurrentValue: () => source.getValue(),
204+
subscribe: callback => {
205+
const subscription = source.subscribe(callback);
206+
return () => subscription.unsubscribe();
207+
},
208+
}),
209+
[source],
210+
),
211+
);
212+
return <Child value={value} />;
213+
}
177214

178215
class Parent extends React.Component {
179216
state = {};
@@ -197,14 +234,7 @@ describe('useSubscription', () => {
197234
}
198235

199236
render() {
200-
return (
201-
<Subscription source={this.state.observed}>
202-
{({value = 'default'}) => {
203-
Scheduler.yieldValue('Subscriber: ' + value);
204-
return <Child value={value} />;
205-
}}
206-
</Subscription>
207-
);
237+
return <Subscription source={this.state.observed} />;
208238
}
209239
}
210240

@@ -215,12 +245,12 @@ describe('useSubscription', () => {
215245
<Parent observed={observableA} />,
216246
{unstable_isConcurrent: true},
217247
);
218-
expect(Scheduler).toFlushAndYield(['Subscriber: a-0', 'Child: a-0']);
248+
expect(Scheduler).toFlushAndYield(['Child: a-0', 'Grandchild: a-0']);
219249
expect(log).toEqual(['Parent.componentDidMount']);
220250

221251
// Start React update, but don't finish
222252
renderer.update(<Parent observed={observableB} />);
223-
expect(Scheduler).toFlushAndYieldThrough(['Subscriber: b-0']);
253+
expect(Scheduler).toFlushAndYieldThrough(['Child: b-0']);
224254
expect(log).toEqual(['Parent.componentDidMount']);
225255

226256
// Emit some updates from the uncommitted subscribable
@@ -236,11 +266,11 @@ describe('useSubscription', () => {
236266
// But the intermediate ones should be ignored,
237267
// And the final rendered output should be the higher-priority observable.
238268
expect(Scheduler).toFlushAndYield([
239-
'Child: b-0',
240-
'Subscriber: b-3',
269+
'Grandchild: b-0',
241270
'Child: b-3',
242-
'Subscriber: a-0',
271+
'Grandchild: b-3',
243272
'Child: a-0',
273+
'Grandchild: a-0',
244274
]);
245275
expect(log).toEqual([
246276
'Parent.componentDidMount',
@@ -252,18 +282,31 @@ describe('useSubscription', () => {
252282
it('should not drop values emitted between updates', () => {
253283
const log = [];
254284

255-
function Child({value}) {
256-
Scheduler.yieldValue('Child: ' + value);
285+
function Grandchild({value}) {
286+
Scheduler.yieldValue('Grandchild: ' + value);
257287
return null;
258288
}
259289

260-
const Subscription = createSubscription({
261-
getCurrentValue: source => source.getValue(),
262-
subscribe: (source, callback) => {
263-
const subscription = source.subscribe(callback);
264-
return () => subscription.unsubscribe();
265-
},
266-
});
290+
function Child({value = 'default'}) {
291+
Scheduler.yieldValue('Child: ' + value);
292+
return <Grandchild value={value} />;
293+
}
294+
295+
function Subscription({source}) {
296+
const value = useSubscription(
297+
React.useMemo(
298+
() => ({
299+
getCurrentValue: () => source.getValue(),
300+
subscribe: callback => {
301+
const subscription = source.subscribe(callback);
302+
return () => subscription.unsubscribe();
303+
},
304+
}),
305+
[source],
306+
),
307+
);
308+
return <Child value={value} />;
309+
}
267310

268311
class Parent extends React.Component {
269312
state = {};
@@ -287,14 +330,7 @@ describe('useSubscription', () => {
287330
}
288331

289332
render() {
290-
return (
291-
<Subscription source={this.state.observed}>
292-
{({value = 'default'}) => {
293-
Scheduler.yieldValue('Subscriber: ' + value);
294-
return <Child value={value} />;
295-
}}
296-
</Subscription>
297-
);
333+
return <Subscription source={this.state.observed} />;
298334
}
299335
}
300336

@@ -305,12 +341,12 @@ describe('useSubscription', () => {
305341
<Parent observed={observableA} />,
306342
{unstable_isConcurrent: true},
307343
);
308-
expect(Scheduler).toFlushAndYield(['Subscriber: a-0', 'Child: a-0']);
344+
expect(Scheduler).toFlushAndYield(['Child: a-0', 'Grandchild: a-0']);
309345
expect(log).toEqual(['Parent.componentDidMount']);
310346

311347
// Start React update, but don't finish
312348
renderer.update(<Parent observed={observableB} />);
313-
expect(Scheduler).toFlushAndYieldThrough(['Subscriber: b-0']);
349+
expect(Scheduler).toFlushAndYieldThrough(['Child: b-0']);
314350
expect(log).toEqual(['Parent.componentDidMount']);
315351

316352
// Emit some updates from the old subscribable
@@ -324,9 +360,9 @@ describe('useSubscription', () => {
324360
// We expect the new subscribable to finish rendering,
325361
// But then the updated values from the old subscribable should be used.
326362
expect(Scheduler).toFlushAndYield([
327-
'Child: b-0',
328-
'Subscriber: a-2',
363+
'Grandchild: b-0',
329364
'Child: a-2',
365+
'Grandchild: a-2',
330366
]);
331367
expect(log).toEqual([
332368
'Parent.componentDidMount',
@@ -345,12 +381,25 @@ describe('useSubscription', () => {
345381
});
346382

347383
it('should guard against updates that happen after unmounting', () => {
348-
const Subscription = createSubscription({
349-
getCurrentValue: source => source.getValue(),
350-
subscribe: (source, callback) => {
351-
return source.subscribe(callback);
352-
},
353-
});
384+
function Child({value = 'default'}) {
385+
Scheduler.yieldValue(value);
386+
return null;
387+
}
388+
389+
function Subscription({source}) {
390+
const value = useSubscription(
391+
React.useMemo(
392+
() => ({
393+
getCurrentValue: () => source.getValue(),
394+
subscribe: callback => {
395+
return source.subscribe(callback);
396+
},
397+
}),
398+
[source],
399+
),
400+
);
401+
return <Child value={value} />;
402+
}
354403

355404
const eventHandler = {
356405
_callbacks: [],
@@ -382,12 +431,7 @@ describe('useSubscription', () => {
382431
});
383432

384433
const renderer = ReactTestRenderer.create(
385-
<Subscription source={eventHandler}>
386-
{({value}) => {
387-
Scheduler.yieldValue(value);
388-
return null;
389-
}}
390-
</Subscription>,
434+
<Subscription source={eventHandler} />,
391435
{unstable_isConcurrent: true},
392436
);
393437

0 commit comments

Comments
 (0)