Skip to content
Merged
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
30 changes: 18 additions & 12 deletions apps/app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { AppLayout } from "./components/layout/AppLayout";
import { AuthCallbackView } from "./views/AuthCallbackView";
import { QuickCreateProjectProvider } from "./hooks/useQuickCreateProject";
import { RouteNavigationProvider } from "./components/ui/app-route-anchor";
import { AppNavigationUrlHost } from "./lib/url-open-routing";
import { AppFileExternalNavigationHost } from "./components/plugin/AppFileExternalNavigationHost";
import { useAppTheme } from "./hooks/useAppTheme";
import { useFaviconColorSync } from "./lib/favicon-color-preference";
import { useDesktopThemeSync } from "./hooks/useDesktopThemeSync";
Expand Down Expand Up @@ -384,18 +386,22 @@ export function App() {
<QuickCreateProjectProvider>
<AppCommandProvider>
<RouteNavigationProvider>
<HashNavigationScroll />
<Routes>
<Route
path={AUTH_CALLBACK_ROUTE_PATH}
element={<AuthCallbackView />}
/>
<Route path="*" element={<AppRoutes />} />
</Routes>
{/* Outside <Routes>: a provider CLI install outlives the page that
started it, so its failure toast can be clicked from any route —
including auth callback, which renders no app shell. */}
<ProviderCliInstallLogDialogHost />
<AppNavigationUrlHost>
<AppFileExternalNavigationHost>
<HashNavigationScroll />
<Routes>
<Route
path={AUTH_CALLBACK_ROUTE_PATH}
element={<AuthCallbackView />}
/>
<Route path="*" element={<AppRoutes />} />
</Routes>
{/* Outside <Routes>: a provider CLI install outlives the page that
started it, so its failure toast can be clicked from any route —
including auth callback, which renders no app shell. */}
<ProviderCliInstallLogDialogHost />
</AppFileExternalNavigationHost>
</AppNavigationUrlHost>
</RouteNavigationProvider>
</AppCommandProvider>
</QuickCreateProjectProvider>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { useEffect, useRef } from "react";
import type { ExperimentalFileOpenOptions } from "@get-bb/plugin-sdk";
import { appToast } from "@/components/ui/app-toast";
import { useLocalOpenTargets } from "@/hooks/useLocalOpenTargets";
import { useResolvedLiveFileTarget } from "@/hooks/useResolvedLiveFileTarget";
import { getExperimentalFileLocationStart } from "@/lib/live-file-navigation";

/** Lazily loaded only after an external-file intent has been accepted. */
export function AppFileExternalNavigationDispatcher({
intent,
onSettled,
}: {
intent: ExperimentalFileOpenOptions;
onSettled: () => void;
}) {
const didSettleRef = useRef(false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚨 slopcop/review — The external-file queue stops after its first item.

React reuses this dispatcher when the host removes the first queue item. didSettleRef stays true, so the second accepted request never runs or leaves the queue.

Give each request an ID and use that ID as the dispatcher key. Add a test that submits two requests before the first settles.

const resolvedTarget = useResolvedLiveFileTarget(intent.target, {
enabled: true,
});
const { isLoading: areLocalTargetsLoading, openPathInPreferredFileTarget } =
useLocalOpenTargets({
enabled: resolvedTarget.status === "available",
...(resolvedTarget.status === "available"
? { openContext: resolvedTarget.openContext }
: {}),
});

useEffect(() => {
if (
didSettleRef.current ||
resolvedTarget.status === "loading" ||
areLocalTargetsLoading
) {
return;
}
didSettleRef.current = true;
onSettled();
if (resolvedTarget.status === "unavailable") {
appToast.error("Failed to open file externally", {
description: "The file target is not available on its declared host.",
});
return;
}
const location = getExperimentalFileLocationStart(intent.location);
void openPathInPreferredFileTarget({
columnNumber: location.columnNumber,
lineNumber: location.lineNumber,
path: resolvedTarget.absolutePath,
});
}, [
intent.location,
areLocalTargetsLoading,
openPathInPreferredFileTarget,
onSettled,
resolvedTarget,
]);

return null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// @vitest-environment jsdom

import {
cleanup,
fireEvent,
render,
screen,
waitFor,
} from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { useAppNavigationHost } from "@/lib/app-navigation-host";
import { AppFileExternalNavigationHost } from "./AppFileExternalNavigationHost";

const openPreferred = vi.hoisted(() => vi.fn());

vi.mock("@/hooks/useResolvedLiveFileTarget", () => ({
useResolvedLiveFileTarget: () => ({
status: "available",
absolutePath: "/workspace/src/example.ts",
hostId: "host_1",
openContext: { kind: "local" },
}),
}));

vi.mock("@/hooks/useLocalOpenTargets", () => ({
useLocalOpenTargets: () => ({
isLoading: false,
openPathInPreferredFileTarget: openPreferred,
}),
}));

function Probe() {
const navigation = useAppNavigationHost();
return (
<button
type="button"
onClick={() =>
navigation.openFileExternally({
target: {
kind: "workspace",
environmentId: "env_1",
path: "src/example.ts",
},
location: { kind: "line", line: 12, column: 3 },
})
}
>
Open external
</button>
);
}

afterEach(() => {
cleanup();
openPreferred.mockReset();
openPreferred.mockResolvedValue(true);
});

describe("AppFileExternalNavigationHost", () => {
it("resolves and dispatches an accepted intent through the preferred target", async () => {
render(
<AppFileExternalNavigationHost>
<Probe />
</AppFileExternalNavigationHost>,
);
fireEvent.click(screen.getByRole("button", { name: "Open external" }));
await waitFor(
() =>
expect(openPreferred).toHaveBeenCalledWith({
columnNumber: 3,
lineNumber: 12,
path: "/workspace/src/example.ts",
}),
{ timeout: 5_000 },
);
});
});
68 changes: 68 additions & 0 deletions apps/app/src/components/plugin/AppFileExternalNavigationHost.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import {
lazy,
Suspense,
useCallback,
useMemo,
useRef,
useState,
type ReactNode,
} from "react";
import type { ExperimentalFileOpenOptions } from "@get-bb/plugin-sdk";
import { AppNavigationHostProvider } from "@/lib/app-navigation-host";

const MAX_PENDING_EXTERNAL_FILE_INTENTS = 32;
const LazyAppFileExternalNavigationDispatcher = lazy(() =>
import("./AppFileExternalNavigationDispatcher").then(
({ AppFileExternalNavigationDispatcher }) => ({
default: AppFileExternalNavigationDispatcher,
}),
),
);

/** App-wide preferred-external file dispatcher; discovery starts on activation. */
export function AppFileExternalNavigationHost({
children,
}: {
children: ReactNode;
}) {
const [queue, setQueue] = useState<ExperimentalFileOpenOptions[]>([]);
const queueRef = useRef(queue);
const replaceQueue = useCallback((next: ExperimentalFileOpenOptions[]) => {
queueRef.current = next;
setQueue(next);
}, []);
const openFileExternally = useCallback(
(intent: ExperimentalFileOpenOptions): boolean => {
if (queueRef.current.length >= MAX_PENDING_EXTERNAL_FILE_INTENTS) {
return false;
}
// Public SDK callers are parsed by useBbNavigate before capabilities are
// invoked; this host only queues that already-normalized internal value.
replaceQueue([...queueRef.current, intent]);
return true;
},
[replaceQueue],
);
const current = queue[0] ?? null;
const settleCurrent = useCallback(() => {
replaceQueue(queueRef.current.slice(1));
}, [replaceQueue]);

const capabilities = useMemo(
() => ({ openFileExternally }),
[openFileExternally],
);
return (
<AppNavigationHostProvider capabilities={capabilities}>
{children}
{current === null ? null : (
<Suspense fallback={null}>
<LazyAppFileExternalNavigationDispatcher
intent={current}
onSettled={settleCurrent}
/>
</Suspense>
)}
</AppNavigationHostProvider>
);
}
73 changes: 73 additions & 0 deletions apps/app/src/components/plugin/ExperimentalFileLink.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// @vitest-environment jsdom

import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { MemoryRouter } from "react-router-dom";
import { RouteNavigationProvider } from "@/components/ui/app-route-anchor";
import { AppNavigationHostProvider } from "@/lib/app-navigation-host";
import { ExperimentalFileLink } from "./ExperimentalFileLink";

afterEach(cleanup);

const target = {
kind: "workspace" as const,
environmentId: "env_1",
path: "src/example.ts",
};

describe("ExperimentalFileLink", () => {
it("sends ordinary activation to the shared preview host", () => {
const openFilePreview = vi.fn(() => true);
render(
<MemoryRouter>
<RouteNavigationProvider>
<AppNavigationHostProvider capabilities={{ openFilePreview }}>
<ExperimentalFileLink
target={target}
location={{ kind: "line", line: 12, column: 4 }}
>
example.ts:12
</ExperimentalFileLink>
</AppNavigationHostProvider>
</RouteNavigationProvider>
</MemoryRouter>,
);
fireEvent.click(screen.getByRole("link", { name: "example.ts:12" }));
expect(openFilePreview).toHaveBeenCalledWith({
target,
location: { kind: "line", line: 12, column: 4 },
});
});

it("leaves modifier clicks native", () => {
const openFilePreview = vi.fn(() => true);
render(
<MemoryRouter>
<AppNavigationHostProvider capabilities={{ openFilePreview }}>
<ExperimentalFileLink target={target}>
example.ts
</ExperimentalFileLink>
</AppNavigationHostProvider>
</MemoryRouter>,
);
fireEvent.click(screen.getByRole("link", { name: "example.ts" }), {
metaKey: true,
});
expect(openFilePreview).not.toHaveBeenCalled();
});

it("does not dispatch a malformed target supplied across a JavaScript boundary", () => {
const openFilePreview = vi.fn(() => true);
render(
<MemoryRouter>
<AppNavigationHostProvider capabilities={{ openFilePreview }}>
<ExperimentalFileLink target={{ ...target, path: "../secret" }}>
invalid
</ExperimentalFileLink>
</AppNavigationHostProvider>
</MemoryRouter>,
);
fireEvent.click(screen.getByRole("link", { name: "invalid" }));
expect(openFilePreview).not.toHaveBeenCalled();
});
});
Loading
Loading