Unsubscribe network requests when releasing temporary retains

Reviewed By: jstejada

Differential Revision: D23277912

fbshipit-source-id: 4924acbb049ff6579448fa6322af49c0fa5cfe41

Author: tyao1

packages/relay-experimental/QueryResource.js

@@ -182,6 +182,12 @@ function createCacheEntry(
         releaseQueryTimeout = null;
         releaseTemporaryRetain = null;
         disposable.dispose();
+        // Normally if this entry never commits, the request would've ended by the
+        // time this timeout expires and the temporary retain is released. However,
+        // we need to do this for live queries which remain open indefinitely.
+        if (retainCount <= 0 && currentNetworkSubscription != null) {
+          currentNetworkSubscription.unsubscribe();
+        }
       };
       releaseQueryTimeout = setTimeout(
         localReleaseTemporaryRetain,

packages/relay-experimental/__tests__/useLazyLoadQueryNode-test.js

@@ -442,9 +442,9 @@ describe('useLazyLoadQueryNode', () => {
     expect(release).toBeCalledTimes(2);
 
     // Assert request in flight is cancelled
-    expect(environment.mock.isLoading(query.request.node, variables)).toEqual(
-      false,
-    );
+    expect(
+      environment.mock.isLoading(query.request.node, variables, {force: true}),
+    ).toEqual(false);
   });
 
   it('disposes ongoing network request when component unmounts after committing', () => {
@@ -477,9 +477,40 @@ describe('useLazyLoadQueryNode', () => {
     // Assert data is released
     expect(release).toBeCalledTimes(1);
     // Assert request in flight is cancelled
-    expect(environment.mock.isLoading(query.request.node, variables)).toEqual(
-      false,
-    );
+    expect(
+      environment.mock.isLoading(query.request.node, variables, {force: true}),
+    ).toEqual(false);
+  });
+
+  it('cancels network request when temporarily retained component that never commits is disposed of after timeout', () => {
+    const instance = render(environment, <Container variables={variables} />);
+
+    expect(instance.toJSON()).toEqual('Fallback');
+    expectToHaveFetched(environment, query);
+    expect(renderFn).not.toBeCalled();
+    expect(environment.retain).toHaveBeenCalledTimes(1);
+    ReactTestRenderer.act(() => {
+      instance.unmount();
+    });
+    // Resolve a payload but don't complete the network request
+    environment.mock.nextValue(gqlQuery, {
+      data: {
+        node: {
+          __typename: 'User',
+          id: variables.id,
+          name: 'Alice',
+        },
+      },
+    });
+
+    // Trigger releasing of the temporary retain
+    jest.runAllTimers();
+    // Assert data is released
+    expect(release).toBeCalledTimes(1);
+    // Assert request in flight is cancelled
+    expect(
+      environment.mock.isLoading(query.request.node, variables, {force: true}),
+    ).toEqual(false);
   });
 
   describe('partial rendering', () => {

packages/relay-experimental/__tests__/useRefetchableFragmentNode-test.js

@@ -3023,7 +3023,7 @@ describe('useRefetchableFragmentNode', () => {
         });
 
         // Assert request was canceled
-        expect(unsubscribe).toBeCalledTimes(1);
+        expect(unsubscribe).toBeCalledTimes(2);
         expectRequestIsInFlight({
           inFlight: false,
           requestCount: 1,