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

Rework reconnect logic, add "Port could not be opened" modal #1513

Merged
merged 23 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 17 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
2 changes: 1 addition & 1 deletion .semgrepignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
src/extension/vscode/client.ts
src/application/components/ClientNotFoundModal.vscode.tsx
src/application/components/Modals/ClientNotFoundModal.vscode.tsx
4 changes: 4 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ export default {
testEnvironment: "jsdom",
globals: {
VERSION: "0.0.0",
__IS_FIREFOX__: false,
__IS_CHROME__: true,
__IS_VSCODE__: false,
__IS_EXTENSION__: true,
},
testPathIgnorePatterns: [
"<rootDir>/build",
Expand Down
10 changes: 0 additions & 10 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 14 additions & 40 deletions src/application/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { useEffect, useState } from "react";
import type { ReactNode } from "react";
import type { TypedDocumentNode } from "@apollo/client";
import { useReactiveVar, gql, useQuery } from "@apollo/client";
import { useMachine } from "@xstate/react";
import { ErrorBoundary } from "react-error-boundary";

import { currentScreen, Screens } from "./components/Layouts/Navigation";
Expand All @@ -23,8 +22,8 @@ import IconGitHubSolid from "@apollo/icons/small/IconGitHubSolid.svg";
import { SettingsModal } from "./components/Layouts/SettingsModal";
import Logo from "@apollo/icons/logos/LogoSymbol.svg";
import { BannerAlert } from "./components/BannerAlert";
import { devtoolsMachine } from "./machines/devtoolsMachine";
import { ClientNotFoundModal } from "./components/ClientNotFoundModal";
import { useDevToolsActorRef } from "./machines/devtoolsMachine";
import { Modals } from "./components/Modals/Modals";
import { ButtonGroup } from "./components/ButtonGroup";
import {
GitHubIssueLink,
Expand All @@ -41,6 +40,7 @@ import { useActorEvent } from "./hooks/useActorEvent";
import { removeClient } from ".";
import { PageError } from "./components/PageError";
import { SidebarLayout } from "./components/Layouts/SidebarLayout";
import { ExternalLink } from "./components/ExternalLink";

const APP_QUERY: TypedDocumentNode<AppQuery, AppQueryVariables> = gql`
query AppQuery {
Expand Down Expand Up @@ -74,26 +74,13 @@ ${SECTIONS.devtoolsVersion}
`;

const stableEmptyClients: Required<AppQuery["clients"]> = [];
const noop = () => {};

export const App = () => {
const [snapshot, send] = useMachine(
devtoolsMachine.provide({
actions: {
resetStore: () => {
apolloClient.clearStore().catch(noop);
},
},
})
);
const {
data,
client: apolloClient,
refetch,
} = useQuery(APP_QUERY, { errorPolicy: "all" });
const { send } = useDevToolsActorRef();
const { data, refetch } = useQuery(APP_QUERY, { errorPolicy: "all" });

useActorEvent("registerClient", () => {
send({ type: "connect" });
send({ type: "client.register" });
// Unfortunately after we clear the store above, the query ends up "stuck"
// holding onto the old list of clients even if we manually write a cache
// update to properly resolve the list. Instead we refetch the list again to
Expand All @@ -102,17 +89,12 @@ export const App = () => {
});

useActorEvent("clientTerminated", (message) => {
// Disconnect if we are terminating the last client. We assume that 1 client
// means we are terminating the selected client
if (clients.length === 1) {
send({ type: "disconnect" });
}

send({ type: "client.terminated" });
removeClient(message.clientId);
});

useActorEvent("pageNavigated", () => {
send({ type: "disconnect" });
send({ type: "client.setCount", count: 0 });
});

const [settingsOpen, setSettingsOpen] = useState(false);
Expand Down Expand Up @@ -149,31 +131,25 @@ export const App = () => {

useEffect(() => {
if (clients.length) {
send({ type: "connect" });
send({ type: "client.setCount", count: clients.length });
}
}, [send, clients.length]);

return (
<>
<SettingsModal open={settingsOpen} onOpen={setSettingsOpen} />
<ClientNotFoundModal
open={snapshot.context.modalOpen}
onClose={() => send({ type: "closeModal" })}
onRetry={() => send({ type: "retry" })}
/>
<Modals />
Copy link
Member Author

Choose a reason for hiding this comment

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

All modals live in that file now.

Copy link
Member

Choose a reason for hiding this comment

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

Can we name this something like ErrorModals? Ideally we have some namespace that describes that these are the modals opened when the clients can't connect in some way.

Otherwise it looks a little bit weird to have SettingsModal right next to Modals 😂.

<BannerAlert />
<Tabs
value={selected}
onChange={(screen) => currentScreen(screen)}
className="flex flex-col h-screen bg-primary dark:bg-primary-dark"
>
<div className="flex items-center border-b border-b-primary dark:border-b-primary-dark gap-4 px-4">
<a
<ExternalLink
Copy link
Member Author

Choose a reason for hiding this comment

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

<a tags in VSCode cannot open new windows, so I need a component for that. The base implementation really just renders an a tag, and we have a .vscode.tsx version of that.

href="https://go.apollo.dev/c/docs"
target="_blank"
title="Apollo Client developer documentation"
className="block"
rel="noreferrer"
>
<Logo
role="img"
Expand All @@ -182,7 +158,7 @@ export const App = () => {
fill="currentColor"
className="text-icon-primary dark:text-icon-primary-dark"
/>
</a>
</ExternalLink>
<Divider orientation="vertical" />
<Tabs.List className="-mb-px">
<Tabs.Trigger value={Screens.Queries}>
Expand All @@ -197,21 +173,19 @@ export const App = () => {
<div className="ml-auto flex-1 justify-end flex items-center gap-2 h-full">
{client?.version && (
<GitHubReleaseHoverCard version={client.version}>
<a
<ExternalLink
className="no-underline"
href={
isSnapshotRelease(client.version)
? `https://github.com/apollographql/apollo-client/pull/${parseSnapshotRelease(client.version).prNumber}`
: `https://github.com/apollographql/apollo-client/releases/tag/v${client.version}`
}
target="_blank"
rel="noreferrer"
>
<Badge variant="info" className="cursor-pointer">
Apollo Client <span className="lowercase">v</span>
{client.version}
</Badge>
</a>
</ExternalLink>
</GitHubReleaseHoverCard>
)}
{clients.length > 1 && (
Expand Down
4 changes: 3 additions & 1 deletion src/application/__tests__/AppProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { render, waitFor, screen } from "@testing-library/react";
import matchMediaMock from "../utilities/testing/matchMedia";
import { Mode, colorTheme } from "../theme";
import { AppProvider } from "../index";
import { devtoolsMachine } from "../machines/devtoolsMachine";
import { createActor } from "xstate";

const matchMedia = matchMediaMock();

Expand All @@ -12,7 +14,7 @@ jest.mock("../App", () => ({

describe("<AppProvider />", () => {
test("changes the color theme", async () => {
render(<AppProvider />);
render(<AppProvider actor={createActor(devtoolsMachine).start()} />);
expect(screen.getByText("App")).toBeInTheDocument();

expect(colorTheme()).toEqual("light");
Expand Down
2 changes: 1 addition & 1 deletion src/application/components/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Spinner } from "./Spinner";

type NativeButtonProps = ComponentPropsWithoutRef<"button">;

type ButtonProps = AsChildProps<NativeButtonProps> &
export type ButtonProps = AsChildProps<NativeButtonProps> &
Variants & {
className?: string;
icon?: ReactElement<{ "aria-hidden": boolean; className?: string }>;
Expand Down
8 changes: 8 additions & 0 deletions src/application/components/ExternalLink.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { type ComponentProps, forwardRef } from "react";

export const ExternalLink = forwardRef<
HTMLAnchorElement,
ComponentProps<"a"> & { href: string }
>((props, ref) => {
return <a target="_blank" rel="noopener noreferrer" {...props} ref={ref} />;
});
39 changes: 39 additions & 0 deletions src/application/components/ExternalLink.vscode.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { type ComponentProps, forwardRef } from "react";
import { getPanelActor } from "../../extension/devtools/panelActor";
import { expectTypeOf } from "expect-type";

expectTypeOf<typeof import("./ExternalLink.jsx")>().toMatchTypeOf<
typeof import("./ExternalLink.vscode.jsx")
>();

/**
* A link that opens links using a vscode command.
*
* Required because sometimes VSCode blocks links from being opened.:
*
* > Blocked opening 'https://...' in a new window because the request was made in a sandboxed frame whose 'allow-popups' permission is not set.
*/
export const ExternalLink = forwardRef<
HTMLAnchorElement,
ComponentProps<"a"> & { href: string }
>((props, ref) => {
return (
<a
{...props}
ref={ref}
onClick={(e) => {
if (props.onClick) {
props.onClick(e);
}
if (e.isDefaultPrevented()) return;

getPanelActor(window).send({
type: "vscode:openExternal",
uri: props.href,
});

e.nativeEvent.preventDefault();
}}
/>
);
});
9 changes: 4 additions & 5 deletions src/application/components/GitHubIssueLink.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ReactNode } from "react";
import { forwardRef } from "react";
import { ExternalLink } from "./ExternalLink";

declare const VERSION: string;

Expand All @@ -21,7 +22,7 @@ export const SECTIONS = {
<!-- Please provide the version of \`@apollo/client\` you are using. -->
`,
devtoolsVersion: `### Apollo Client Devtools version
${VERSION}
${VERSION} (${__IS_FIREFOX__ ? "Firefox" : __IS_VSCODE__ ? "VSCode" : "Chrome"})
Copy link
Member Author

Choose a reason for hiding this comment

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

Some additional info can be very helpful here to distinguish the different builds in bug reports.

`,
reproduction: `### Link to Reproduction
<!-- Please provide a link to the reproduction of the issue. -->
Expand Down Expand Up @@ -58,15 +59,13 @@ export const GitHubIssueLink = forwardRef<
}

return (
<a
<ExternalLink
{...props}
ref={ref}
className={className}
rel="noreferrer noopener"
target="_blank"
href={`https://github.com/apollographql/${repository}/issues/new?${params}`}
>
{children}
</a>
</ExternalLink>
);
});
27 changes: 11 additions & 16 deletions src/application/components/GitHubReleaseHoverCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from "../utilities/github";
import { useGitHubApi } from "../hooks/useGitHubAPI";
import { StatusBadge } from "./StatusBadge";
import { ExternalLink } from "./ExternalLink";

interface GitHubReleaseHoverCardProps {
children?: ReactNode;
Expand Down Expand Up @@ -68,27 +69,25 @@ function SnapshotCardContents({ version }: { version: SnapshotVersion }) {
<div className="flex gap-1 items-center text-xs font-bold uppercase text-secondary dark:text-secondary-dark">
Published {formatDate(parseSnapshotTimestamp(release.timestamp))}
</div>
<a
<ExternalLink
className="flex gap-1 items-center mt-2"
href={`https://github.com/apollographql/apollo-client/pull/${release.prNumber}`}
target="_blank"
rel="noreferrer"
>
View pull request in GitHub <IconOutlink className="size-3" />
</a>
</ExternalLink>
</>
)}
</header>

<section>
<div className="mb-2">
<a href={pullRequest.user.html_url} target="_blank" rel="noreferrer">
<ExternalLink href={pullRequest.user.html_url}>
@{pullRequest.user.login}
</a>{" "}
</ExternalLink>{" "}
opened{" "}
<a href={pullRequest.html_url} target="_blank" rel="noreferrer">
<ExternalLink href={pullRequest.html_url}>
#{pullRequest.number}
</a>{" "}
</ExternalLink>{" "}
on {formatDate(Date.parse(pullRequest.created_at))}:
</div>
<h2 className="text-lg text-heading dark:text-heading-dark font-medium mb-2">
Expand All @@ -103,13 +102,11 @@ function SnapshotCardContents({ version }: { version: SnapshotVersion }) {
icon={<IconBranch />}
>
Merged {formatDate(Date.parse(pullRequest.merged_at))} into{" "}
<a
<ExternalLink
href={`https://github.com/apollographql/apollo-client/commit/${pullRequest.merge_commit_sha}`}
target="_blank"
rel="noreferrer"
>
{pullRequest.merge_commit_sha.slice(0, 7)}
</a>
</ExternalLink>
</StatusBadge>
) : pullRequest.state === "closed" ? (
<StatusBadge
Expand Down Expand Up @@ -178,14 +175,12 @@ function ReleaseCardContents({ version }: { version: string }) {
<div className="flex gap-1 items-center text-xs font-bold uppercase text-secondary dark:text-secondary-dark">
Published {formatDate(Date.parse(release.published_at))}
</div>
<a
<ExternalLink
className="flex gap-1 items-center mt-2"
href={`https://github.com/apollographql/apollo-client/releases/tag/v${version}`}
target="_blank"
rel="noreferrer"
>
View release in GitHub <IconOutlink className="size-3" />
</a>
</ExternalLink>
</header>
<Markdown>{release.body}</Markdown>
</div>
Expand Down
7 changes: 3 additions & 4 deletions src/application/components/Layouts/SettingsModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Button } from "../Button";
import { ExternalLink } from "../ExternalLink";
import { Modal } from "../Modal";

declare const VERSION: string;
Expand All @@ -17,14 +18,12 @@ export function SettingsModal({
</Modal.Header>
<Modal.Body>
Devtools version:{" "}
<a
<ExternalLink
className="font-code"
target="_blank"
rel="noopener noreferrer"
href={`https://github.com/apollographql/apollo-client-devtools/releases/tag/v${VERSION}`}
>
{VERSION}
</a>
</ExternalLink>
</Modal.Body>
<Modal.Footer>
<Button variant="primary" size="md" onClick={() => onOpen(false)}>
Expand Down
Loading
Loading