Cancel running `ObservableQuery` link on unsubscribe #12633

phryneas · 2025-05-20T12:25:33Z

Fixes #12554

changeset-bot · 2025-05-20T12:25:36Z

🦋 Changeset detected

Latest commit: 5b828d8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pkg-pr-new · 2025-05-20T12:26:49Z

npm i https://pkg.pr.new/apollographql/apollo-client/@apollo/client@12633

commit: 5b828d8

github-actions · 2025-05-20T12:27:22Z

size-limit report 📦

Path	Size
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client" (CJS)	42.23 KB (+0.48% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client" (production) (CJS)	37.33 KB (+0.32% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client"	32.49 KB (+0.67% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "@apollo/client" (production)	26.98 KB (+0.67% 🔺)
import { ApolloProvider } from "@apollo/client/react"	5.56 KB (0%)
import { ApolloProvider } from "@apollo/client/react" (production)	963 B (0%)
import { useQuery } from "@apollo/client/react"	11.59 KB (+1.49% 🔺)
import { useQuery } from "@apollo/client/react" (production)	7 KB (+2.41% 🔺)
import { useLazyQuery } from "@apollo/client/react"	6.72 KB (0%)
import { useLazyQuery } from "@apollo/client/react" (production)	2.12 KB (0%)
import { useMutation } from "@apollo/client/react"	6.1 KB (0%)
import { useMutation } from "@apollo/client/react" (production)	1.47 KB (0%)
import { useSubscription } from "@apollo/client/react"	6.46 KB (0%)
import { useSubscription } from "@apollo/client/react" (production)	1.83 KB (0%)
import { useSuspenseQuery } from "@apollo/client/react"	8.17 KB (0%)
import { useSuspenseQuery } from "@apollo/client/react" (production)	3.57 KB (0%)
import { useBackgroundQuery } from "@apollo/client/react"	7.99 KB (0%)
import { useBackgroundQuery } from "@apollo/client/react" (production)	3.4 KB (0%)
import { useLoadableQuery } from "@apollo/client/react"	7.97 KB (0%)
import { useLoadableQuery } from "@apollo/client/react" (production)	3.38 KB (0%)
import { useReadQuery } from "@apollo/client/react"	6.22 KB (0%)
import { useReadQuery } from "@apollo/client/react" (production)	1.61 KB (0%)
import { useFragment } from "@apollo/client/react"	6.29 KB (0%)
import { useFragment } from "@apollo/client/react" (production)	1.68 KB (0%)

src/utilities/internal/LazyPromise.ts

phryneas · 2025-05-21T14:22:17Z

src/core/__tests__/ApolloClient/general.test.ts

@@ -489,7 +490,8 @@ describe("ApolloClient", () => {
    expect(onRequestUnsubscribe).toHaveBeenCalledTimes(1);
  });

-  it("causes link unsubscription after multiple requests finish", async () => {
+  // restoring original test from https://github.com/apollographql/apollo-client/blob/326cbe8260b1cc04b339ed2b968e23c08a392cc4/src/core/__tests__/ApolloClient/general.test.ts#L366-L461
+  it("causes link unsubscription after reobserve", async () => {


restoring original test from

apollo-client/src/core/__tests__/ApolloClient/general.test.ts

Lines 366 to 461 in 326cbe8

it("causes link unsubscription after reobserve", async () => {

const expResult = {

data: {

allPeople: {

people: [

{

name: "Luke Skywalker",

},

],

},

},

};

const request = {

query: gql`

query people($offset: Int) {

allPeople(first: $offset) {

people {

name

}

}

}

`,

variables: undefined,

};

const mockedResponse = {

request,

result: expResult,

};

const onRequestSubscribe = jest.fn();

const onRequestUnsubscribe = jest.fn();

const mockedSingleLink = new ApolloLink(() => {

return new Observable((observer) => {

onRequestSubscribe();

// Delay (100ms) must be bigger than sum of reobserve and unsubscribe awaits (5ms each)

// to show clearly that the connection was aborted before completing

const timer = setTimeout(() => {

observer.next(mockedResponse.result);

observer.complete();

}, 100);

return () => {

onRequestUnsubscribe();

clearTimeout(timer);

};

});

});

const client = new ApolloClient({

link: mockedSingleLink,

cache: new InMemoryCache(),

defaultOptions: {

watchQuery: {

fetchPolicy: "cache-and-network",

returnPartialData: false,

notifyOnNetworkStatusChange: true,

},

query: {

fetchPolicy: "network-only",

},

},

queryDeduplication: false,

});

const observableQuery = client.watchQuery<

(typeof expResult)["data"],

{ offset?: number | undefined }

>({

query: request.query,

variables: request.variables,

});

const stream = new ObservableStream(observableQuery);

expect(onRequestSubscribe).toHaveBeenCalledTimes(1);

// This is the most important part of this test

// Check that reobserve cancels the previous connection while watchQuery remains active

void observableQuery.reobserve({ variables: { offset: 20 } });

await waitFor(() => {

// Verify that previous connection was aborted by reobserve

expect(onRequestUnsubscribe).toHaveBeenCalledTimes(1);

});

stream.unsubscribe();

await wait(10);

expect(onRequestSubscribe).toHaveBeenCalledTimes(2);

expect(onRequestUnsubscribe).toHaveBeenCalledTimes(2);

});

phryneas · 2025-05-22T12:53:07Z

src/core/__tests__/ApolloClient/general.test.ts

@@ -413,7 +414,7 @@ describe("ApolloClient", () => {
    expect(stream.unsubscribe).not.toThrow();
  });

-  it("unsubscribes from link after initial reobserve when unsubscribing immediately", async () => {
+  it("causes link unsubscription if unsubscribed", async () => {


Restoring pre-rxjs test
https://github.com/apollographql/apollo-client/blob/9ec9584b20e2aaf453974c6327448a1ef1b802f0/src/core/__tests__/ApolloClient/general.test.ts#L297C1-L368C6

src/core/ObservableQuery.ts

https://github.com/apollographql/apollo-client/blob/9ec9584b20e2aaf453974c6327448a1ef1b802f0/src/core/__tests__/ApolloClient/general.test.ts#L297C1-L368C6

…vate method

phryneas · 2025-05-23T09:10:15Z

src/core/ObservableQuery.ts

@@ -80,7 +81,6 @@ interface TrackedOperation {
  variables: OperationVariables;
 }

-const newNetworkStatusSymbol: any = Symbol();


The changes around newNetworkStatusSymbol actually wouldn't be necessary after all, since I didn't have to add a second internal option.
On the other hand, I believe it's an improvement to type safety and readability, so I'd suggest to keep it in anyways.

phryneas · 2025-05-23T09:13:24Z

src/core/ObservableQuery.ts

@@ -1708,11 +1743,11 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
  // this.options.fetchPolicy is "cache-and-network" or "network-only". When
  // this.options.fetchPolicy is any other policy ("cache-first", "cache-only",
  // "standby", or "no-cache"), we call this.reobserve() as usual.
-  private reobserveCacheFirst() {
+  private reobserveCacheFirst(): void {


The return value was never used, so I changed this to void

jerelmiller

This looks really good! If you're down with changing useLazyQuery to default to the non-retain version, I'll approve once those changes are made.

.changeset/twelve-mangos-dance.md

src/core/ObservableQuery.ts

.changeset/twelve-mangos-dance.md

src/core/__tests__/ApolloClient/general.test.ts

jerelmiller · 2025-05-27T20:27:33Z

src/react/hooks/useLazyQuery.ts

+            // ensuring the key exists in options
+            variables: executeOptions?.variables,
+          })
+          .retain(/* create a persistent subscription on the query */);


I still think it makes sense to remove this by default and give control over to the user. We didn't have this type of control/mechanism before and even using AbortController in 3.x was problematic since changes to context would mean calling reobserve more often than it should have (i.e. #11835).

Now that we have a really elegant solution here, I think having consistency throughout the client is useful and its a single line of code to restore this behavior in app code. Doing so also means someone won't have to resort to AbortController to achieve this behavior which is going to be a lot more code.

One more pro is that retain makes it more obvious that the promise will be kept around rather than being buried as an implementation detail.

src/core/ObservableQuery.ts

Co-authored-by: Jerel Miller <[email protected]>

…xpose it instead

jerelmiller

The code generally looks good but I think we'll need to dig a bit deeper to change the value resolves/rejected from the promise when the in-flight query is aborted, otherwise it just resolves with data: undefined and that might be surprising.

The useLazyQuery tests fail for this reason so we'll need to revisit those. I can confirm though calling retain in those tests does work as expected so thats great news!

…not retained and not required anymore

phryneas · 2025-06-02T13:15:02Z

@jerelmiller what do you think about 60180f5 (#12633) ?

jerelmiller

Looks great! Thank you!

…uery-cancel-on-unsubscribe

phryneas commented May 20, 2025

View reviewed changes

src/utilities/internal/LazyPromise.ts Outdated Show resolved Hide resolved

phryneas mentioned this pull request May 20, 2025

Extract localState to own module and require opt-in. Add additional improvements #12614

Merged

phryneas closed this May 21, 2025

phryneas force-pushed the pr/observablequery-cancel-on-unsubscribe branch from 988eb04 to 7fe6af1 Compare May 21, 2025 14:09

phryneas reopened this May 21, 2025

phryneas commented May 21, 2025

View reviewed changes

Base automatically changed from pr/rewrite-observable-query-subscription-logic to release-4.0 May 21, 2025 16:35

phryneas force-pushed the pr/observablequery-cancel-on-unsubscribe branch from 5dcdb67 to bda126f Compare May 22, 2025 09:43

phryneas changed the base branch from release-4.0 to pr/prevent-standby-loadings-state May 22, 2025 10:56

phryneas commented May 22, 2025

View reviewed changes

src/core/ObservableQuery.ts Outdated Show resolved Hide resolved

phryneas added 13 commits May 23, 2025 10:50

cancel running ObservableQuery link on unsubscribe

8e8928c

adjust toStringTag

8f57b7c

remove comment

5230ce8

WIP

8580d2f

isolate all situations where subscriptions can be created

a8e4db8

tap for polling to avoid a permanent subscription

969a495

restore old test

b484906

https://github.com/apollographql/apollo-client/blob/9ec9584b20e2aaf453974c6327448a1ef1b802f0/src/core/__tests__/ApolloClient/general.test.ts#L297C1-L368C6

remove unused import

760c188

chores

faef9b6

warn about lazy-eager conversion on return from async

62fa2d7

switch to a promise with a retain property instead

82e0f1e

move reobserve signature with internal argument to _reobserve pri…

68ec7b2

…vate method

chores

44b8378

phryneas force-pushed the pr/observablequery-cancel-on-unsubscribe branch from 16b2610 to 44b8378 Compare May 23, 2025 08:50

undo a few changes that are not neccessary any more

fbe39f5

phryneas changed the base branch from pr/prevent-standby-loadings-state to release-4.0 May 23, 2025 08:56

phryneas added 3 commits May 23, 2025 11:03

chores

5ec614c

more chores

89e0641

changeset

67a93b3

phryneas commented May 23, 2025

View reviewed changes

minor optimization

47d6e84

phryneas force-pushed the pr/observablequery-cancel-on-unsubscribe branch from ad068f6 to 47d6e84 Compare May 23, 2025 09:22

chores

2300437

phryneas marked this pull request as ready for review May 23, 2025 09:30

phryneas changed the title ~~[WIP] cancel running ObservableQuery link on unsubscribe~~ Cancel running ObservableQuery link on unsubscribe May 23, 2025

phryneas requested a review from jerelmiller May 23, 2025 09:30

jerelmiller reviewed May 27, 2025

View reviewed changes

phryneas and others added 4 commits May 30, 2025 13:48

Update .changeset/twelve-mangos-dance.md

1dab306

Co-authored-by: Jerel Miller <[email protected]>

Apply suggestions from code review

ee4660e

Co-authored-by: Jerel Miller <[email protected]>

changes from code review

6259eb3

do not autocall .retain in the useLazyQuery execute function, e…

ea36754

…xpose it instead

phryneas requested a review from jerelmiller May 30, 2025 12:06

jerelmiller requested changes May 30, 2025

View reviewed changes

throw AbortError from ObservableQuery.reobserve promise if it is …

60180f5

…not retained and not required anymore

run format

9c481ca

jerelmiller approved these changes Jun 2, 2025

View reviewed changes

phryneas added 2 commits June 3, 2025 09:58

Merge branch 'release-4.0' into pr/observablequery-cancel-on-unsubscribe

443267b

Merge remote-tracking branch 'origin/release-4.0' into pr/observableq…

5b828d8

…uery-cancel-on-unsubscribe

phryneas merged commit 9bfb51f into release-4.0 Jun 3, 2025
42 of 43 checks passed

github-actions bot mentioned this pull request Jun 3, 2025

Version Packages (alpha) #12669

Merged

	it("causes link unsubscription after reobserve", async () => {
	const expResult = {
	data: {
	allPeople: {
	people: [
	{
	name: "Luke Skywalker",
	},
	],
	},
	},
	};

	const request = {
	query: gql`
	query people($offset: Int) {
	allPeople(first: $offset) {
	people {
	name
	}
	}
	}
	`,
	variables: undefined,
	};

	const mockedResponse = {
	request,
	result: expResult,
	};

	const onRequestSubscribe = jest.fn();
	const onRequestUnsubscribe = jest.fn();

	const mockedSingleLink = new ApolloLink(() => {
	return new Observable((observer) => {
	onRequestSubscribe();

	// Delay (100ms) must be bigger than sum of reobserve and unsubscribe awaits (5ms each)
	// to show clearly that the connection was aborted before completing
	const timer = setTimeout(() => {
	observer.next(mockedResponse.result);
	observer.complete();
	}, 100);

	return () => {
	onRequestUnsubscribe();
	clearTimeout(timer);
	};
	});
	});

	const client = new ApolloClient({
	link: mockedSingleLink,
	cache: new InMemoryCache(),
	defaultOptions: {
	watchQuery: {
	fetchPolicy: "cache-and-network",
	returnPartialData: false,
	notifyOnNetworkStatusChange: true,
	},
	query: {
	fetchPolicy: "network-only",
	},
	},
	queryDeduplication: false,
	});

	const observableQuery = client.watchQuery<
	(typeof expResult)["data"],
	{ offset?: number \| undefined }
	>({
	query: request.query,
	variables: request.variables,
	});

	const stream = new ObservableStream(observableQuery);

	expect(onRequestSubscribe).toHaveBeenCalledTimes(1);

	// This is the most important part of this test
	// Check that reobserve cancels the previous connection while watchQuery remains active
	void observableQuery.reobserve({ variables: { offset: 20 } });

	await waitFor(() => {
	// Verify that previous connection was aborted by reobserve
	expect(onRequestUnsubscribe).toHaveBeenCalledTimes(1);
	});

	stream.unsubscribe();

	await wait(10);

	expect(onRequestSubscribe).toHaveBeenCalledTimes(2);
	expect(onRequestUnsubscribe).toHaveBeenCalledTimes(2);
	});

Cancel running ObservableQuery link on unsubscribe #12633

Cancel running ObservableQuery link on unsubscribe #12633

Uh oh!

Conversation

phryneas commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

pkg-pr-new bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Uh oh!

phryneas May 21, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phryneas May 23, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas May 23, 2025

Choose a reason for hiding this comment

Uh oh!

jerelmiller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jerelmiller May 27, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas May 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jerelmiller left a comment

Choose a reason for hiding this comment

Uh oh!

phryneas commented Jun 2, 2025

Uh oh!

jerelmiller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Cancel running `ObservableQuery` link on unsubscribe #12633

Cancel running `ObservableQuery` link on unsubscribe #12633

phryneas commented May 20, 2025 •

edited

Loading

changeset-bot bot commented May 20, 2025 •

edited

Loading

pkg-pr-new bot commented May 20, 2025 •

edited

Loading

github-actions bot commented May 20, 2025 •

edited

Loading