From 29d37ba95d72fb1b9778475815f6189259bc07b4 Mon Sep 17 00:00:00 2001 From: SayantanCode Date: Sun, 26 Jul 2026 21:47:18 +0530 Subject: [PATCH] Fix version-switcher stuck-on-unversioned bug and Environments modal duplicates Found while manually testing the docs UI against a real production API: - The active version could get permanently stuck on "Unversioned" - the app's own default version key (usually "v1") only ever appeared in the switcher once a real ApiVersionDoc existed for it, so switching away from it before ever creating one was a one-way trip. VersionSwitcher now always surfaces it as a selectable option in that case. - Switching versions could leave the PREVIOUS version's tree/content on screen for as long as the new version's fetch took - the loading state from the last fix only covered the very first load, not subsequent switches. Now cleared and re-armed on every version change too. - The Environments modal allowed double-clicking Save to create two identical environments (nothing disabled the button mid-request) and never checked for duplicate names. Added a saving state and a duplicate-name check. --- ...version-switcher-and-environments-fixes.md | 31 +++++++++++++ packages/ui/src/DocsApp.tsx | 32 +++++++++---- .../ui/src/components/EnvironmentsModal.tsx | 43 +++++++++++++---- .../ui/src/components/VersionSwitcher.test.ts | 35 ++++++++++++++ .../ui/src/components/VersionSwitcher.tsx | 46 ++++++++++++++++++- 5 files changed, 167 insertions(+), 20 deletions(-) create mode 100644 .changeset/version-switcher-and-environments-fixes.md create mode 100644 packages/ui/src/components/VersionSwitcher.test.ts diff --git a/.changeset/version-switcher-and-environments-fixes.md b/.changeset/version-switcher-and-environments-fixes.md new file mode 100644 index 0000000..918c8f7 --- /dev/null +++ b/.changeset/version-switcher-and-environments-fixes.md @@ -0,0 +1,31 @@ +--- +"@vayo-hq/ui": patch +--- + +Fixed three more real issues found while using the docs UI against a real +production API: + +- **The active version could get permanently stuck on "Unversioned."** The + very first endpoint ever captured resolves to a default version key + (schema-engine's `resolveVersion` fallback — usually "v1") before anyone + has explicitly created that version via "Manage versions…" — real data + exists under it, but no `ApiVersionDoc` does yet, so it never appeared as + a selectable option in the version switcher. Once a user switched to + "Unversioned" (always listed, unconditionally), there was no way back + short of manually creating a version with that exact name. + `VersionSwitcher` now always surfaces the app's default version key as a + selectable option when it isn't a real stored version yet. +- **Switching versions could leave the previous version's tree/content on + screen** for however long the new version's spec/folders fetch took — + genuinely indistinguishable from the switch having silently failed, + especially on a large real API where that fetch can take several real + seconds. The loading state added previously only covered the very first + load; it's now also cleared and re-armed on every version switch. +- **The Environments modal allowed duplicate names and double-submission.** + Nothing disabled the Save button while a create/update request was in + flight, so a double-click read the same identical form state twice and + created two duplicate environments; the form also never reset after a + successful creation. Added a `saving` state that disables Save/Delete/ + Close and shows "Saving…" during the request, resets the form after a + successful creation, and added a duplicate-name check that surfaces a + clear inline error instead of silently allowing it. diff --git a/packages/ui/src/DocsApp.tsx b/packages/ui/src/DocsApp.tsx index 76ea93e..8e9642a 100644 --- a/packages/ui/src/DocsApp.tsx +++ b/packages/ui/src/DocsApp.tsx @@ -107,12 +107,15 @@ export function DocsApp({ const [me, setMe] = useState(null); const [doc, setDoc] = useState(null); const [folders, setFolders] = useState([]); - // True until the first spec/folders fetch resolves — an empty `folders` - // during that window means "haven't heard back yet," not "there's - // nothing here." A large real API can take several real seconds to - // answer, and without this the sidebar/main pane flash "No endpoints - // yet" the whole time, then swap to the real content once it arrives. - const [initialLoadPending, setInitialLoadPending] = useState(true); + // True while a spec/folders fetch is in flight — on first load AND on + // every version switch. An empty `folders` during that window means + // "haven't heard back yet," not "there's nothing here." A large real API + // can take several real seconds to answer, and without this the + // sidebar/main pane either flash "No endpoints yet" on first load, or + // (on a version switch) keep showing the PREVIOUS version's tree/content + // until the new version's slower response finally arrives — genuinely + // indistinguishable from the switch having silently failed. + const [specLoadPending, setSpecLoadPending] = useState(true); const [selectedVayoId, setSelectedVayoId] = useState(null); const [activeTab, setActiveTab] = useState("details"); // "endpoint" = today's one-at-a-time workspace (Details/Flowmap/History/ @@ -301,10 +304,18 @@ export function DocsApp({ useEffect(() => { if (!token) return; + setSpecLoadPending(true); + // Cleared up front, not just left over from whichever version was + // active before — otherwise the OLD version's tree/content stays on + // screen for however long the new version's fetch takes (the loading + // state below only kicks in once `folders`/`doc` are actually empty), + // genuinely indistinguishable from the switch having silently failed. + setFolders([]); + setDoc(null); refetchSpecAndFolders() .then(() => setError(null)) .catch((err) => setError(err instanceof ApiError ? err.message : "Failed to load spec")) - .finally(() => setInitialLoadPending(false)); + .finally(() => setSpecLoadPending(false)); // eslint-disable-next-line react-hooks/exhaustive-deps }, [config, token, activeVersion]); @@ -612,6 +623,7 @@ export function DocsApp({ setVersionsModalOpen(true)} onCompare={() => setDiffModalOpen(true)} @@ -709,7 +721,7 @@ export function DocsApp({ onMoveToFolder={canEdit ? handleMoveToFolder : noop} onAutoOrganize={canEdit ? handleAutoOrganize : noop} onBlockedMove={setError} - isLoading={initialLoadPending} + isLoading={specLoadPending} />
{error &&
{error}
} @@ -737,12 +749,12 @@ export function DocsApp({ onTryIt={tryItFromFullDoc} onSectionInView={setSelectedVayoId} settings={settings} - isLoading={initialLoadPending} + isLoading={specLoadPending} /> )} {viewMode === "endpoint" && !selected && (
- {initialLoadPending + {specLoadPending ? "Loading endpoints…" : "No endpoints captured yet — hit some routes on your API, or create one manually, and they'll show up here."}
diff --git a/packages/ui/src/components/EnvironmentsModal.tsx b/packages/ui/src/components/EnvironmentsModal.tsx index d2d4e91..9acfc64 100644 --- a/packages/ui/src/components/EnvironmentsModal.tsx +++ b/packages/ui/src/components/EnvironmentsModal.tsx @@ -39,20 +39,45 @@ export function EnvironmentsModal({ environments, onCreate, onUpdate, onDelete, const [name, setName] = useState(selected?.name ?? ""); const [rows, setRows] = useState(toRows(selected?.variables ?? {})); + // Guards against a double-click firing save() twice before the first + // request even resolves — with nothing to disable the button in between, + // both calls read the identical name/variables and create two duplicate + // environments (both selectedId and the typed name are unchanged until the + // first call actually completes). + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); function select(id: string | "new") { setSelectedId(id); const env = id !== "new" ? (environments.find((e) => e._id === id) ?? null) : null; setName(env?.name ?? ""); setRows(toRows(env?.variables ?? {})); + setError(null); } async function save() { - const variables = toVariables(rows); - if (selectedId === "new") { - await onCreate(name.trim() || "New environment", variables); - } else if (selected) { - await onUpdate(selected._id, { name: name.trim() || selected.name, variables }); + const trimmedName = name.trim() || "New environment"; + const isDuplicateName = environments.some( + (env) => env._id !== selectedId && env.name.toLowerCase() === trimmedName.toLowerCase(), + ); + if (isDuplicateName) { + setError(`An environment named "${trimmedName}" already exists.`); + return; + } + + setError(null); + setSaving(true); + try { + const variables = toVariables(rows); + if (selectedId === "new") { + await onCreate(trimmedName, variables); + setName(""); + setRows(toRows({})); + } else if (selected) { + await onUpdate(selected._id, { name: trimmedName, variables }); + } + } finally { + setSaving(false); } } @@ -122,11 +147,13 @@ export function EnvironmentsModal({ environments, onCreate, onUpdate, onDelete, + {error &&
{error}
}
{selected && ( )} - -
diff --git a/packages/ui/src/components/VersionSwitcher.test.ts b/packages/ui/src/components/VersionSwitcher.test.ts new file mode 100644 index 0000000..ac47e21 --- /dev/null +++ b/packages/ui/src/components/VersionSwitcher.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import type { ApiVersionDoc } from "@vayo-hq/types"; +import { isDefaultVersionPhantom } from "./VersionSwitcher.js"; + +function version(v: string): ApiVersionDoc { + return { + _id: `id_${v}`, + version: v, + basePathPattern: `/api/${v}`, + status: "active", + deprecatedAt: null, + sunsetAt: null, + }; +} + +describe("isDefaultVersionPhantom", () => { + it("is phantom when no ApiVersionDoc exists yet for the default version", () => { + expect(isDefaultVersionPhantom("v1", [])).toBe(true); + }); + + it("is not phantom once a real ApiVersionDoc exists for it", () => { + expect(isDefaultVersionPhantom("v1", [version("v1")])).toBe(false); + }); + + it("is not phantom when other versions exist but not the default one — still surfaced separately", () => { + // This case is still "phantom" (true) since v1 itself isn't a real + // version yet, even though v2 is — the whole point is that v1 needs + // to be synthesized regardless of what else exists. + expect(isDefaultVersionPhantom("v1", [version("v2")])).toBe(true); + }); + + it("is never phantom for 'unversioned' — it's already unconditionally rendered", () => { + expect(isDefaultVersionPhantom("unversioned", [])).toBe(false); + }); +}); diff --git a/packages/ui/src/components/VersionSwitcher.tsx b/packages/ui/src/components/VersionSwitcher.tsx index b4a5ebf..076a6b3 100644 --- a/packages/ui/src/components/VersionSwitcher.tsx +++ b/packages/ui/src/components/VersionSwitcher.tsx @@ -1,7 +1,10 @@ // @vayo-hq/ui — header dropdown for picking the active API version // (docs/07-api-versioning.md). Includes a permanent "Unversioned" entry — // captured traffic that matched no configured basePathPattern lands there, -// and it should stay reachable even when empty, same as any other version. +// and it should stay reachable even when empty, same as any other version — +// and, when it hasn't been explicitly created as a real version yet, the +// app's own default version key (see `defaultVersion` below), for the same +// reason. import { useRef, useState } from "react"; import { ChevronDown, GitBranch, GitCompare, Settings } from "lucide-react"; @@ -11,16 +14,43 @@ import { useDismiss } from "../hooks/useDismiss.js"; interface VersionSwitcherProps { versions: ApiVersionDoc[]; activeVersion: string; + /** The version key `DocsApp` was originally loaded with (the whole app's + * `version` prop, fixed at mount) — real endpoint data can exist under + * this key (schema-engine's `resolveVersion` fallback resolves everything + * to it, usually "v1", until an ApiVersionDoc is explicitly created) + * before it's ever a "real" stored version. Kept separate from + * `activeVersion` (which changes as the user navigates) specifically so + * this stays reachable even after switching away to "Unversioned" or + * anywhere else — otherwise it's a one-way trip, since nothing but + * manually creating a version with this exact name would restore it. */ + defaultVersion: string; onSelect: (version: string) => void; onManage: () => void; onCompare: () => void; } -export function VersionSwitcher({ versions, activeVersion, onSelect, onManage, onCompare }: VersionSwitcherProps): JSX.Element { +/** True when `defaultVersion` needs to be synthesized as a selectable + * option because no real `ApiVersionDoc` exists for it yet — "unversioned" + * is never phantom, since it's already unconditionally rendered below. + * Exported for direct unit testing (see VersionSwitcher.test.ts), same + * reasoning as FolderTree's `isBlockedGroupMove`. */ +export function isDefaultVersionPhantom(defaultVersion: string, versions: ApiVersionDoc[]): boolean { + return defaultVersion !== "unversioned" && !versions.some((v) => v.version === defaultVersion); +} + +export function VersionSwitcher({ + versions, + activeVersion, + defaultVersion, + onSelect, + onManage, + onCompare, +}: VersionSwitcherProps): JSX.Element { const [open, setOpen] = useState(false); const ref = useRef(null); useDismiss(ref, () => setOpen(false), open); const active = versions.find((v) => v.version === activeVersion) ?? null; + const defaultVersionIsPhantom = isDefaultVersionPhantom(defaultVersion, versions); return (
@@ -32,6 +62,18 @@ export function VersionSwitcher({ versions, activeVersion, onSelect, onManage, o {open && (
+ {defaultVersionIsPhantom && ( + + )} {versions.map((v) => (