Skip to content

Modify legacy registration mechanism #1375

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.

Sign up for GitHub

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

Merged
jerelmiller merged 7 commits into from
May 22, 2024
Merged

Modify legacy registration mechanism #1375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e8ce3a6
Define a property setter that will register legacy clients
jerelmiller May 17, 2024
235d46d
No need to find client on initialization anymore
jerelmiller May 17, 2024
a21334a
Add changeset
jerelmiller May 17, 2024
0b39b19
Allow undefined when setting the client and conditionally register
jerelmiller May 19, 2024
50fc96b
Wrap registerClient in a setTimeout
jerelmiller May 19, 2024
90c806d
Remove handler on load event
jerelmiller May 19, 2024
5040547
Merge branch 'main' into jerel/register-legacy
jerelmiller May 22, 2024
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/young-brooms-protect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"apollo-client-devtools": patch
---

Use `Object.defineProperty` to register legacy clients to avoid the need to search for the client in a loop in initialization.
32 changes: 24 additions & 8 deletions src/extension/tab/hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
getMutations,
getMainDefinition,
} from "./helpers";
import type { QueryResult } from "../../types";
import type { QueryResult, SafeAny } from "../../types";
import { getPrivateAccess } from "../../privateAccess";
import type { JSONObject } from "../../application/types/json";
import { createWindowActor } from "../actor";
Expand Down Expand Up @@ -91,12 +91,6 @@ window.onbeforeunload = () => {
tab.send({ type: "disconnectFromDevtools" });
};

window.addEventListener("load", () => {
if (hook.ApolloClient) {
Copy link
Copy Markdown
Member Author

@jerelmiller jerelmiller May 19, 2024

Choose a reason for hiding this comment

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

This was almost never true, but with the changes in this PR, we don't need it anyways since we will register the client immediately if its available when this content script loads, or register it when the client is assigned on the window.

sendHookDataToDevTools("connectToDevtools");
}
});

function getClientData() {
// We need to JSON stringify the data here in case the cache contains
// references to irregular data such as `URL` instances which are not
Expand Down Expand Up @@ -301,4 +295,26 @@ if (Array.isArray(preExisting)) {
(preExisting as Array<ApolloClient<any>>).forEach(registerClient);
}

findClient();
Copy link
Copy Markdown
Member Author

@jerelmiller jerelmiller May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We no longer need this on initialization since we register the client immediately if its available, or registers it when the value is assigned from a legacy client.

// Handles registering legacy clients (< v3.7) which do not use the new
// registration mechanism above.
let globalClient = window.__APOLLO_CLIENT__;
Object.defineProperty(window, "__APOLLO_CLIENT__", {
get() {
return globalClient;
},
set(client: ApolloClient<SafeAny> | undefined) {
if (client) {
// We call this in a setTimeout because the client is not fully
// instantiated before the property on window is assigned since it
// connects from the constructor of ApolloClient. This allows
// initialization to finish before we register it.
setTimeout(() => registerClient(client));
}

globalClient = client;
},
});

if (globalClient) {
registerClient(globalClient);
}