History

fix(react): resolve built-in Executor sidebar icon in cloud and self-host

The multiplayer shell rendered IntegrationFavicon without sourceId, so the built-in executor source (no preset icon, no displayUrl) fell through the resolution cascade to the neutral BoxIcon placeholder. Thread sourceId={slug} so it resolves the bundled /favicon-32.png, matching the local/desktop shell. Extract the resolution cascade into a pure integrationFaviconSrc helper and add a repro test pinning the with/without-sourceId divergence for the executor source.

Rhys Sullivan committed 4 months ago

Commit 79a60791 parent 606f39b3 files · +47 −4Browse files

Unified · Split

3 files
  1. packages/react/src/components/integration-favicon.test.tsx +23 −0
  2. packages/react/src/components/integration-favicon.tsx +23 −4
  3. packages/react/src/multiplayer/shell.tsx +1 −0
packages/react/src/components/integration-favicon.test.tsx+23−0
@@ -1,6 +1,7 @@
1import { describe, expect, it } from "@effect/vitest";1import { describe, expect, it } from "@effect/vitest";
2 2
3import {3import {
4 integrationFaviconSrc,
4 integrationFaviconUrl,5 integrationFaviconUrl,
5 integrationInferredUrl,6 integrationInferredUrl,
6 integrationLocalIconUrl,7 integrationLocalIconUrl,
@@ -30,6 +31,28 @@ describe("IntegrationFavicon", () => { · 23 unmodified lines
30 expect(integrationLocalIconUrl("openapi")).toBeNull();31 expect(integrationLocalIconUrl("openapi")).toBeNull();
31 });32 });
32 33
34 it("resolves the Executor sidebar icon only when the source id is threaded (cloud/self-host repro)", () => {
35 // Reconstruct the props the multiplayer shell derives for the built-in
36 // executor source (EXECUTOR_SOURCE: kind "built-in", name "Executor", no
37 // displayUrl). The sidebar's IntegrationList builds icon/url exactly this way.
38 const slug = "executor";
39 const name = "Executor";
40 const icon = integrationPresetIconUrl({ id: slug, kind: "built-in", name, url: undefined }, []);
41 const url = integrationInferredUrl({ id: slug, name }) ?? undefined;
42
43 // The built-in source matches no preset and has no inferable host, so the
44 // sourceId branch is the only thing that can resolve its icon.
45 expect(icon).toBeNull();
46 expect(url).toBeUndefined();
47
48 // Bug: cloud/self-host dropped sourceId, so the cascade fell through to null
49 // and rendered the neutral BoxIcon placeholder.
50 expect(integrationFaviconSrc({ icon, url, size: 16 })).toBeNull();
51
52 // Fix: threading sourceId resolves the bundled Executor favicon.
53 expect(integrationFaviconSrc({ icon, sourceId: slug, url, size: 16 })).toBe("/favicon-32.png");
54 });
55
33 it("finds preset icons from a source URL", () => {56 it("finds preset icons from a source URL", () => {
34 expect(57 expect(
35 integrationPresetIconUrl(58 integrationPresetIconUrl(
packages/react/src/components/integration-favicon.tsx+23−4
@@ -160,6 +160,28 @@ export function integrationPresetIconUrl( · 159 unmodified lines
160 return preset?.icon ?? null;160 return preset?.icon ?? null;
161}161}
162 162
163// Resolution cascade for the rendered favicon: first non-null, non-failed of an
164// explicit preset icon, the bundled local icon for a known source id, then a
165// favicon-service URL derived from the source URL. The built-in executor source
166// has no preset icon and no URL, so it resolves ONLY through the sourceId branch:
167// callers that drop sourceId fall through to the neutral BoxIcon placeholder.
168export function integrationFaviconSrc(args: {
169 icon?: string | null;
170 sourceId?: string;
171 url?: string;
172 size: number;
173 failedSrcs?: readonly string[];
174}): string | null {
175 const failedSrcs = args.failedSrcs ?? [];
176 return (
177 [
178 args.icon ?? null,
179 integrationLocalIconUrl(args.sourceId),
180 integrationFaviconUrl(args.url, args.size),
181 ].find((candidate) => candidate !== null && !failedSrcs.includes(candidate)) ?? null
182 );
183}
184
163export function IntegrationFavicon({185export function IntegrationFavicon({
164 icon,186 icon,
165 sourceId,187 sourceId,
@@ -172,10 +194,7 @@ export function IntegrationFavicon({ · 6 unmodified lines
172 size?: number;194 size?: number;
173}) {195}) {
174 const [failedSrcs, setFailedSrcs] = useState<readonly string[]>([]);196 const [failedSrcs, setFailedSrcs] = useState<readonly string[]>([]);
175 const src =197 const src = integrationFaviconSrc({ icon, sourceId, url, size, failedSrcs });
176 [icon ?? null, integrationLocalIconUrl(sourceId), integrationFaviconUrl(url, size)].find(
177 (candidate) => candidate !== null && !failedSrcs.includes(candidate),
178 ) ?? null;
179 198
180 if (!src) {199 if (!src) {
181 return (200 return (
packages/react/src/multiplayer/shell.tsx+1−0
@@ -198,6 +198,7 @@ function IntegrationList(props: { pathname: string; onNavigate?: () => void }) { · 197 unmodified lines
198 { id: slug, kind: integration.kind, name, url: integration.displayUrl },198 { id: slug, kind: integration.kind, name, url: integration.displayUrl },
199 integrationPlugins,199 integrationPlugins,
200 )}200 )}
201 sourceId={slug}
201 url={202 url={
202 integration.displayUrl ??203 integration.displayUrl ??
203 integrationInferredUrl({ id: slug, name }) ??204 integrationInferredUrl({ id: slug, name }) ??