Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up subscriptions using requestIdleCallback instead of setTimeout #11228

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brown-cats-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Clean up subscriptions using requestIdleCallback instead of setTimeout
7 changes: 4 additions & 3 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
fixObservableSubclass,
getQueryDefinition,
} from "../utilities/index.js";
import { requestIdleCallback } from "../utilities/common/requestIdleCallback.js";
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I polyfilled this for compatibility with other environments (including Jest…), but exposing it via the public API for utilities seemed like overkill. Let me know if this feels like the wrong place and I'm happy to move things around.

import type { ApolloError } from "../errors/index.js";
import type { QueryManager } from "./QueryManager.js";
import type {
Expand Down Expand Up @@ -204,7 +205,7 @@ export class ObservableQuery<
//
// We do this in order to prevent observers piling up within
// the QueryManager. Notice that we only fully unsubscribe
// from the subscription in a setTimeout(..., 0) call. This call can
// from the subscription in an idle callback. This callback can
// actually be handled by the browser at a much later time. If queries
// are fired in the meantime, observers that should have been removed
// from the QueryManager will continue to fire, causing an unnecessary
Expand All @@ -214,9 +215,9 @@ export class ObservableQuery<
this.queryManager.removeQuery(this.queryId);
}

setTimeout(() => {
requestIdleCallback(() => {
subscription.unsubscribe();
}, 0);
});
},
error: reject,
};
Expand Down
39 changes: 39 additions & 0 deletions src/utilities/common/__tests__/requestIdleCallback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { requestIdleCallback } from "../requestIdleCallback";

describe("requestIdleCallback", () => {
const originalRequestIdleCallback = window.requestIdleCallback;

afterAll(() => {
Object.defineProperty(window, "requestIdleCallback", {
value: originalRequestIdleCallback,
});
});

it("should use the window method when possible", () => {
Object.defineProperty(window, "requestIdleCallback", {
value: jest.fn((callback) => callback()),
configurable: true,
});

const task = jest.fn();
requestIdleCallback(task);
expect(task).toHaveBeenCalled();
});

it("should fall back to setTimeout when the window method is not available", () => {
// @ts-expect-error
delete window.requestIdleCallback;

jest.useFakeTimers();

const task = jest.fn();
requestIdleCallback(task);

expect(task).not.toHaveBeenCalled();

jest.runAllTimers();
expect(task).toHaveBeenCalled();

jest.useRealTimers();
});
});
15 changes: 15 additions & 0 deletions src/utilities/common/requestIdleCallback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Light polyfill for requestIdleCallback when used in non-browser environments.
*/
export function requestIdleCallback(
callback: () => void,
options?: IdleRequestOptions
) {
if (
!Object.prototype.hasOwnProperty.call(window, "requestIdleCallback") ||
typeof window.requestIdleCallback === "undefined"
) {
return setTimeout(callback, options?.timeout ?? 0);
}
return window.requestIdleCallback(callback, options);
}