fix(web): address open vsx review findings

t3-code[bot] committed 3 months ago

Commit 4e5bafc1 parent 64490d43 files · +43 −24Browse files

Unified · Split

3 files

apps/web/src/components/settings/ThemeSearchDialog.tsx +4 −12
apps/web/src/openVsxThemes.test.ts +19 −2
apps/web/src/openVsxThemes.ts +20 −10

apps/web/src/components/settings/ThemeSearchDialog.tsx+4−12

-import { CheckCircle2Icon, PlusIcon, SearchIcon, ShieldCheckIcon } from "lucide-react";
+import { CheckCircle2Icon, PaletteIcon, PlusIcon, SearchIcon, ShieldCheckIcon } from "lucide-react";
 import type { FormEvent } from "react";
 import { useCallback, useEffect, useRef, useState } from "react";
 import {
                       key={extension.id}
                     >
                       <div className="flex min-w-0 gap-3">
-                        {extension.iconUrl ? (
+                        <div className="flex size-10 shrink-0 items-center justify-center rounded-lg bg-muted text-muted-foreground">
-                          <img
+                          <PaletteIcon className="size-4" />
-                            alt=""
+                        </div>
-                            className="size-10 shrink-0 rounded-lg bg-muted object-cover"
-                            src={extension.iconUrl}
-                          />
-                        ) : (
-                          <div className="flex size-10 shrink-0 items-center justify-center rounded-lg bg-muted text-muted-foreground">
-                            <SearchIcon className="size-4" />
-                          </div>
-                        )}
                         <div className="min-w-0 flex-1">
                           <h4 className="truncate text-sm font-medium">{extension.name}</h4>
                           <p className="truncate text-muted-foreground text-xs">

apps/web/src/openVsxThemes.test.ts+19−2

     expect(searchUrl.searchParams.get("size")).toBe("16");
   });
+  it("reports an unavailable search when every detail request fails", async () => {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(async (input: RequestInfo | URL) => {
+        if (String(input).includes("/-/search?")) {
+          return new Response(
+            JSON.stringify({ extensions: [{ namespace: "demo", name: "theme" }] }),
+            { status: 200 },
+          );
+        }
+        return new Response(null, { status: 429 });
+      }),
+    );
+    await expect(searchOpenVsxThemes("dracula")).rejects.toThrow(
+      "Open VSX theme details are unavailable",
+    );
+  });
   it("downloads a verified VSIX, reads JSONC includes, and pairs contributed variants", async () => {
     const zip = new JSZip();
     zip.file(
       publisher: "demo",
       description: "",
       downloadCount: 1,
-      iconUrl: null,
       license: "MIT",
       manifestUrl: `${ASSET_ROOT}/package.json`,
       sha256Url: `${ASSET_ROOT}/demo.theme-1.0.0.sha256`,
       publisher: "demo",
       description: "",
       downloadCount: 1,
-      iconUrl: null,
       license: "MIT",
       manifestUrl: `${ASSET_ROOT}/package.json`,
       sha256Url: `${ASSET_ROOT}/demo.theme-1.0.0.sha256`,

apps/web/src/openVsxThemes.ts+20−10

   publisher: string;
   description: string;
   downloadCount: number;
-  iconUrl: string | null;
   manifestUrl: string;
   sha256Url: string;
   vsixUrl: string;
       typeof value.downloadCount === "number" && Number.isFinite(value.downloadCount)
         ? value.downloadCount
         : 0,
-    iconUrl: trustedOpenVsxUrl(value.files.icon),
     manifestUrl,
     sha256Url,
     vsixUrl,
     identities.slice(0, 16).map(async ([namespace, name]) => {
       const detailUrl = `https://open-vsx.org/api/${encodeURIComponent(namespace)}/${encodeURIComponent(name)}`;
       const detailResponse = await fetch(detailUrl, signal ? { signal } : {});
-      if (!detailResponse.ok) return null;
+      if (!detailResponse.ok) throw new Error("Open VSX theme details are unavailable.");
       const detailBytes = await readCappedResponse(
         detailResponse,
         MAX_DETAIL_BYTES,
     }),
   );
   if (signal?.aborted) throw new DOMException("The operation was aborted.", "AbortError");
-  return details
+  const completedDetails = details.filter((result) => result.status === "fulfilled");
-    .flatMap((result) => (result.status === "fulfilled" && result.value ? [result.value] : []))
+  if (identities.length > 0 && completedDetails.length === 0) {
-    .slice(0, 8);
+    throw new Error("Open VSX theme details are unavailable right now.");
+  }
+  return completedDetails.flatMap((result) => (result.value ? [result.value] : [])).slice(0, 8);
 }
 function parseJsoncObject(source: string, description: string): Record<string, unknown> {
 async function loadThemeObject(
   zip: JSZip,
   path: string,
+  cache: Map<string, Record<string, unknown>>,
   ancestors: ReadonlySet<string> = new Set(),
 ): Promise<Record<string, unknown>> {
   if (ancestors.size >= MAX_INCLUDE_DEPTH) throw new Error("Theme includes are nested too deeply.");
   if (ancestors.has(path)) throw new Error("Theme includes contain a cycle.");
+  const cached = cache.get(path);
+  if (cached) return cached;
   const value = parseJsoncObject(await readZipText(zip, path, path), path);
-  if (typeof value.include !== "string") return value;
+  if (typeof value.include !== "string") {
+    cache.set(path, value);
+    return value;
+  }
   const includePath = normalizePackagePath(value.include, path);
   const nextAncestors = new Set(ancestors);
   nextAncestors.add(path);
-  const base = await loadThemeObject(zip, includePath, nextAncestors);
+  const base = await loadThemeObject(zip, includePath, cache, nextAncestors);
-  return {
+  const resolved = {
     ...base,
     ...value,
     colors: {
       ...(isRecord(value.colors) ? value.colors : {}),
     },
   };
+  cache.set(path, resolved);
+  return resolved;
 }
 async function readCappedResponse(
   const parsed: Array<{ theme: ThemeDefinition; sourceName: string }> = [];
   const failures: string[] = [];
+  const themeCache = new Map<string, Record<string, unknown>>();
   for (const contribution of contributions as ThemeContribution[]) {
     if (typeof contribution.path !== "string") continue;
     try {
       const path = normalizePackagePath(contribution.path);
-      const themeValue = await loadThemeObject(zip, path);
+      const themeValue = await loadThemeObject(zip, path, themeCache);
       const type = contributionType(contribution.uiTheme);
       const label =
         typeof contribution.label === "string" && contribution.label.trim()