Skip to content

Commit b3ecce2

Browse files
tyao1facebook-github-bot
authored andcommittedJul 27, 2020
holdGC for incremental responses in QueryExecutor
Reviewed By: josephsavona Differential Revision: D22714580 fbshipit-source-id: a0dc921d025e1d65260aa1e5416f0f85af1e5eca
1 parent 8a17ff0 commit b3ecce2

File tree

2 files changed

+140
-11
lines changed

2 files changed

+140
-11
lines changed
 

‎packages/relay-runtime/store/RelayModernQueryExecutor.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ import type {
5757
NormalizationSplitOperation,
5858
NormalizationSelectableNode,
5959
} from '../util/NormalizationNode';
60-
import type {DataID, Variables} from '../util/RelayRuntimeTypes';
60+
import type {DataID, Variables, Disposable} from '../util/RelayRuntimeTypes';
6161
import type {GetDataID} from './RelayResponseNormalizer';
6262
import type {NormalizationOptions} from './RelayResponseNormalizer';
6363

@@ -136,6 +136,7 @@ class Executor {
136136
_store: Store;
137137
_subscriptions: Map<number, Subscription>;
138138
_updater: ?SelectorStoreUpdater;
139+
_retainDisposable: ?Disposable;
139140
+_isClientPayload: boolean;
140141

141142
constructor({
@@ -223,6 +224,10 @@ class Executor {
223224
}
224225
this._incrementalResults.clear();
225226
this._completeOperationTracker();
227+
if (this._retainDisposable) {
228+
this._retainDisposable.dispose();
229+
this._retainDisposable = null;
230+
}
226231
}
227232

228233
_updateActiveState(): void {
@@ -436,6 +441,9 @@ class Executor {
436441
const updatedOwners = this._publishQueue.run(this._operation);
437442
this._updateOperationTracker(updatedOwners);
438443
this._processPayloadFollowups(payloadFollowups);
444+
if (this._incrementalPayloadsPending && !this._retainDisposable) {
445+
this._retainDisposable = this._store.retain(this._operation);
446+
}
439447
}
440448

441449
if (incrementalResponses.length > 0) {

‎packages/relay-runtime/store/__tests__/RelayModernEnvironment-ExecuteWithStreamedConnection-test.js

+131-10
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,6 @@ describe('execute() fetches a @stream-ed @connection', () => {
8383
}
8484
}
8585
}
86-
87-
fragment FeedEdgeFragment on NewsFeedEdge {
88-
cursor
89-
node {
90-
id
91-
feedback {
92-
id
93-
}
94-
}
95-
}
9686
`));
9787
variables = {enableStream: true, after: null};
9888
operation = createOperationDescriptor(query, variables);
@@ -1268,4 +1258,135 @@ describe('execute() fetches a @stream-ed @connection', () => {
12681258
},
12691259
});
12701260
});
1261+
1262+
it('does not garbage collect the server connection when a pagination query is in flight', () => {
1263+
environment.retain(operation);
1264+
const initialSnapshot = environment.lookup(selector);
1265+
callback = jest.fn();
1266+
environment.subscribe(initialSnapshot, callback);
1267+
1268+
// populate the first "page" of results (one item) using the query
1269+
// with streaming disabled
1270+
variables = {enableStream: false, after: null};
1271+
operation = createOperationDescriptor(query, variables);
1272+
environment.execute({operation}).subscribe({});
1273+
dataSource.next({
1274+
data: {
1275+
viewer: {
1276+
newsFeed: {
1277+
edges: [
1278+
{
1279+
cursor: 'cursor-1',
1280+
node: {
1281+
__typename: 'Story',
1282+
id: '1',
1283+
feedback: {
1284+
id: 'feedback-1',
1285+
actors: [
1286+
{
1287+
id: 'actor-1',
1288+
__typename: 'User',
1289+
name: 'Alice',
1290+
},
1291+
],
1292+
},
1293+
},
1294+
},
1295+
],
1296+
pageInfo: {
1297+
hasNextPage: true,
1298+
endCursor: 'cursor-1',
1299+
},
1300+
},
1301+
},
1302+
},
1303+
});
1304+
dataSource.complete();
1305+
jest.runAllTimers();
1306+
next.mockClear();
1307+
callback.mockClear();
1308+
1309+
// Start pagination query
1310+
const paginationOperation = createOperationDescriptor(query, {
1311+
enableStream: true,
1312+
after: 'cursor-1',
1313+
});
1314+
environment.execute({operation: paginationOperation}).subscribe(callbacks);
1315+
dataSource.next({
1316+
data: {
1317+
viewer: {
1318+
newsFeed: {
1319+
edges: [],
1320+
},
1321+
},
1322+
},
1323+
});
1324+
jest.runAllTimers();
1325+
next.mockClear();
1326+
callback.mockClear();
1327+
1328+
// Triggers a GC
1329+
store.__gc();
1330+
jest.runAllTimers();
1331+
1332+
// Second edge should be appended correctly
1333+
dataSource.next({
1334+
data: {
1335+
cursor: 'cursor-2',
1336+
node: {
1337+
__typename: 'Story',
1338+
id: '2',
1339+
feedback: {
1340+
id: 'feedback-2',
1341+
actors: [
1342+
{
1343+
id: 'actor-2',
1344+
__typename: 'User',
1345+
name: 'Bob',
1346+
},
1347+
],
1348+
},
1349+
},
1350+
},
1351+
label: 'FeedFragment$stream$RelayModernEnvironment_newsFeed',
1352+
path: ['viewer', 'newsFeed', 'edges', 0],
1353+
});
1354+
expect(error.mock.calls.map(call => call[0].stack)).toEqual([]);
1355+
expect(next).toBeCalledTimes(1);
1356+
expect(callback).toBeCalledTimes(1);
1357+
const snapshot = callback.mock.calls[0][0];
1358+
expect(snapshot.isMissingData).toBe(false);
1359+
expect(snapshot.data).toEqual({
1360+
newsFeed: {
1361+
edges: [
1362+
{
1363+
cursor: 'cursor-1',
1364+
node: {
1365+
__typename: 'Story',
1366+
id: '1',
1367+
feedback: {
1368+
id: 'feedback-1',
1369+
actors: [{id: 'actor-1', name: 'ALICE'}],
1370+
},
1371+
},
1372+
},
1373+
{
1374+
cursor: 'cursor-2',
1375+
node: {
1376+
__typename: 'Story',
1377+
id: '2',
1378+
feedback: {
1379+
id: 'feedback-2',
1380+
actors: [{id: 'actor-2', name: 'BOB'}],
1381+
},
1382+
},
1383+
},
1384+
],
1385+
pageInfo: {
1386+
endCursor: 'cursor-1',
1387+
hasNextPage: true,
1388+
},
1389+
},
1390+
});
1391+
});
12711392
});

0 commit comments

Comments
 (0)
Please sign in to comment.