From f4ca6bd73300a2064ab95962c8a3e986d88edeb3 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 02:55:25 +0500 Subject: [PATCH 01/16] feat(admin): add a plugin directory to browse and search published plugins Discovery only: the page joins a curated catalogue against what admin-meta reports installed, and offers no control that writes to a project's source or changes plugin state, because installing a plugin is a dependency plus a line in nextly.config.ts. Where a listed plugin is installed it describes itself - its own icon and description win over the catalogue's, through the precedence rule the registry already owns rather than a second ordering written at the call site. PluginIcon grew a multi-candidate form for that, and the candidate list comes from the module that owns the precedence. Browse plugins sits in the page header rather than only in the empty state, since a project that already has plugins is the one most likely to want another. --- .changeset/plugin-directory-browse.md | 32 +++ .../layout/sidebar/SubSidebarContent.tsx | 16 ++ .../components/shared/plugin-icon/index.tsx | 46 ++++- packages/admin/src/constants/routes.ts | 1 + .../lib/plugins/registry/install-command.ts | 41 ++++ .../resolve-catalogue-presentation.ts | 32 ++- .../pages/dashboard/plugins/browse.test.tsx | 113 +++++++++++ .../src/pages/dashboard/plugins/browse.tsx | 183 ++++++++++++++++++ .../plugins/components/PluginCard.tsx | 92 +++++++++ .../src/pages/dashboard/plugins/index.tsx | 10 + packages/admin/src/pages/registry.ts | 12 ++ 11 files changed, 561 insertions(+), 17 deletions(-) create mode 100644 .changeset/plugin-directory-browse.md create mode 100644 packages/admin/src/lib/plugins/registry/install-command.ts create mode 100644 packages/admin/src/pages/dashboard/plugins/browse.test.tsx create mode 100644 packages/admin/src/pages/dashboard/plugins/browse.tsx create mode 100644 packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx diff --git a/.changeset/plugin-directory-browse.md b/.changeset/plugin-directory-browse.md new file mode 100644 index 0000000000..bdf400a14c --- /dev/null +++ b/.changeset/plugin-directory-browse.md @@ -0,0 +1,32 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/builder": patch +"@nextlyhq/module-specifiers": patch +--- + +The admin now has a plugin directory, at Plugins then Browse plugins. + +It lists the plugins Nextly publishes with a description, category and author, marks the ones already installed, and searches by name, description and tags. A curated row sits above the grid while there is more in the grid than in the row. + +It is discovery only. Installing a plugin means adding a dependency and a line to `nextly.config.ts`, so the directory never writes to your source or changes plugin state. Where a listed plugin is already installed, its own icon and description are shown rather than the directory's copy of them. diff --git a/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx b/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx index e7ddf0045a..22211ad8bd 100644 --- a/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx +++ b/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx @@ -127,6 +127,22 @@ export function SubSidebarContent({

+ {/* Below the installed plugins, not above: this panel is for + getting to what the project already has, and the directory is + the occasional trip. Not filtered by `pluginSearch` either — + that box searches installed plugins, and an entry that ignores + it while sitting among entries that obey it reads as a bug. */} + + + + + Browse plugins + + + diff --git a/packages/admin/src/components/shared/plugin-icon/index.tsx b/packages/admin/src/components/shared/plugin-icon/index.tsx index 65731669a9..e3fdb35419 100644 --- a/packages/admin/src/components/shared/plugin-icon/index.tsx +++ b/packages/admin/src/components/shared/plugin-icon/index.tsx @@ -2,13 +2,20 @@ import type React from "react"; import { useState } from "react"; import * as Icons from "@admin/components/icons"; -import { resolvePluginIcon } from "@admin/lib/plugins/resolve-plugin-icon"; +import { resolvePluginIconFrom } from "@admin/lib/plugins/resolve-plugin-icon"; import { cn } from "@admin/lib/utils"; import type { PluginMetadata } from "@admin/types/branding"; -interface PluginIconProps { - /** The plugin whose icon to render. Only `appearance` is read. */ - plugin: Pick; +type IconCandidate = Pick | undefined; + +interface PluginIconFromProps { + /** + * Appearance sources in precedence order; the first that declares an icon + * wins. A caller with one source passes one. Callers with two — an installed + * plugin and the catalogue entry describing it — must not decide the order + * here: ask the module that owns the precedence rule for the list. + */ + candidates: readonly IconCandidate[]; /** * The lucide icon to use when the plugin declares none. * @@ -23,6 +30,11 @@ interface PluginIconProps { alt?: string; } +interface PluginIconProps extends Omit { + /** The plugin whose icon to render. Only `appearance` is read. */ + plugin: Pick; +} + /** * Render a plugin's icon, whether it ships an image or names a lucide glyph. * @@ -33,12 +45,12 @@ interface PluginIconProps { * * @module components/shared/plugin-icon */ -export function PluginIcon({ - plugin, +export function PluginIconFrom({ + candidates, fallback, className, alt = "", -}: PluginIconProps): React.ReactElement { +}: PluginIconFromProps): React.ReactElement { // A declared asset can still fail to arrive: a mistyped path, a deleted // file, or a Content-Security-Policy that blocks the origin. Without this the // surface keeps a broken-image glyph forever, which is worse than the plain @@ -51,10 +63,16 @@ export function PluginIcon({ // broken logo would suppress the next plugin's working one. Keying on the // source means a different asset is always attempted. const [failedSrc, setFailedSrc] = useState(null); - const declaredAsset = plugin.appearance?.iconAsset; - const source = resolvePluginIcon(plugin, { + // Assets are disallowed once the one this chain would pick has failed, which + // drops that candidate to its lucide name and lets the next candidate answer. + // Comparing against the chain's own choice rather than a single declared + // asset keeps that true when more than one candidate ships an image. + const declaredAsset = resolvePluginIconFrom(candidates, { fallback }); + const source = resolvePluginIconFrom(candidates, { fallback, - allowAsset: declaredAsset !== undefined && declaredAsset !== failedSrc, + allowAsset: !( + declaredAsset.kind === "asset" && declaredAsset.src === failedSrc + ), }); if (source.kind === "asset") { @@ -82,3 +100,11 @@ export function PluginIcon({ return ; } + +/** The single-source case, which is every surface but the catalogue. */ +export function PluginIcon({ + plugin, + ...rest +}: PluginIconProps): React.ReactElement { + return ; +} diff --git a/packages/admin/src/constants/routes.ts b/packages/admin/src/constants/routes.ts index cfca143ebc..33fd581777 100644 --- a/packages/admin/src/constants/routes.ts +++ b/packages/admin/src/constants/routes.ts @@ -107,6 +107,7 @@ export const ROUTES = { // Plugin routes PLUGINS: "/admin/plugins", + PLUGIN_BROWSE: "/admin/plugins/browse", PLUGIN_DETAIL: "/admin/plugins/[slug]", PLUGIN_SETTINGS: "/admin/plugins/[slug]/settings", } as const; diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts new file mode 100644 index 0000000000..936bb0cf03 --- /dev/null +++ b/packages/admin/src/lib/plugins/registry/install-command.ts @@ -0,0 +1,41 @@ +/** + * The shell command that installs a catalogue plugin. + * + * Derived from the entry's `id` rather than stored beside it, so a package + * rename cannot leave the install command fetching the old name while the + * detail page joins installed state on the new one. + * + * @module lib/plugins/registry/install-command + */ + +/** + * Package managers a Nextly project can be installed with. + * + * Enumerated rather than free-form because the add subcommand differs: npm and + * bun take `install`, pnpm and yarn take `add`. A caller passing its own string + * would have to know that, which is the knowledge this module exists to hold. + */ +export const PACKAGE_MANAGERS = ["pnpm", "npm", "yarn", "bun"] as const; + +export type PackageManager = (typeof PACKAGE_MANAGERS)[number]; + +const ADD_SUBCOMMAND: Record = { + pnpm: "add", + npm: "install", + yarn: "add", + bun: "add", +}; + +/** + * `installCommand("@acme/p")` → `"pnpm add @acme/p"`. + * + * Defaults to pnpm, which is what Nextly's own docs and scaffolder use, while + * staying answerable for the other three so a reader on npm is not handed a + * command their project cannot run. + */ +export function installCommand( + packageName: string, + manager: PackageManager = "pnpm" +): string { + return `${manager} ${ADD_SUBCOMMAND[manager]} ${packageName}`; +} diff --git a/packages/admin/src/lib/plugins/registry/resolve-catalogue-presentation.ts b/packages/admin/src/lib/plugins/registry/resolve-catalogue-presentation.ts index ba590eb3bd..adeee6bdec 100644 --- a/packages/admin/src/lib/plugins/registry/resolve-catalogue-presentation.ts +++ b/packages/admin/src/lib/plugins/registry/resolve-catalogue-presentation.ts @@ -43,6 +43,21 @@ function catalogueAppearance( }; } +/** + * The appearance sources for a catalogue entry, in precedence order. + * + * Exported so a component rendering the icon takes the ordering from here + * instead of writing `[installed, entry]` itself. That second spelling would be + * a second answer to which source wins, and the two would agree until one of + * them gained a step. + */ +export function cataloguePresentationCandidates( + entry: RegistryPlugin, + installed: Pick | undefined +): readonly (Pick | undefined)[] { + return [installed, catalogueAppearance(entry)]; +} + export interface CataloguePresentation { icon: PluginIconSource; description: string; @@ -66,13 +81,16 @@ export function resolveCataloguePresentation( opts: { allowAsset?: boolean } = {} ): CataloguePresentation { return { - icon: resolvePluginIconFrom([installed, catalogueAppearance(entry)], { - // Unreachable in practice: `catalogueAppearance` always carries a lucide - // name, since `RegistryPlugin.icon.lucide` is required. Named rather - // than asserted so the chain has a total answer if that ever loosens. - fallback: entry.icon.lucide, - allowAsset: opts.allowAsset, - }), + icon: resolvePluginIconFrom( + cataloguePresentationCandidates(entry, installed), + { + // Unreachable in practice: `catalogueAppearance` always carries a lucide + // name, since `RegistryPlugin.icon.lucide` is required. Named rather + // than asserted so the chain has a total answer if that ever loosens. + fallback: entry.icon.lucide, + allowAsset: opts.allowAsset, + } + ), // An installed plugin declaring an empty description says nothing, so it // does not get to blank the catalogue's text. description: installed?.description?.trim() diff --git a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx new file mode 100644 index 0000000000..632d9fc39e --- /dev/null +++ b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx @@ -0,0 +1,113 @@ +/** + * The directory joins two sources: a curated catalogue, and what the server + * reports as installed. These cover that join, and the route that reaches it — + * `/admin/plugins/browse` is also a legal `/admin/plugins/[slug]`, so which + * page answers is a property worth pinning rather than assuming. + */ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { ROUTES } from "@admin/constants/routes"; +import { resolveRoute } from "@admin/lib/routing"; +import type { AdminBranding } from "@admin/types/branding"; + +let mockBranding: AdminBranding = { plugins: [] } as unknown as AdminBranding; + +vi.mock("@admin/lib/api/publicApi", () => ({ + publicApi: { get: () => Promise.resolve(mockBranding) }, +})); + +import PluginBrowsePage from "./browse"; + +function renderBrowse() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + + + + ); +} + +afterEach(() => { + mockBranding = { plugins: [] } as unknown as AdminBranding; + vi.restoreAllMocks(); +}); + +describe("plugin browse route", () => { + /** + * `/admin/plugins/[slug]` matches this path too. Two independent things keep + * the directory reachable — the exact-match pass in `resolveRoute`, and the + * browse route being registered before the detail route, since the dynamic + * matcher takes the first pattern that matches. Either alone is sufficient, + * so this fails only when both are gone, which is the state that would + * actually render a plugin detail page for a plugin named "browse". + */ + it("resolves the browse path to the browse page, not the detail page", () => { + const resolved = resolveRoute(ROUTES.PLUGIN_BROWSE, ""); + + expect(resolved.Component).toBe(PluginBrowsePage); + expect(resolved.params).toEqual({}); + }); + + it("still resolves a real plugin slug to the detail page", () => { + // Positive control on the matcher: the dynamic route is reachable, so the + // assertion above is about precedence rather than about a detail route + // that stopped matching anything. + const resolved = resolveRoute("/admin/plugins/nextlyhq-plugin-seo", ""); + + expect(resolved.Component).not.toBe(PluginBrowsePage); + expect(resolved.params).toEqual({ slug: "nextlyhq-plugin-seo" }); + }); +}); + +describe("PluginBrowsePage", () => { + it("marks a catalogue entry installed when admin-meta reports it", async () => { + mockBranding = { + plugins: [{ name: "@nextlyhq/plugin-page-builder" }], + } as unknown as AdminBranding; + + renderBrowse(); + + expect(await screen.findAllByText("Page Builder")).not.toHaveLength(0); + expect( + screen.getAllByTestId("installed-@nextlyhq/plugin-page-builder").length + ).toBeGreaterThan(0); + }); + + /** + * Required alongside the case above: on its own, the positive test passes on + * a page that marks everything installed, and this one passes on a page that + * renders nothing. Only the pair separates them, which is why each asserts + * the entry IS rendered before asserting what it is not. + */ + it("does not mark it installed when admin-meta does not report it", async () => { + mockBranding = { plugins: [] } as unknown as AdminBranding; + + renderBrowse(); + + expect(await screen.findAllByText("Page Builder")).not.toHaveLength(0); + expect( + screen.queryByTestId("installed-@nextlyhq/plugin-page-builder") + ).toBeNull(); + }); + + it("prefers an installed plugin's own description over the catalogue's", async () => { + mockBranding = { + plugins: [ + { + name: "@nextlyhq/plugin-seo", + description: "What the installed plugin says about itself", + }, + ], + } as unknown as AdminBranding; + + renderBrowse(); + + expect( + await screen.findByText("What the installed plugin says about itself") + ).toBeInTheDocument(); + }); +}); diff --git a/packages/admin/src/pages/dashboard/plugins/browse.tsx b/packages/admin/src/pages/dashboard/plugins/browse.tsx new file mode 100644 index 0000000000..7efb44707f --- /dev/null +++ b/packages/admin/src/pages/dashboard/plugins/browse.tsx @@ -0,0 +1,183 @@ +"use client"; + +/** + * Plugin Directory + * + * Browse the plugins Nextly publishes, see which are already installed, and + * get the commands to add one. Discovery only: installing a plugin means + * adding a dependency and a line to `nextly.config.ts`, so this page never + * writes to a project's source and never mutates plugin state. + * + * @module pages/dashboard/plugins/browse + */ + +import { useSuspenseQuery } from "@tanstack/react-query"; +import type React from "react"; +import { Suspense, useMemo, useState } from "react"; + +import { PageContainer } from "@admin/components/layout/page-container"; +import { Breadcrumbs } from "@admin/components/shared"; +import { PageErrorFallback } from "@admin/components/shared/error-fallbacks"; +import { QueryErrorBoundary } from "@admin/components/shared/query-error-boundary"; +import { SearchBar } from "@admin/components/shared/search-bar"; +import { ROUTES } from "@admin/constants/routes"; +import { publicApi } from "@admin/lib/api/publicApi"; +import { + shouldShowFeatured, + staticRegistrySource, +} from "@admin/lib/plugins/registry/static-source"; +import type { RegistryPlugin } from "@admin/lib/plugins/registry/types"; +import type { AdminBranding, PluginMetadata } from "@admin/types/branding"; + +import { PluginCard } from "./components/PluginCard"; + +/** Name, description and tags. Author is deliberately not searched: every + * first-party entry shares one, so it would match the whole catalogue. */ +function matches(plugin: RegistryPlugin, query: string): boolean { + const haystack = [plugin.name, plugin.description, ...(plugin.tags ?? [])] + .join(" ") + .toLowerCase(); + return haystack.includes(query); +} + +function PluginGrid({ + plugins, + installedByName, +}: { + plugins: RegistryPlugin[]; + installedByName: Map; +}): React.ReactElement { + return ( +
+ {plugins.map(plugin => ( + + ))} +
+ ); +} + +function BrowseContent(): React.ReactElement { + const [query, setQuery] = useState(""); + + const { data: branding } = useSuspenseQuery({ + queryKey: ["admin-meta"], + queryFn: () => publicApi.get("/admin-meta"), + staleTime: 5 * 60 * 1000, + }); + const { data: entries } = useSuspenseQuery({ + queryKey: ["plugin-registry"], + queryFn: () => staticRegistrySource.list(), + }); + const { data: featuredIds } = useSuspenseQuery({ + queryKey: ["plugin-registry-featured"], + queryFn: () => staticRegistrySource.featured(), + }); + + // Installed truth comes from admin-meta and nothing else. An entry absent + // from it is not installed; an installed plugin absent from the catalogue is + // private or third-party and is correctly not listed here. + const installedByName = useMemo(() => { + const map = new Map(); + for (const plugin of branding?.plugins ?? []) map.set(plugin.name, plugin); + return map; + }, [branding?.plugins]); + + const normalized = query.trim().toLowerCase(); + const visible = useMemo( + () => (normalized ? entries.filter(e => matches(e, normalized)) : entries), + [entries, normalized] + ); + + const featured = useMemo( + () => + featuredIds + .map(id => entries.find(e => e.id === id)) + .filter((e): e is RegistryPlugin => e !== undefined), + [entries, featuredIds] + ); + + // The strip is a recommendation, so it is hidden while searching: a filtered + // grid is the user's own list, and a fixed "start here" row above it answers + // a question they stopped asking. + const showFeatured = !normalized && shouldShowFeatured(entries, featuredIds); + + return ( + <> +
+ +
+ + {showFeatured && ( +
+ + +
+ )} + +
+

+ {showFeatured ? "All plugins" : "Plugins"} +

+ {visible.length > 0 ? ( + + ) : ( +

+ No plugins match “{query.trim()}”. +

+ )} +
+ + ); +} + +const PluginBrowsePage: React.FC = () => ( + }> + + + +
+

Browse plugins

+

+ Plugins published by Nextly. Adding one is a dependency and a line in + your Nextly config, so open a plugin to get both. +

+
+ + + + +); + +export default PluginBrowsePage; diff --git a/packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx b/packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx new file mode 100644 index 0000000000..4991fc5351 --- /dev/null +++ b/packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx @@ -0,0 +1,92 @@ +import { Badge } from "@nextlyhq/ui"; +import type React from "react"; + +import { PluginIconFrom } from "@admin/components/shared/plugin-icon"; +import { Link } from "@admin/components/ui/link"; +import { ROUTES, buildRoute } from "@admin/constants/routes"; +import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; +import { pluginSlug } from "@admin/lib/plugins/plugin-slug"; +import { + cataloguePresentationCandidates, + resolveCataloguePresentation, +} from "@admin/lib/plugins/registry/resolve-catalogue-presentation"; +import type { RegistryPlugin } from "@admin/lib/plugins/registry/types"; +import type { PluginMetadata } from "@admin/types/branding"; + +interface PluginCardProps { + plugin: RegistryPlugin; + /** + * The installed plugin this entry describes, when the project has it. + * + * The metadata itself rather than a boolean: an installed plugin is the + * authority on its own icon and description, so the card needs what it + * declared and not merely the fact that it exists. + */ + installed: Pick | undefined; +} + +/** + * One plugin in the directory grid. + * + * A link rather than a card with a button inside it: the whole card is one + * destination, so making the card the anchor gives a keyboard user one stop + * instead of a container they must enter to find the real control. + * + * @module pages/dashboard/plugins/components/PluginCard + */ +export function PluginCard({ + plugin, + installed, +}: PluginCardProps): React.ReactElement { + const presentation = resolveCataloguePresentation(plugin, installed); + const label = categoryLabel(plugin.category); + + return ( + +
+ + + + +
+ {/* `truncate` needs the min-w-0 above: a flex child defaults to + min-width:auto, which refuses to shrink below its content. */} +

{plugin.name}

+

+ {plugin.author} +

+
+ + {presentation.isInstalled && ( + + Installed + + )} +
+ + {/* Two lines, not a character count: a clamp adapts to the card's real + width, where a truncated string would cut at a different place on + every breakpoint. */} +

+ {presentation.description} +

+ + {label && ( + + {label} + + )} + + ); +} diff --git a/packages/admin/src/pages/dashboard/plugins/index.tsx b/packages/admin/src/pages/dashboard/plugins/index.tsx index 95d4b9d55d..6523d462ac 100644 --- a/packages/admin/src/pages/dashboard/plugins/index.tsx +++ b/packages/admin/src/pages/dashboard/plugins/index.tsx @@ -10,6 +10,7 @@ * @module pages/dashboard/plugins */ +import { Button } from "@nextlyhq/ui"; import type React from "react"; import { Suspense } from "react"; @@ -17,6 +18,7 @@ import { PageContainer } from "@admin/components/layout/page-container"; import { Breadcrumbs } from "@admin/components/shared"; import { PageErrorFallback } from "@admin/components/shared/error-fallbacks"; import { QueryErrorBoundary } from "@admin/components/shared/query-error-boundary"; +import { Link } from "@admin/components/ui/link"; import { ROUTES } from "@admin/constants/routes"; import PluginsTable from "./components/PluginsTable"; @@ -52,6 +54,14 @@ const PluginsOverviewPage: React.FC = () => { whether it is enabled.

+ + {/* In the header rather than only in the empty state: a project that + already has plugins is the one most likely to want another, and an + action that appears only while the list is empty is unreachable + exactly then. */} + {/* Plugins table */} diff --git a/packages/admin/src/pages/registry.ts b/packages/admin/src/pages/registry.ts index 7bc48ffb8e..0af3262e92 100644 --- a/packages/admin/src/pages/registry.ts +++ b/packages/admin/src/pages/registry.ts @@ -20,6 +20,7 @@ import DashboardPage from "./dashboard/index"; import MediaLibraryPage from "./dashboard/media/index"; import PluginDetailPage from "./dashboard/plugins/[slug]"; import PluginSettingsPage from "./dashboard/plugins/[slug]/settings"; +import PluginBrowsePage from "./dashboard/plugins/browse"; import PluginsOverviewPage from "./dashboard/plugins/index"; import CollectionsLandingRedirect from "./dashboard/redirects/CollectionsLandingRedirect"; import FieldGroupsLandingRedirect from "./dashboard/redirects/FieldGroupsLandingRedirect"; @@ -372,6 +373,17 @@ export const routeConfig: Record = { type: "private", requiredPermission: "manage-settings", }, + // `/admin/plugins/[slug]` matches `/admin/plugins/browse` too, so which page + // answers is decided rather than incidental. Two things decide it, and + // either alone is sufficient: `resolveRoute` tries an exact-match pass before + // the dynamic matcher, and the dynamic matcher returns the first pattern that + // matches in insertion order, which is this one. Kept above PLUGIN_DETAIL so + // the ordering half stays true if the exact pass is ever reworked. + [ROUTES.PLUGIN_BROWSE]: { + component: PluginBrowsePage, + type: "private", + requiredPermission: "manage-settings", + }, [ROUTES.PLUGIN_DETAIL]: { component: PluginDetailPage, type: "private", From 59009918e5682145f52a53bb72be87bb8522919e Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 03:02:07 +0500 Subject: [PATCH 02/16] fix(admin): stop the plugin directory listing featured entries twice The grid rendered every entry, including the two the strip above it already showed, so two of three cards appeared on screen twice. The grid now holds what the strip does not, and its heading says More plugins. The page's search box also read Search plugins, the same words as the sidebar box beside it, while searching a different set: the sidebar filters installed plugins and this one filters the directory. It now says Search the directory. The empty state distinguishes a search that matched nothing, which names the term, from an empty catalogue, which cannot quote a term the reader never typed. --- .../src/pages/dashboard/plugins/browse.tsx | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/admin/src/pages/dashboard/plugins/browse.tsx b/packages/admin/src/pages/dashboard/plugins/browse.tsx index 7efb44707f..5335a59c6e 100644 --- a/packages/admin/src/pages/dashboard/plugins/browse.tsx +++ b/packages/admin/src/pages/dashboard/plugins/browse.tsx @@ -105,13 +105,25 @@ function BrowseContent(): React.ReactElement { // a question they stopped asking. const showFeatured = !normalized && shouldShowFeatured(entries, featuredIds); + // The grid holds what the strip does not. Showing every entry below a strip + // that repeats two of them puts the same card on screen twice, which reads + // as a rendering fault rather than as a recommendation. + const featuredIdSet = useMemo( + () => new Set(showFeatured ? featuredIds : []), + [showFeatured, featuredIds] + ); + const rest = useMemo( + () => visible.filter(e => !featuredIdSet.has(e.id)), + [visible, featuredIdSet] + ); + return ( <>
@@ -132,13 +144,18 @@ function BrowseContent(): React.ReactElement { id="all-plugins" className="mb-3 text-xs font-medium uppercase tracking-wide text-muted-foreground" > - {showFeatured ? "All plugins" : "Plugins"} + {showFeatured ? "More plugins" : "Plugins"} - {visible.length > 0 ? ( - + {rest.length > 0 ? ( + ) : ( + // Two different nothings. A search that matched nothing names the + // term, because the reader chose it and can edit it; an empty + // catalogue must not, since quoting an empty string reads as a bug.

- No plugins match “{query.trim()}”. + {normalized + ? `No plugins match “${query.trim()}”.` + : "No plugins to show yet."}

)} From 6a6887ce4b01771857ff47d4bc4bb102ccb80a7e Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 07:46:02 +0500 Subject: [PATCH 03/16] refactor(admin): drop the install-command helper until a page calls it It derives the shell command for a catalogue entry, which the plugin detail page needs and the directory does not. A helper with no caller is untested against the shape its caller will actually want, so it lands with that page instead. --- .../lib/plugins/registry/install-command.ts | 41 ------------------- 1 file changed, 41 deletions(-) delete mode 100644 packages/admin/src/lib/plugins/registry/install-command.ts diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts deleted file mode 100644 index 936bb0cf03..0000000000 --- a/packages/admin/src/lib/plugins/registry/install-command.ts +++ /dev/null @@ -1,41 +0,0 @@ -/** - * The shell command that installs a catalogue plugin. - * - * Derived from the entry's `id` rather than stored beside it, so a package - * rename cannot leave the install command fetching the old name while the - * detail page joins installed state on the new one. - * - * @module lib/plugins/registry/install-command - */ - -/** - * Package managers a Nextly project can be installed with. - * - * Enumerated rather than free-form because the add subcommand differs: npm and - * bun take `install`, pnpm and yarn take `add`. A caller passing its own string - * would have to know that, which is the knowledge this module exists to hold. - */ -export const PACKAGE_MANAGERS = ["pnpm", "npm", "yarn", "bun"] as const; - -export type PackageManager = (typeof PACKAGE_MANAGERS)[number]; - -const ADD_SUBCOMMAND: Record = { - pnpm: "add", - npm: "install", - yarn: "add", - bun: "add", -}; - -/** - * `installCommand("@acme/p")` → `"pnpm add @acme/p"`. - * - * Defaults to pnpm, which is what Nextly's own docs and scaffolder use, while - * staying answerable for the other three so a reader on npm is not handed a - * command their project cannot run. - */ -export function installCommand( - packageName: string, - manager: PackageManager = "pnpm" -): string { - return `${manager} ${ADD_SUBCOMMAND[manager]} ${packageName}`; -} From abd925ccd3572eed482a3e0597e0866a1bd76b9a Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 08:19:46 +0500 Subject: [PATCH 04/16] feat(admin): give an uninstalled catalogue plugin its own page Every directory card linked to /admin/plugins/[slug], which searched only the installed plugins, so a card for a plugin nobody had installed rendered Plugin not found. Uninstalled is the ordinary case for a directory, which made the primary discovery path a dead end. That page now consults the catalogue when the project has no such plugin, and renders a deliberately thin view: what the catalogue claims, the install command with a package-manager switch, and the config line, each copyable. Nothing about contributions, permissions or routes, because nothing here has been observed running - the invariant is that verified content only ever appears in the verified section, and the page says so rather than leaving the absence to be misread. The catalogue lookup lives in its own component so its query is not a dependency of rendering an installed plugin, which would have made a query client a requirement of pages that need none. Also: the sidebar Browse item is gated on the permission its route requires, Installed Plugins matches exactly now that it has a sibling subpage, the directory search reads the description the card renders rather than the catalogue's copy of it, and a failed icon asset now removes only that URL from the chain instead of disabling every candidate's image. --- .../features/dashboard/DynamicPluginNav.tsx | 7 +- .../layout/sidebar/SubSidebarContent.tsx | 31 +-- .../components/shared/plugin-icon/index.tsx | 40 ++-- .../lib/plugins/registry/install-command.ts | 41 ++++ .../src/lib/plugins/resolve-plugin-icon.ts | 24 ++- .../src/pages/dashboard/plugins/[slug].tsx | 81 ++++++-- .../pages/dashboard/plugins/browse.test.tsx | 37 +++- .../src/pages/dashboard/plugins/browse.tsx | 31 ++- .../plugins/components/NotInstalledPlugin.tsx | 189 ++++++++++++++++++ .../plugins/not-installed-detail.test.tsx | 95 +++++++++ .../dashboard/plugins/plugin-detail.test.tsx | 20 +- 11 files changed, 533 insertions(+), 63 deletions(-) create mode 100644 packages/admin/src/lib/plugins/registry/install-command.ts create mode 100644 packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx create mode 100644 packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx diff --git a/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx b/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx index 7ed82c2ebf..9c85dce082 100644 --- a/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx +++ b/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx @@ -54,9 +54,12 @@ interface DynamicPluginNavProps { function PluginOverviewLink({ isActive, }: { - isActive: (href?: string) => boolean; + isActive: (href?: string, exactMatch?: boolean) => boolean; }) { - const active = isActive("/admin/plugins"); + // Exact: `/admin/plugins` is a prefix of `/admin/plugins/browse`, and the + // default match treats every descendant as active, so both sibling entries + // would highlight at once for anyone on the directory. + const active = isActive("/admin/plugins", true); return ( diff --git a/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx b/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx index 22211ad8bd..bd2f23788b 100644 --- a/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx +++ b/packages/admin/src/components/layout/sidebar/SubSidebarContent.tsx @@ -127,22 +127,29 @@ export function SubSidebarContent({

- {/* Below the installed plugins, not above: this panel is for + {/* Gated on the permission `PLUGIN_BROWSE` itself requires. The + panel stays open to a user who can only read a plugin-owned + collection, so without this they would be offered a destination + that redirects them the moment they choose it. + + Below the installed plugins, not above: this panel is for getting to what the project already has, and the directory is the occasional trip. Not filtered by `pluginSearch` either — that box searches installed plugins, and an entry that ignores it while sitting among entries that obey it reads as a bug. */} - - - - - Browse plugins - - - + {hasPermission("manage-settings") && ( + + + + + Browse plugins + + + + )} diff --git a/packages/admin/src/components/shared/plugin-icon/index.tsx b/packages/admin/src/components/shared/plugin-icon/index.tsx index e3fdb35419..37963d36e9 100644 --- a/packages/admin/src/components/shared/plugin-icon/index.tsx +++ b/packages/admin/src/components/shared/plugin-icon/index.tsx @@ -54,25 +54,22 @@ export function PluginIconFrom({ // A declared asset can still fail to arrive: a mistyped path, a deleted // file, or a Content-Security-Policy that blocks the origin. Without this the // surface keeps a broken-image glyph forever, which is worse than the plain - // icon it replaced. On failure the component re-resolves with assets - // disallowed, so it lands on whatever lucide name the plugin declared beside - // the asset before reaching the caller's fallback. - // The failed URL rather than a boolean. A boolean survives client-side - // navigation between two plugin detail pages, because the router renders the - // same component type without a key, so React keeps the state: one plugin's - // broken logo would suppress the next plugin's working one. Keying on the - // source means a different asset is always attempted. - const [failedSrc, setFailedSrc] = useState(null); - // Assets are disallowed once the one this chain would pick has failed, which - // drops that candidate to its lucide name and lets the next candidate answer. - // Comparing against the chain's own choice rather than a single declared - // asset keeps that true when more than one candidate ships an image. - const declaredAsset = resolvePluginIconFrom(candidates, { fallback }); + // icon it replaced. + // + // The URLs that have failed, not a boolean. A broken image on one candidate + // says nothing about a different image a later candidate ships, so each + // failure removes exactly one URL from consideration and the chain is + // re-resolved: the next asset is tried, and only when none load does it + // settle on a glyph. Keying on the URL also survives client-side navigation + // between two plugin detail pages, where the router renders the same + // component type without a key so React keeps this state — one plugin's + // broken logo must not suppress the next plugin's working one. + const [failedSrcs, setFailedSrcs] = useState>( + () => new Set() + ); const source = resolvePluginIconFrom(candidates, { fallback, - allowAsset: !( - declaredAsset.kind === "asset" && declaredAsset.src === failedSrc - ), + skipAssets: failedSrcs, }); if (source.kind === "asset") { @@ -85,7 +82,14 @@ export function PluginIconFrom({ {alt} setFailedSrc(source.src)} + onError={() => + setFailedSrcs(prev => { + if (prev.has(source.src)) return prev; + const next = new Set(prev); + next.add(source.src); + return next; + }) + } className={cn("object-contain", className)} /> ); diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts new file mode 100644 index 0000000000..936bb0cf03 --- /dev/null +++ b/packages/admin/src/lib/plugins/registry/install-command.ts @@ -0,0 +1,41 @@ +/** + * The shell command that installs a catalogue plugin. + * + * Derived from the entry's `id` rather than stored beside it, so a package + * rename cannot leave the install command fetching the old name while the + * detail page joins installed state on the new one. + * + * @module lib/plugins/registry/install-command + */ + +/** + * Package managers a Nextly project can be installed with. + * + * Enumerated rather than free-form because the add subcommand differs: npm and + * bun take `install`, pnpm and yarn take `add`. A caller passing its own string + * would have to know that, which is the knowledge this module exists to hold. + */ +export const PACKAGE_MANAGERS = ["pnpm", "npm", "yarn", "bun"] as const; + +export type PackageManager = (typeof PACKAGE_MANAGERS)[number]; + +const ADD_SUBCOMMAND: Record = { + pnpm: "add", + npm: "install", + yarn: "add", + bun: "add", +}; + +/** + * `installCommand("@acme/p")` → `"pnpm add @acme/p"`. + * + * Defaults to pnpm, which is what Nextly's own docs and scaffolder use, while + * staying answerable for the other three so a reader on npm is not handed a + * command their project cannot run. + */ +export function installCommand( + packageName: string, + manager: PackageManager = "pnpm" +): string { + return `${manager} ${ADD_SUBCOMMAND[manager]} ${packageName}`; +} diff --git a/packages/admin/src/lib/plugins/resolve-plugin-icon.ts b/packages/admin/src/lib/plugins/resolve-plugin-icon.ts index 1d1ca5a2cf..cf75bb9b1e 100644 --- a/packages/admin/src/lib/plugins/resolve-plugin-icon.ts +++ b/packages/admin/src/lib/plugins/resolve-plugin-icon.ts @@ -39,20 +39,36 @@ export type PluginIconSource = */ export function resolvePluginIconFrom( candidates: readonly (Pick | undefined)[], - opts: { fallback: string; allowAsset: false } + opts: { + fallback: string; + allowAsset: false; + skipAssets?: ReadonlySet; + } ): Extract; export function resolvePluginIconFrom( candidates: readonly (Pick | undefined)[], - opts: { fallback: string; allowAsset?: boolean } + opts: { + fallback: string; + allowAsset?: boolean; + skipAssets?: ReadonlySet; + } ): PluginIconSource; export function resolvePluginIconFrom( candidates: readonly (Pick | undefined)[], - opts: { fallback: string; allowAsset?: boolean } + opts: { + fallback: string; + allowAsset?: boolean; + skipAssets?: ReadonlySet; + } ): PluginIconSource { for (const candidate of candidates) { const asset = candidate?.appearance?.iconAsset; - if (asset && opts.allowAsset !== false) + // A skipped asset drops only that candidate's image; the candidate's own + // glyph is still preferred over the next candidate, because a plugin + // naming a glyph beside a broken logo has said what to show instead. + if (asset && opts.allowAsset !== false && !opts.skipAssets?.has(asset)) { return { kind: "asset", src: asset }; + } const name = candidate?.appearance?.icon; if (name) return { kind: "lucide", name }; diff --git a/packages/admin/src/pages/dashboard/plugins/[slug].tsx b/packages/admin/src/pages/dashboard/plugins/[slug].tsx index eec6eba8fd..413f4cde54 100644 --- a/packages/admin/src/pages/dashboard/plugins/[slug].tsx +++ b/packages/admin/src/pages/dashboard/plugins/[slug].tsx @@ -1,6 +1,7 @@ "use client"; import { Badge } from "@nextlyhq/ui"; +import { useSuspenseQuery } from "@tanstack/react-query"; import { BookOpen, @@ -26,8 +27,10 @@ import { ROUTES, buildRoute } from "@admin/constants/routes"; import { useBranding } from "@admin/context/providers/BrandingProvider"; import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; import { pluginSlug } from "@admin/lib/plugins/plugin-slug"; +import { staticRegistrySource } from "@admin/lib/plugins/registry/static-source"; import type { PluginMetadata } from "@admin/types/branding"; +import { NotInstalledPlugin } from "./components/NotInstalledPlugin"; import { PluginStatusPill } from "./components/PluginsTable"; const PLACEMENT_LABELS: Record = { @@ -66,38 +69,82 @@ export default function PluginDetailPage({ ); } -function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { - const branding = useBranding(); - const plugins = branding?.plugins ?? []; - const plugin = activeSlug - ? plugins.find(p => pluginSlug(p.name) === activeSlug) +/** + * A slug the project has no plugin for. + * + * Two outcomes, and they are different facts: the catalogue knows this package + * and the reader has simply not installed it, or nothing knows it at all. The + * first is the ordinary path from the directory, where most entries are not + * installed, so it must not be reported as an error. + * + * Its own component because it queries the catalogue. Held inside + * `PluginDetailContent` the query would run for every installed plugin too, + * making a `QueryClientProvider` a requirement of rendering a page that has no + * need of one. + */ +function UninstalledOrMissing({ activeSlug }: { activeSlug?: string }) { + const { data: entries } = useSuspenseQuery({ + queryKey: ["plugin-registry"], + queryFn: () => staticRegistrySource.list(), + }); + const entry = activeSlug + ? entries.find(e => pluginSlug(e.id) === activeSlug) : undefined; - if (!plugin) { + if (entry) { return (
-
- -

- Plugin not found -

-

- No installed plugin matches this address. It may have been removed - from your Nextly config. -

-
+
); } + return ( +
+ +
+ +

+ Plugin not found +

+

+ No installed plugin matches this address, and it is not in the plugin + directory either. It may have been removed from your Nextly config. +

+
+
+ ); +} + +function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { + const branding = useBranding(); + const plugins = branding?.plugins ?? []; + const plugin = activeSlug + ? plugins.find(p => pluginSlug(p.name) === activeSlug) + : undefined; + + // Installed metadata is observed; the catalogue is a claim, so the observed + // one decides. Only when the project has no such plugin is the catalogue + // consulted, and that lookup lives inside `UninstalledOrMissing` so its query + // is not a dependency of rendering an installed plugin. + if (!plugin) return ; + const title = plugin.appearance?.label ?? plugin.name; return ( diff --git a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx index 632d9fc39e..15dac9f269 100644 --- a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx @@ -5,7 +5,7 @@ * page answers is a property worth pinning rather than assuming. */ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { ROUTES } from "@admin/constants/routes"; @@ -94,6 +94,41 @@ describe("PluginBrowsePage", () => { ).toBeNull(); }); + /** + * Search reads what the card renders, not what the catalogue stores. An + * installed plugin's own description is what a reader sees, so typing a word + * from it must keep the card rather than filter it away. + */ + it("finds a card by the installed description it actually shows", async () => { + mockBranding = { + plugins: [ + { + name: "@nextlyhq/plugin-seo", + description: "Zebra crossing metadata", + }, + ], + } as unknown as AdminBranding; + + renderBrowse(); + await screen.findByText("Zebra crossing metadata"); + + fireEvent.change(screen.getByPlaceholderText("Search the directory"), { + target: { value: "zebra" }, + }); + + // Waited for, not asserted immediately: `SearchBar` debounces by 300ms, so + // a synchronous check runs before the query reaches this page and passes + // whatever the search would have done. + // + // The disappearance is the control. Asserting only that SEO survives is + // satisfied by a page that never filtered at all, which is the same green + // a search reading the wrong field produces. + await waitFor(() => + expect(screen.queryByRole("heading", { name: "Page Builder" })).toBeNull() + ); + expect(screen.getByRole("heading", { name: "SEO" })).toBeInTheDocument(); + }); + it("prefers an installed plugin's own description over the catalogue's", async () => { mockBranding = { plugins: [ diff --git a/packages/admin/src/pages/dashboard/plugins/browse.tsx b/packages/admin/src/pages/dashboard/plugins/browse.tsx index 5335a59c6e..00c7872567 100644 --- a/packages/admin/src/pages/dashboard/plugins/browse.tsx +++ b/packages/admin/src/pages/dashboard/plugins/browse.tsx @@ -22,6 +22,7 @@ import { QueryErrorBoundary } from "@admin/components/shared/query-error-boundar import { SearchBar } from "@admin/components/shared/search-bar"; import { ROUTES } from "@admin/constants/routes"; import { publicApi } from "@admin/lib/api/publicApi"; +import { resolveCataloguePresentation } from "@admin/lib/plugins/registry/resolve-catalogue-presentation"; import { shouldShowFeatured, staticRegistrySource, @@ -31,10 +32,25 @@ import type { AdminBranding, PluginMetadata } from "@admin/types/branding"; import { PluginCard } from "./components/PluginCard"; -/** Name, description and tags. Author is deliberately not searched: every - * first-party entry shares one, so it would match the whole catalogue. */ -function matches(plugin: RegistryPlugin, query: string): boolean { - const haystack = [plugin.name, plugin.description, ...(plugin.tags ?? [])] +/** + * Name, the RENDERED description, and tags. + * + * The description comes from the same resolution the card renders, so an + * installed plugin is searched by the text a reader can actually see. Reading + * `plugin.description` here instead would search the catalogue's copy while + * the card shows the plugin's own, and typing a word visibly on screen would + * make that card disappear. + * + * Author is deliberately not searched: every first-party entry shares one, so + * it would match the whole catalogue. + */ +function matches( + plugin: RegistryPlugin, + installed: Pick | undefined, + query: string +): boolean { + const { description } = resolveCataloguePresentation(plugin, installed); + const haystack = [plugin.name, description, ...(plugin.tags ?? [])] .join(" ") .toLowerCase(); return haystack.includes(query); @@ -88,8 +104,11 @@ function BrowseContent(): React.ReactElement { const normalized = query.trim().toLowerCase(); const visible = useMemo( - () => (normalized ? entries.filter(e => matches(e, normalized)) : entries), - [entries, normalized] + () => + normalized + ? entries.filter(e => matches(e, installedByName.get(e.id), normalized)) + : entries, + [entries, installedByName, normalized] ); const featured = useMemo( diff --git a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx new file mode 100644 index 0000000000..8be8156d85 --- /dev/null +++ b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx @@ -0,0 +1,189 @@ +"use client"; + +import { Button } from "@nextlyhq/ui"; +import type React from "react"; +import { useState } from "react"; + +import { Check, Copy } from "@admin/components/icons"; +import { PluginIcon } from "@admin/components/shared/plugin-icon"; +import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; +import { + PACKAGE_MANAGERS, + installCommand, + type PackageManager, +} from "@admin/lib/plugins/registry/install-command"; +import type { RegistryPlugin } from "@admin/lib/plugins/registry/types"; + +/** + * A copyable command line. + * + * The confirmation lives on the button rather than in a toast: the reader is + * looking at the thing they clicked, and a toast in the corner asks them to + * look somewhere else to learn that something local succeeded. + */ +function CopyLine({ value, label }: { value: string; label: string }) { + const [copied, setCopied] = useState(false); + + return ( +
+

+ {label} +

+
+ + {value} + + +
+
+ ); +} + +/** + * What a catalogue plugin's page shows when the project has not installed it. + * + * Deliberately thin, and the reason is the invariant the whole surface is + * built on: verified content only ever appears in the verified section. Nothing + * here has been observed running, so there is no contributions section, no + * permissions and no routes — only what the catalogue claims, plus the two + * commands that would make the claims checkable. + * + * @module pages/dashboard/plugins/components/NotInstalledPlugin + */ +export function NotInstalledPlugin({ + plugin, +}: { + plugin: RegistryPlugin; +}): React.ReactElement { + const [manager, setManager] = useState("pnpm"); + const label = categoryLabel(plugin.category); + + return ( +
+
+ + + +
+

+ {plugin.name} +

+

+ {plugin.author} + {label ? ` · ${label}` : ""} +

+
+
+ +

+ {plugin.description} +

+ +
+
+

Add it to your project

+

+ Two steps: install the package, then register it in your Nextly + config. Nextly picks it up on the next start. +

+
+ +
+ {PACKAGE_MANAGERS.map(pm => ( + + ))} +
+ + + +
+ + {/* States the boundary rather than leaving it implied: a reader looking + for permissions and routes should learn why they are absent instead + of concluding the plugin contributes nothing. */} +

+ What this plugin adds — its collections, permissions and API routes — is + only known once it is installed and Nextly has loaded it. +

+ + {plugin.links && ( +
+ {plugin.links.homepage && ( + + )} + {plugin.links.repository && ( + + )} + {plugin.links.docs && ( + + )} +
+ )} +
+ ); +} diff --git a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx new file mode 100644 index 0000000000..39dfd80043 --- /dev/null +++ b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx @@ -0,0 +1,95 @@ +/** + * A plugin's page has one URL whether or not the project has it. These cover + * the uninstalled half: the directory links every card here, and most cards + * are for plugins nobody has installed yet, so that is the ordinary path + * rather than an error. + */ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { AdminBranding } from "@admin/types/branding"; + +let mockBranding: AdminBranding = { plugins: [] } as unknown as AdminBranding; + +vi.mock("@admin/context/providers/BrandingProvider", () => ({ + useBranding: () => mockBranding, +})); + +import PluginDetailPage from "./[slug]"; + +function renderDetail(slug: string) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + + + + ); +} + +afterEach(() => { + mockBranding = { plugins: [] } as unknown as AdminBranding; + vi.restoreAllMocks(); +}); + +describe("plugin detail, not installed", () => { + /** + * The separating case. Before this view existed the page searched only + * `branding.plugins`, so every catalogue entry the directory linked to + * rendered "Plugin not found" — a dead end on the primary discovery path. + */ + it("shows the catalogue entry instead of a not-found page", async () => { + renderDetail("nextlyhq-plugin-seo"); + + expect( + await screen.findByRole("heading", { name: "SEO" }) + ).toBeInTheDocument(); + expect(screen.queryByText("Plugin not found")).toBeNull(); + }); + + it("offers the install command and the config line", async () => { + renderDetail("nextlyhq-plugin-seo"); + + expect( + await screen.findByText("pnpm add @nextlyhq/plugin-seo") + ).toBeInTheDocument(); + expect(screen.getByText(/plugins: \[seoPlugin\(/)).toBeInTheDocument(); + }); + + /** + * The invariant the whole surface rests on: verified content only ever + * appears in the verified section. Nothing here has been observed running, + * so the sections that report what a plugin contributes must be absent. + */ + it("reports no contributions, because none have been observed", async () => { + renderDetail("nextlyhq-plugin-seo"); + + await screen.findByRole("heading", { name: "SEO" }); + expect(screen.queryByText("Permissions")).toBeNull(); + expect(screen.queryByText("API routes")).toBeNull(); + }); + + it("still says not found for a slug no plugin and no entry matches", async () => { + renderDetail("no-such-plugin"); + + expect(await screen.findByText("Plugin not found")).toBeInTheDocument(); + }); + + /** + * Installed metadata is observed; the catalogue is a claim. When both + * describe the same package the observed one decides which view renders, + * otherwise installing a plugin would leave it looking uninstalled. + */ + it("prefers the installed plugin over the catalogue entry", async () => { + mockBranding = { + plugins: [{ name: "@nextlyhq/plugin-seo", version: "1.0.0" }], + } as unknown as AdminBranding; + + renderDetail("nextlyhq-plugin-seo"); + + expect(await screen.findByText("v1.0.0")).toBeInTheDocument(); + expect(screen.queryByText("pnpm add @nextlyhq/plugin-seo")).toBeNull(); + }); +}); diff --git a/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx index c669474b44..5e1ceab998 100644 --- a/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx @@ -1,3 +1,4 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; @@ -101,8 +102,21 @@ describe("PluginDetailPage", () => { expect(screen.getByText(/its behavior does not load/i)).toBeInTheDocument(); }); - it("renders a not-found state for an unknown slug", () => { - render(); - expect(screen.getByText("Plugin not found")).toBeInTheDocument(); + /** + * Not found now means neither installed NOR in the plugin directory, so this + * path reads the catalogue and needs a query client. A slug that IS in the + * catalogue renders the uninstalled view instead, covered in + * `not-installed-detail.test.tsx`. + */ + it("renders a not-found state for a slug nothing knows", async () => { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + render( + + + + ); + expect(await screen.findByText("Plugin not found")).toBeInTheDocument(); }); }); From fcce38ead1df45c2abaa3ee1ee8d1b3489c6ecf0 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 08:51:41 +0500 Subject: [PATCH 05/16] fix(admin): complete the plugin setup recipe and gate absence on loaded metadata The catalogue's install instructions showed a plugins-array entry without the import that introduces its binding, so a copied recipe referenced an undeclared identifier. The detail page also read a still-loading admin-meta response as proof a plugin was absent, and the sidebar overview stopped matching a plugin's own pages. Icon names in the catalogue are now checked against the icon barrel, which is what surfaced Layout never being re-exported. --- .../dashboard/DynamicPluginNav.test.tsx | 55 ++++++++++++++++ .../features/dashboard/DynamicPluginNav.tsx | 13 ++-- packages/admin/src/components/icons/index.ts | 3 + packages/admin/src/components/icons/names.ts | 25 ++++++++ .../shared/plugin-icon/index.test.tsx | 26 ++++++++ .../context/providers/BrandingProvider.tsx | 59 +++++++++++++++-- .../resolve-catalogue-presentation.test.ts | 17 +++-- .../admin/src/lib/plugins/registry/entries.ts | 16 +++-- .../lib/plugins/registry/install-command.ts | 37 +++++++++-- .../admin/src/lib/plugins/registry/types.ts | 36 +++++++++-- .../pages/dashboard/plugins/[slug].test.tsx | 3 + .../src/pages/dashboard/plugins/[slug].tsx | 64 +++++++++++++++++-- .../plugins/components/NotInstalledPlugin.tsx | 28 ++++++-- .../plugins/not-installed-detail.test.tsx | 41 +++++++++++- .../dashboard/plugins/plugin-detail.test.tsx | 2 + 15 files changed, 380 insertions(+), 45 deletions(-) create mode 100644 packages/admin/src/components/icons/names.ts diff --git a/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx b/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx index 19070770c8..13d28b6766 100644 --- a/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx +++ b/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx @@ -44,9 +44,34 @@ vi.mock("@admin/hooks/queries", () => ({ })); import { DynamicPluginNav } from "@admin/components/features/dashboard/DynamicPluginNav"; import { SidebarProvider } from "@admin/components/layout/sidebar"; +import { useSidebarNavigation } from "@admin/hooks/useSidebarNavigation"; const noop = () => false; +/** + * Renders the panel with the REAL `isActive`, for the one pathname given. + * + * The sidebar's matcher rather than one written here: a copy would agree on + * the day it was written and then answer for itself, and what is under test is + * how the overview composes with that matcher, not whether the composition can + * be restated. + * + * Empty navigation items because `isActive` reads only the pathname; the items + * feed accordion state, which nothing here asserts on. + */ +function ActiveStateHarness({ pathname }: { pathname: string }) { + const { isActive } = useSidebarNavigation([], pathname); + return ; +} + +function renderAt(pathname: string) { + return render( + + + + ); +} + // The real provider rather than a mocked `useSidebar`: the component reads // collapsed state from it, so a stub would let a change to that contract pass // unnoticed here. @@ -192,6 +217,36 @@ describe("DynamicPluginNav", () => { }); }); +/** + * Which sidebar entry the overview claims. + * + * `/admin/plugins` is a prefix of both the directory and every plugin's own + * page, and the three cases pull in opposite directions: claiming the whole + * subtree highlights the overview and Browse together on the directory, while + * demanding an exact match leaves a plugin's own page with nothing selected. + */ +describe("DynamicPluginNav overview active state", () => { + function overviewActive() { + return screen + .getByRole("link", { name: /installed plugins/i }) + .getAttribute("data-active"); + } + + it.each([ + ["/admin/plugins", "true"], + ["/admin/plugins/acme-forms", "true"], + ["/admin/plugins/acme-forms/settings", "true"], + // The sibling page owns itself; Browse is the entry that highlights here. + ["/admin/plugins/browse", "false"], + ])("at %s the overview is active=%s", (pathname, expected) => { + mockBranding = { plugins: [] } as unknown as AdminBranding; + + renderAt(pathname); + + expect(overviewActive()).toBe(expected); + }); +}); + describe("DynamicPluginNav expanded", () => { /** * A group is expandable when it retains a collection, and only then. diff --git a/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx b/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx index 9c85dce082..f62ef1c9c7 100644 --- a/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx +++ b/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx @@ -54,12 +54,15 @@ interface DynamicPluginNavProps { function PluginOverviewLink({ isActive, }: { - isActive: (href?: string, exactMatch?: boolean) => boolean; + isActive: (href?: string) => boolean; }) { - // Exact: `/admin/plugins` is a prefix of `/admin/plugins/browse`, and the - // default match treats every descendant as active, so both sibling entries - // would highlight at once for anyone on the directory. - const active = isActive("/admin/plugins", true); + // `/admin/plugins` is a prefix of `/admin/plugins/browse`, so claiming the + // whole subtree would highlight both sibling entries at once on the + // directory. Subtracting the sibling rather than demanding an exact match: + // a plugin's own pages — `/admin/plugins/` and its settings — are + // descendants too, and under an exact match they left the secondary + // navigation with nothing selected at all. + const active = isActive(ROUTES.PLUGINS) && !isActive(ROUTES.PLUGIN_BROWSE); return ( diff --git a/packages/admin/src/components/icons/index.ts b/packages/admin/src/components/icons/index.ts index b7089ae806..4931e48e29 100644 --- a/packages/admin/src/components/icons/index.ts +++ b/packages/admin/src/components/icons/index.ts @@ -108,6 +108,7 @@ export { Italic, // Rich text editor Key, Laptop, + Layout, // Plugin appearance: the icon @nextlyhq/plugin-page-builder declares LayoutGrid, // Collection Builder: blocks field Layers, LayoutDashboard, @@ -219,3 +220,5 @@ export const Discord = ({ size = 24, ...props }: LucideProps) => { }) ); }; + +export type { AdminIconName } from "./names"; diff --git a/packages/admin/src/components/icons/names.ts b/packages/admin/src/components/icons/names.ts new file mode 100644 index 0000000000..e081fc3c95 --- /dev/null +++ b/packages/admin/src/components/icons/names.ts @@ -0,0 +1,25 @@ +/** + * The set of icon names the barrel can render. + * + * Its own module rather than a line in the barrel, because the type has to + * refer to the barrel's own exports and a module cannot describe itself + * without the inline `import()` form that this package's lint rules forbid. + * + * @module components/icons/names + */ +import type * as Icons from "./index"; + +/** + * Every icon name this admin can render, derived from the exports rather than + * listed beside them, so a name and the export it refers to cannot disagree. + * + * Curated data that picks an icon — the plugin catalogue — types its icon + * field as this, and a name the barrel does not carry stops compiling instead + * of resolving to nothing and silently rendering the caller's generic + * fallback. + * + * Not for icon names that arrive at runtime: a third-party plugin declares + * `appearance.icon` as a free string this admin cannot constrain, so that path + * keeps its fallback. + */ +export type AdminIconName = keyof typeof Icons; diff --git a/packages/admin/src/components/shared/plugin-icon/index.test.tsx b/packages/admin/src/components/shared/plugin-icon/index.test.tsx index 743b7d0b7a..e5b2293c1f 100644 --- a/packages/admin/src/components/shared/plugin-icon/index.test.tsx +++ b/packages/admin/src/components/shared/plugin-icon/index.test.tsx @@ -7,6 +7,7 @@ import { fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it } from "vitest"; +import * as Icons from "@admin/components/icons"; import { PluginIcon } from "@admin/components/shared/plugin-icon"; import type { PluginMetadata } from "@admin/types/branding"; @@ -17,6 +18,31 @@ const withAsset = (src: string, icon?: string) => >; describe("PluginIcon", () => { + /** + * The glyph `@nextlyhq/plugin-page-builder` declares. A name the barrel does + * not re-export resolves to nothing and falls through to the caller's + * fallback, so the plugin's own icon disappears everywhere in the admin + * while every surface still looks intact. + */ + it("renders a first-party plugin's declared glyph rather than the fallback", () => { + const { container } = render( + + ); + + // Compared against what the barrel's own `Layout` draws rather than a + // pinned class name: lucide ships `Layout` as an alias, so the rendered + // class is the target's and hard-coding it would tie this to which icon + // lucide happens to alias it to. + const expected = render().container.querySelector("svg"); + expect(container.querySelector("svg")?.getAttribute("class")).toBe( + expected?.getAttribute("class") + ); + expect(container.querySelector(".lucide-package")).toBeNull(); + }); + it("renders a declared asset", () => { render(); expect(screen.getByRole("presentation", { hidden: true })).toBeTruthy(); diff --git a/packages/admin/src/context/providers/BrandingProvider.tsx b/packages/admin/src/context/providers/BrandingProvider.tsx index b53560a37c..969a84cf4a 100644 --- a/packages/admin/src/context/providers/BrandingProvider.tsx +++ b/packages/admin/src/context/providers/BrandingProvider.tsx @@ -2,7 +2,7 @@ import { useQuery } from "@tanstack/react-query"; import type React from "react"; -import { createContext, useContext, useEffect } from "react"; +import { createContext, useContext, useEffect, useMemo } from "react"; import { publicApi } from "../../lib/api/publicApi"; import { @@ -18,10 +18,47 @@ import type { // Context // ============================================================================ -const BrandingContext = createContext(undefined); +/** + * Whether the admin-meta request has answered yet, alongside its answer. + * + * The two are held together because most readers want only the answer, and + * one reader — a page that treats a plugin's ABSENCE from the list as a fact + * about the project — needs to know the list has actually arrived. Before it + * does, `branding` is undefined, which is indistinguishable from a project + * that has no plugins. + */ +interface BrandingState { + branding: AdminBranding | undefined; + /** True until the admin-meta query settles, either way. */ + isPending: boolean; + /** True when it settled without an answer, so absence proves nothing. */ + isError: boolean; +} + +const BrandingContext = createContext(undefined); export function useBranding(): AdminBranding { - return useContext(BrandingContext) ?? {}; + return useContext(BrandingContext)?.branding ?? {}; +} + +/** + * The admin-meta request's state, for readers that draw a conclusion from + * something being MISSING from branding. + * + * Derived from the same context entry `useBranding` reads, rather than from a + * second query: two `useQuery` calls on one key would report their states + * independently and could disagree about whether the data has arrived. + */ +export function useBrandingStatus(): Omit { + const state = useContext(BrandingContext); + // No provider above: nothing is loading and nothing failed, which is the + // same shape a settled empty response has. A reader outside the provider is + // already reading `{}` from `useBranding`, and reporting "still pending" + // here would hang it forever. + return { + isPending: state?.isPending ?? false, + isError: state?.isError ?? false, + }; } // ============================================================================ @@ -136,7 +173,11 @@ interface BrandingProviderProps { } export function BrandingProvider({ children }: BrandingProviderProps) { - const { data: fetchedData } = useQuery({ + const { + data: fetchedData, + isPending, + isError, + } = useQuery({ queryKey: ["admin-meta"], queryFn: () => publicApi.get("/admin-meta"), // Refetch periodically to pick up changes to custom sidebar groups, @@ -148,8 +189,16 @@ export function BrandingProvider({ children }: BrandingProviderProps) { useColorInjection(fetchedData?.colors); useFaviconInjection(fetchedData?.favicon); + // Memoized because the value is now an object built here rather than the + // query's own stable `data` reference: without this every consumer of the + // context re-renders on each render of this provider. + const value = useMemo( + () => ({ branding: fetchedData, isPending, isError }), + [fetchedData, isPending, isError] + ); + return ( - + {children} ); diff --git a/packages/admin/src/lib/plugins/registry/__tests__/resolve-catalogue-presentation.test.ts b/packages/admin/src/lib/plugins/registry/__tests__/resolve-catalogue-presentation.test.ts index b495bf667f..5cf57822ed 100644 --- a/packages/admin/src/lib/plugins/registry/__tests__/resolve-catalogue-presentation.test.ts +++ b/packages/admin/src/lib/plugins/registry/__tests__/resolve-catalogue-presentation.test.ts @@ -16,8 +16,11 @@ const ENTRY: RegistryPlugin = { description: "The catalogue's description", author: "Acme", category: "content", - icon: { lucide: "CatalogueGlyph" }, - configSnippet: "plugins: [thing()]", + // A real barrel export, unlike `PluginGlyph` on the installed side below: a + // catalogue entry's glyph is type-checked against the icon barrel, while a + // plugin declares its own as a free string this admin cannot constrain. + icon: { lucide: "Archive" }, + config: { exportName: "thing", callArgs: "" }, }; function installed( @@ -31,7 +34,7 @@ describe("resolveCataloguePresentation", () => { it("uses the catalogue when the plugin is not installed", () => { const p = resolveCataloguePresentation(ENTRY, undefined); - expect(p.icon).toEqual({ kind: "lucide", name: "CatalogueGlyph" }); + expect(p.icon).toEqual({ kind: "lucide", name: "Archive" }); expect(p.description).toBe("The catalogue's description"); expect(p.isInstalled).toBe(false); }); @@ -59,7 +62,7 @@ describe("resolveCataloguePresentation", () => { ); expect(p.description).toBe("The plugin's own description"); - expect(p.icon).toEqual({ kind: "lucide", name: "CatalogueGlyph" }); + expect(p.icon).toEqual({ kind: "lucide", name: "Archive" }); }); it("keeps the catalogue text when an installed plugin declares none", () => { @@ -96,7 +99,7 @@ describe("resolveCataloguePresentation", () => { it("lets an installed glyph outrank a catalogue asset", () => { const withAsset: RegistryPlugin = { ...ENTRY, - icon: { lucide: "CatalogueGlyph", asset: "/catalogue.svg" }, + icon: { lucide: "Archive", asset: "/catalogue.svg" }, }; // Positive control: with nothing installed the catalogue asset IS chosen, @@ -117,7 +120,7 @@ describe("resolveCataloguePresentation", () => { it("skips every asset for a surface that cannot render one", () => { const withAsset: RegistryPlugin = { ...ENTRY, - icon: { lucide: "CatalogueGlyph", asset: "/catalogue.svg" }, + icon: { lucide: "Archive", asset: "/catalogue.svg" }, }; const p = resolveCataloguePresentation( @@ -126,6 +129,6 @@ describe("resolveCataloguePresentation", () => { { allowAsset: false } ); - expect(p.icon).toEqual({ kind: "lucide", name: "CatalogueGlyph" }); + expect(p.icon).toEqual({ kind: "lucide", name: "Archive" }); }); }); diff --git a/packages/admin/src/lib/plugins/registry/entries.ts b/packages/admin/src/lib/plugins/registry/entries.ts index 142bdee413..8f1db5f299 100644 --- a/packages/admin/src/lib/plugins/registry/entries.ts +++ b/packages/admin/src/lib/plugins/registry/entries.ts @@ -31,7 +31,7 @@ export const REGISTRY_ENTRIES: RegistryPlugin[] = [ category: "content", tags: ["blocks", "editor", "pages"], icon: { lucide: "Layout" }, - configSnippet: "plugins: [pageBuilder()]", + config: { exportName: "pageBuilder", callArgs: "" }, links: { homepage: "https://nextlyhq.com", repository: "https://github.com/nextlyhq/nextly", @@ -45,10 +45,10 @@ export const REGISTRY_ENTRIES: RegistryPlugin[] = [ category: "forms", tags: ["forms", "submissions"], icon: { lucide: "FileText" }, - // `formBuilderPlugin`, not `formBuilder()`: the factory returns a + // `callArgs: null` — the export goes in uncalled. The factory returns a // FormBuilderPluginResult whose definition is at `.plugin`, and the package // exports the unwrapped value for exactly this use. - configSnippet: "plugins: [formBuilderPlugin]", + config: { exportName: "formBuilderPlugin", callArgs: null }, links: { homepage: "https://nextlyhq.com", repository: "https://github.com/nextlyhq/nextly", @@ -63,9 +63,13 @@ export const REGISTRY_ENTRIES: RegistryPlugin[] = [ category: "seo", tags: ["seo", "meta"], icon: { lucide: "Search" }, - // `seoPlugin`, and `collections` is required: the plugin adds the SEO group - // only to the collections it is given, so a bare call does not type-check. - configSnippet: 'plugins: [seoPlugin({ collections: ["posts"] })]', + // `collections` is required rather than illustrative: the plugin adds the + // SEO group only to the collections it is given, so a bare call does not + // type-check. + config: { + exportName: "seoPlugin", + callArgs: '{ collections: ["posts"] }', + }, links: { homepage: "https://nextlyhq.com", repository: "https://github.com/nextlyhq/nextly", diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts index 936bb0cf03..00449a5eee 100644 --- a/packages/admin/src/lib/plugins/registry/install-command.ts +++ b/packages/admin/src/lib/plugins/registry/install-command.ts @@ -1,12 +1,15 @@ /** - * The shell command that installs a catalogue plugin. + * The three lines that add a catalogue plugin to a project: the shell command + * that installs it, and the two edits to nextly.config.ts. * - * Derived from the entry's `id` rather than stored beside it, so a package - * rename cannot leave the install command fetching the old name while the - * detail page joins installed state on the new one. + * All derived from the entry rather than stored beside it, so a package rename + * cannot leave the install command fetching the old name while the detail page + * joins installed state on the new one, and an entry cannot name a binding its + * import does not bring in. * * @module lib/plugins/registry/install-command */ +import type { RegistryPlugin } from "./types"; /** * Package managers a Nextly project can be installed with. @@ -39,3 +42,29 @@ export function installCommand( ): string { return `${manager} ${ADD_SUBCOMMAND[manager]} ${packageName}`; } + +/** + * The import that brings the plugin's binding into nextly.config.ts. + * + * Installing the package does not introduce the identifier, so the entry below + * references nothing without this line. It is the reader's first edit, and + * omitting it hands them a recipe that does not compile. + */ +export function importStatement(plugin: RegistryPlugin): string { + return `import { ${plugin.config.exportName} } from "${plugin.id}";`; +} + +/** + * The entry to add inside `plugins: [...]`. + * + * `callArgs` decides whether the binding is called at all: `null` means the + * package exports a ready-made plugin value, which is how + * `@nextlyhq/plugin-form-builder` ships one — its factory returns a result + * object whose definition sits at `.plugin`, so calling it here would be + * wrong rather than merely verbose. + */ +export function pluginsArrayEntry(plugin: RegistryPlugin): string { + const { exportName, callArgs } = plugin.config; + const value = callArgs === null ? exportName : `${exportName}(${callArgs})`; + return `plugins: [${value}]`; +} diff --git a/packages/admin/src/lib/plugins/registry/types.ts b/packages/admin/src/lib/plugins/registry/types.ts index d70fe37f53..cec1f54baa 100644 --- a/packages/admin/src/lib/plugins/registry/types.ts +++ b/packages/admin/src/lib/plugins/registry/types.ts @@ -1,3 +1,5 @@ +import type { AdminIconName } from "@admin/components/icons"; + import type { PluginCategory } from "../plugin-categories"; /** @@ -22,16 +24,36 @@ export interface RegistryPlugin { author: string; category: PluginCategory; tags?: string[]; - icon: { lucide: string; asset?: string }; /** - * The line to add inside `plugins: [...]` in nextly.config.ts. + * `lucide` is checked against the icon barrel, unlike a plugin's own + * `appearance.icon`: this catalogue is ours, so a name nothing exports is a + * mistake we can refuse at compile time rather than a third party's string + * we have to tolerate at runtime. + */ + icon: { lucide: AdminIconName; asset?: string }; + /** + * What the reader has to write in nextly.config.ts, in the two parts they + * write it in: an import at the top of the file and an entry in the plugins + * array. * - * No package name beside it: that is `id`, and storing it twice means a - * rename can update one and leave the other, so the detail page would join - * on the new name while the install command still fetched the old one. - * Derive the command with `installCommand()`. + * Held as the binding and its arguments rather than as finished lines, + * because the two mention the same identifier and a stored pair can + * disagree — an entry naming a symbol the import does not bring in is a + * recipe that does not compile. Neither part repeats the package name + * either; that is `id`, and `importStatement()` reads it from there so a + * rename cannot leave the import fetching the old package while the detail + * page joins installed state on the new one. */ - configSnippet: string; + config: { + /** The binding the package exports and the plugins array references. */ + exportName: string; + /** + * The arguments as written inside the call: `""` for a call that takes + * none, or `null` when the export goes into the array uncalled. The two + * are different facts about the package's API, not two spellings of one. + */ + callArgs: string | null; + }; links?: { homepage?: string; repository?: string; docs?: string }; } diff --git a/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx b/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx index 8aa87c8729..bcc2a61230 100644 --- a/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx @@ -12,6 +12,9 @@ import { renderWithProviders } from "../../../__tests__/utils"; let mockBranding: AdminBranding | undefined; vi.mock("@admin/context/providers/BrandingProvider", () => ({ useBranding: () => mockBranding, + // Settled with an answer: every case here is about a plugin the branding + // payload already carries. + useBrandingStatus: () => ({ isPending: false, isError: false }), })); import PluginDetailPage from "./[slug]"; diff --git a/packages/admin/src/pages/dashboard/plugins/[slug].tsx b/packages/admin/src/pages/dashboard/plugins/[slug].tsx index 413f4cde54..1b325d8d42 100644 --- a/packages/admin/src/pages/dashboard/plugins/[slug].tsx +++ b/packages/admin/src/pages/dashboard/plugins/[slug].tsx @@ -1,7 +1,7 @@ "use client"; import { Badge } from "@nextlyhq/ui"; -import { useSuspenseQuery } from "@tanstack/react-query"; +import { useQueryClient, useSuspenseQuery } from "@tanstack/react-query"; import { BookOpen, @@ -11,6 +11,7 @@ import { Globe, LayoutDashboard, Layers, + Loader2, Menu as MenuIcon, Package, Route, @@ -19,12 +20,18 @@ import { } from "@admin/components/icons"; import { PageContainer } from "@admin/components/layout/page-container"; import { Breadcrumbs } from "@admin/components/shared"; -import { PageErrorFallback } from "@admin/components/shared/error-fallbacks"; +import { + PageErrorFallback, + SectionErrorFallback, +} from "@admin/components/shared/error-fallbacks"; import { PluginIcon } from "@admin/components/shared/plugin-icon"; import { QueryErrorBoundary } from "@admin/components/shared/query-error-boundary"; import { Link } from "@admin/components/ui/link"; import { ROUTES, buildRoute } from "@admin/constants/routes"; -import { useBranding } from "@admin/context/providers/BrandingProvider"; +import { + useBranding, + useBrandingStatus, +} from "@admin/context/providers/BrandingProvider"; import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; import { pluginSlug } from "@admin/lib/plugins/plugin-slug"; import { staticRegistrySource } from "@admin/lib/plugins/registry/static-source"; @@ -132,8 +139,49 @@ function UninstalledOrMissing({ activeSlug }: { activeSlug?: string }) { ); } +/** + * Shown while the installed-plugin list is still in flight. + * + * The alternative — rendering the catalogue view immediately and correcting it + * when the list lands — flashes install instructions at someone who already + * has the plugin, and reads as an answer rather than as a wait. + */ +function LoadingInstalledPlugins() { + return ( +
+ + Loading plugin… +
+ ); +} + +/** + * Shown when admin-meta failed, so whether this plugin is installed is + * unknown. + * + * Not the catalogue view: that one states the plugin is absent, which is a + * claim this page cannot make when the request that would have told it failed. + */ +function InstalledPluginsUnavailable() { + const queryClient = useQueryClient(); + + return ( + { + void queryClient.invalidateQueries({ queryKey: ["admin-meta"] }); + }} + /> + ); +} + function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { const branding = useBranding(); + const { isPending, isError } = useBrandingStatus(); const plugins = branding?.plugins ?? []; const plugin = activeSlug ? plugins.find(p => pluginSlug(p.name) === activeSlug) @@ -143,7 +191,15 @@ function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { // one decides. Only when the project has no such plugin is the catalogue // consulted, and that lookup lives inside `UninstalledOrMissing` so its query // is not a dependency of rendering an installed plugin. - if (!plugin) return ; + // + // Absence is only a fact once admin-meta has answered. Until then the list is + // empty for a reason that says nothing about the project, and reading it as + // "not installed" tells someone who HAS this plugin to go and install it. + if (!plugin) { + if (isPending) return ; + if (isError) return ; + return ; + } const title = plugin.appearance?.label ?? plugin.name; diff --git a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx index 8be8156d85..666250c474 100644 --- a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx +++ b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx @@ -9,7 +9,9 @@ import { PluginIcon } from "@admin/components/shared/plugin-icon"; import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; import { PACKAGE_MANAGERS, + importStatement, installCommand, + pluginsArrayEntry, type PackageManager, } from "@admin/lib/plugins/registry/install-command"; import type { RegistryPlugin } from "@admin/lib/plugins/registry/types"; @@ -64,8 +66,8 @@ function CopyLine({ value, label }: { value: string; label: string }) { * Deliberately thin, and the reason is the invariant the whole surface is * built on: verified content only ever appears in the verified section. Nothing * here has been observed running, so there is no contributions section, no - * permissions and no routes — only what the catalogue claims, plus the two - * commands that would make the claims checkable. + * permissions and no routes — only what the catalogue claims, plus the three + * lines that would make the claims checkable. * * @module pages/dashboard/plugins/components/NotInstalledPlugin */ @@ -111,8 +113,11 @@ export function NotInstalledPlugin({

Add it to your project

- Two steps: install the package, then register it in your Nextly - config. Nextly picks it up on the next start. + Install the package, then make two edits in{" "} + nextly.config.ts: import the + plugin at the top of the file, and add it to the{" "} + plugins array. Nextly picks it up + on the next start.

@@ -135,8 +140,19 @@ export function NotInstalledPlugin({ ))} - - + {/* Three lines because the reader makes three edits, each copyable on + its own: the import and the array entry land in different places in + the same file, so joining them into one block would be a snippet + nobody can paste anywhere. */} + + + {/* States the boundary rather than leaving it implied: a reader looking diff --git a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx index 39dfd80043..05c6551225 100644 --- a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx @@ -11,9 +11,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { AdminBranding } from "@admin/types/branding"; let mockBranding: AdminBranding = { plugins: [] } as unknown as AdminBranding; +let mockBrandingStatus = { isPending: false, isError: false }; vi.mock("@admin/context/providers/BrandingProvider", () => ({ useBranding: () => mockBranding, + useBrandingStatus: () => mockBrandingStatus, })); import PluginDetailPage from "./[slug]"; @@ -31,6 +33,7 @@ function renderDetail(slug: string) { afterEach(() => { mockBranding = { plugins: [] } as unknown as AdminBranding; + mockBrandingStatus = { isPending: false, isError: false }; vi.restoreAllMocks(); }); @@ -49,15 +52,51 @@ describe("plugin detail, not installed", () => { expect(screen.queryByText("Plugin not found")).toBeNull(); }); - it("offers the install command and the config line", async () => { + /** + * All three lines, because the recipe only works as a set: installing the + * package does not introduce the binding, so an array entry shown without + * its import names an identifier that does not exist in the file. + */ + it("offers the install command, the import, and the array entry", async () => { renderDetail("nextlyhq-plugin-seo"); expect( await screen.findByText("pnpm add @nextlyhq/plugin-seo") ).toBeInTheDocument(); + expect( + screen.getByText('import { seoPlugin } from "@nextlyhq/plugin-seo";') + ).toBeInTheDocument(); expect(screen.getByText(/plugins: \[seoPlugin\(/)).toBeInTheDocument(); }); + /** + * A plugin the project HAS, on a page opened cold. Until admin-meta answers + * the installed list is empty for a reason that says nothing about the + * project, and reading that as "not installed" tells the reader to install + * something they already have. + */ + it("does not offer to install while the installed list is still loading", async () => { + mockBrandingStatus = { isPending: true, isError: false }; + + renderDetail("nextlyhq-plugin-seo"); + + expect(await screen.findByRole("status")).toBeInTheDocument(); + expect(screen.queryByText("pnpm add @nextlyhq/plugin-seo")).toBeNull(); + expect(screen.queryByText("Plugin not found")).toBeNull(); + }); + + /** Same reasoning, permanent: a failed request is not evidence of absence. */ + it("does not offer to install when the installed list could not be loaded", async () => { + mockBrandingStatus = { isPending: false, isError: true }; + + renderDetail("nextlyhq-plugin-seo"); + + expect( + await screen.findByText("Could not load your installed plugins") + ).toBeInTheDocument(); + expect(screen.queryByText("pnpm add @nextlyhq/plugin-seo")).toBeNull(); + }); + /** * The invariant the whole surface rests on: verified content only ever * appears in the verified section. Nothing here has been observed running, diff --git a/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx index 5e1ceab998..5ebf1dd037 100644 --- a/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx @@ -44,6 +44,8 @@ const plugins: PluginMetadata[] = [ vi.mock("@admin/context/providers/BrandingProvider", () => ({ useBranding: () => ({ plugins }), + // Settled with an answer, so a slug missing from `plugins` really is absent. + useBrandingStatus: () => ({ isPending: false, isError: false }), })); describe("PluginDetailPage", () => { From d46db1672a6ba951efd81af15a4ce5bd720e444c Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 10:16:52 +0500 Subject: [PATCH 06/16] fix(admin): pin plugin installs to the running admin release Every first-party plugin declares an exact nextly peer for its own release, so an unpinned install on a project that is not on the newest one resolves a plugin whose peer names a core that project does not have. The admin bundle now carries its own version as a build-time constant, mirroring how core resolves __NEXTLY_CORE_VERSION__, and the install command names that release. --- packages/admin/src/lib/admin-version.ts | 29 +++++ .../__tests__/install-command.test.ts | 104 ++++++++++++++++++ .../lib/plugins/registry/install-command.ts | 30 ++++- .../plugins/components/NotInstalledPlugin.tsx | 3 +- .../plugins/not-installed-detail.test.tsx | 22 +++- packages/admin/tsup.config.ts | 11 ++ packages/admin/vitest.config.ts | 11 ++ 7 files changed, 197 insertions(+), 13 deletions(-) create mode 100644 packages/admin/src/lib/admin-version.ts create mode 100644 packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts diff --git a/packages/admin/src/lib/admin-version.ts b/packages/admin/src/lib/admin-version.ts new file mode 100644 index 0000000000..9f089e443a --- /dev/null +++ b/packages/admin/src/lib/admin-version.ts @@ -0,0 +1,29 @@ +/** + * The release this admin bundle belongs to. + * + * Injected at build time from `package.json` by the tsup and vitest `define` + * blocks, mirroring how core resolves `__NEXTLY_CORE_VERSION__`. No runtime + * `package.json` read as core has: this bundle targets the browser, where + * there is no module resolver to fall back to. + * + * @module lib/admin-version + */ + +declare const __NEXTLY_ADMIN_VERSION__: string | undefined; + +/** + * The concrete admin version (e.g. `"0.0.2-alpha.57"`), or `undefined` when + * the constant was not injected. + * + * `undefined` rather than a `"0.0.0"` sentinel, because the only caller uses + * this to pin a package specifier: a sentinel would produce an install command + * for a version that does not exist, which is worse than the unpinned command + * it replaced. Callers have to handle not knowing. + */ +export function adminVersion(): string | undefined { + const injected = + typeof __NEXTLY_ADMIN_VERSION__ !== "undefined" + ? __NEXTLY_ADMIN_VERSION__ + : undefined; + return injected && injected.length > 0 ? injected : undefined; +} diff --git a/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts new file mode 100644 index 0000000000..0066c9bcd6 --- /dev/null +++ b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts @@ -0,0 +1,104 @@ +/** + * The three lines a reader copies to add a plugin. + * + * Two properties matter and they are separate: the lines must be derived from + * the entry rather than stored beside it, and the install must name a version. + * An unpinned install is the failure with teeth — every first-party plugin + * declares an exact `nextly` peer for its own release, so resolving `latest` + * on a project that is not on the newest one is a peer conflict on npm and a + * startup compatibility failure elsewhere. + */ +import { describe, expect, it } from "vitest"; + +import { adminVersion } from "@admin/lib/admin-version"; + +import pkg from "../../../../../package.json"; +import { + importStatement, + installCommand, + pluginsArrayEntry, +} from "../install-command"; +import type { RegistryPlugin } from "../types"; + +function entry(config: RegistryPlugin["config"]): RegistryPlugin { + return { + id: "@acme/thing", + name: "Thing", + description: "d", + author: "Acme", + category: "content", + icon: { lucide: "Archive" }, + config, + }; +} + +describe("installCommand", () => { + it("pins to the given release", () => { + expect(installCommand("@acme/p", "pnpm", "1.2.3")).toBe( + "pnpm add @acme/p@1.2.3" + ); + }); + + /** + * The separating case for the pin. Asserting only that a version appears + * would pass on a build that injected a sentinel, and a command naming a + * version that was never published is worse than one naming none. + */ + it("omits the version rather than inventing one when it is unknown", () => { + expect(installCommand("@acme/p", "pnpm", undefined)).toBe( + "pnpm add @acme/p" + ); + }); + + /** + * What the call site actually passes. `adminVersion()` reads a constant the + * build injects from this package.json, so comparing the two confirms the + * injection is wired rather than merely that some string came back — a + * missing `define` would have it return undefined and the command would + * silently go back to resolving `latest`. + */ + it("names this package's own release when given the injected version", () => { + expect(installCommand("@acme/p", "pnpm", adminVersion())).toBe( + `pnpm add @acme/p@${pkg.version}` + ); + }); + + it("uses each manager's own add subcommand", () => { + expect(installCommand("@acme/p", "npm", "1.0.0")).toBe( + "npm install @acme/p@1.0.0" + ); + expect(installCommand("@acme/p", "yarn", "1.0.0")).toBe( + "yarn add @acme/p@1.0.0" + ); + expect(installCommand("@acme/p", "bun", "1.0.0")).toBe( + "bun add @acme/p@1.0.0" + ); + }); +}); + +describe("config lines", () => { + it("imports the binding the array entry references", () => { + const plugin = entry({ exportName: "thing", callArgs: "" }); + + // Asserted together, because the defect they guard is disagreement: an + // entry naming a symbol the import does not bring in compiles nowhere. + expect(importStatement(plugin)).toBe( + 'import { thing } from "@acme/thing";' + ); + expect(pluginsArrayEntry(plugin)).toBe("plugins: [thing()]"); + }); + + it("leaves an uncalled export uncalled", () => { + expect( + pluginsArrayEntry(entry({ exportName: "thing", callArgs: null })) + ).toBe("plugins: [thing]"); + }); + + it("places required arguments inside the call", () => { + expect( + pluginsArrayEntry( + entry({ exportName: "thing", callArgs: '{ collections: ["posts"] }' }) + ) + ).toBe('plugins: [thing({ collections: ["posts"] })]'); + }); +}); diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts index 00449a5eee..bd5a877fbc 100644 --- a/packages/admin/src/lib/plugins/registry/install-command.ts +++ b/packages/admin/src/lib/plugins/registry/install-command.ts @@ -30,17 +30,35 @@ const ADD_SUBCOMMAND: Record = { }; /** - * `installCommand("@acme/p")` → `"pnpm add @acme/p"`. + * `installCommand("@acme/p", "pnpm", "1.2.3")` → `"pnpm add @acme/p@1.2.3"`. * - * Defaults to pnpm, which is what Nextly's own docs and scaffolder use, while - * staying answerable for the other three so a reader on npm is not handed a - * command their project cannot run. + * Answerable for all four managers, so a reader on npm is not handed a command + * their project cannot run. + * + * Pinned to the running admin's release, and that is not a nicety. Every + * first-party plugin declares an EXACT `nextly` peer for its own release, so + * an unpinned install on a project that is not on the newest one resolves a + * plugin whose peer names a core the project does not have: npm rejects it, + * and a manager that installs it anyway produces a plugin core refuses to load + * at startup. Admin's own version is the release the project is on, since the + * whole train publishes in lockstep and this bundle came from that install. + * + * Unpinned when `version` is undefined — a build that did not inject the + * constant. Naming a version that may not exist would be a worse answer than + * declining to name one. + * + * `version` is required rather than defaulted to `adminVersion()`, so a caller + * has to answer the question instead of inheriting an answer. A default would + * also make the unpinned branch unreachable from a test, since passing + * `undefined` re-triggers the default. */ export function installCommand( packageName: string, - manager: PackageManager = "pnpm" + manager: PackageManager, + version: string | undefined ): string { - return `${manager} ${ADD_SUBCOMMAND[manager]} ${packageName}`; + const specifier = version ? `${packageName}@${version}` : packageName; + return `${manager} ${ADD_SUBCOMMAND[manager]} ${specifier}`; } /** diff --git a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx index 666250c474..0680ac0c4f 100644 --- a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx +++ b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx @@ -6,6 +6,7 @@ import { useState } from "react"; import { Check, Copy } from "@admin/components/icons"; import { PluginIcon } from "@admin/components/shared/plugin-icon"; +import { adminVersion } from "@admin/lib/admin-version"; import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; import { PACKAGE_MANAGERS, @@ -146,7 +147,7 @@ export function NotInstalledPlugin({ nobody can paste anywhere. */} { it("offers the install command, the import, and the array entry", async () => { renderDetail("nextlyhq-plugin-seo"); - expect( - await screen.findByText("pnpm add @nextlyhq/plugin-seo") - ).toBeInTheDocument(); + expect(await screen.findByText(INSTALL_COMMAND)).toBeInTheDocument(); expect( screen.getByText('import { seoPlugin } from "@nextlyhq/plugin-seo";') ).toBeInTheDocument(); @@ -81,7 +91,7 @@ describe("plugin detail, not installed", () => { renderDetail("nextlyhq-plugin-seo"); expect(await screen.findByRole("status")).toBeInTheDocument(); - expect(screen.queryByText("pnpm add @nextlyhq/plugin-seo")).toBeNull(); + expect(screen.queryByText(INSTALL_COMMAND)).toBeNull(); expect(screen.queryByText("Plugin not found")).toBeNull(); }); @@ -94,7 +104,7 @@ describe("plugin detail, not installed", () => { expect( await screen.findByText("Could not load your installed plugins") ).toBeInTheDocument(); - expect(screen.queryByText("pnpm add @nextlyhq/plugin-seo")).toBeNull(); + expect(screen.queryByText(INSTALL_COMMAND)).toBeNull(); }); /** @@ -129,6 +139,6 @@ describe("plugin detail, not installed", () => { renderDetail("nextlyhq-plugin-seo"); expect(await screen.findByText("v1.0.0")).toBeInTheDocument(); - expect(screen.queryByText("pnpm add @nextlyhq/plugin-seo")).toBeNull(); + expect(screen.queryByText(INSTALL_COMMAND)).toBeNull(); }); }); diff --git a/packages/admin/tsup.config.ts b/packages/admin/tsup.config.ts index 7659f286f6..2a117154c8 100644 --- a/packages/admin/tsup.config.ts +++ b/packages/admin/tsup.config.ts @@ -1,4 +1,5 @@ import fs from "fs"; +import { createRequire } from "module"; import path from "path"; import { fileURLToPath } from "url"; @@ -8,6 +9,13 @@ import { defineConfig } from "tsup"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); +// Read at build time so `adminVersion()` can name the release this bundle was +// published in. The browser bundle has no module resolver to read it later. +const require = createRequire(import.meta.url); +const { version: ADMIN_VERSION } = require("./package.json") as { + version: string; +}; + /** * Bundle size target in KB (minified, not gzipped) * Based on industry standards for comprehensive UI libraries: @@ -100,6 +108,9 @@ export default defineConfig(options => [ resolve: true, }, tsconfig: "tsconfig.json", + define: { + __NEXTLY_ADMIN_VERSION__: JSON.stringify(ADMIN_VERSION), + }, external: EXTERNAL_DEPS, noExternal: NO_EXTERNAL_DEPS, splitting: true, diff --git a/packages/admin/vitest.config.ts b/packages/admin/vitest.config.ts index 8853a7cd9b..e740b12f34 100644 --- a/packages/admin/vitest.config.ts +++ b/packages/admin/vitest.config.ts @@ -1,9 +1,20 @@ +import { createRequire } from "module"; + import react from "@vitejs/plugin-react"; import tsconfigPaths from "vite-tsconfig-paths"; import { defineConfig } from "vitest/config"; +// Mirror the tsup `define` so `adminVersion()` resolves under test. +const require = createRequire(import.meta.url); +const { version: adminVersion } = require("./package.json") as { + version: string; +}; + export default defineConfig({ plugins: [react(), tsconfigPaths()], + define: { + __NEXTLY_ADMIN_VERSION__: JSON.stringify(adminVersion), + }, test: { name: "admin", globals: true, From 84f69e7acf9e4a6582e27c798d70c80e722cd44a Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 10:54:04 +0500 Subject: [PATCH 07/16] fix(admin): make the setup lines appendable and move the directory off the plugin namespace The plugins line was the whole property, so a project with plugins already configured could not paste it without dropping them; it is now the single array element. The SEO recipe named a collection the blank template does not have, which throws at startup, and now carries a placeholder that has to be replaced. A failed background refetch no longer reads as admin-meta having no answer. The directory moves to /admin/plugin-directory, where no plugin slug can shadow it. --- .../dashboard/DynamicPluginNav.test.tsx | 17 +++++---- .../features/dashboard/DynamicPluginNav.tsx | 12 +++---- packages/admin/src/constants/routes.ts | 8 ++++- .../context/providers/BrandingProvider.tsx | 23 ++++++++---- .../__tests__/install-command.test.ts | 6 ++-- .../admin/src/lib/plugins/registry/entries.ts | 8 ++++- .../lib/plugins/registry/install-command.ts | 11 ++++-- .../pages/dashboard/plugins/[slug].test.tsx | 2 +- .../src/pages/dashboard/plugins/[slug].tsx | 4 +-- .../pages/dashboard/plugins/browse.test.tsx | 36 ++++++++++++------- .../plugins/not-installed-detail.test.tsx | 15 +++++--- .../dashboard/plugins/plugin-detail.test.tsx | 2 +- packages/admin/src/pages/registry.ts | 9 ++--- 13 files changed, 98 insertions(+), 55 deletions(-) diff --git a/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx b/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx index 13d28b6766..65ac81f685 100644 --- a/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx +++ b/packages/admin/src/components/features/dashboard/DynamicPluginNav.test.tsx @@ -220,10 +220,11 @@ describe("DynamicPluginNav", () => { /** * Which sidebar entry the overview claims. * - * `/admin/plugins` is a prefix of both the directory and every plugin's own - * page, and the three cases pull in opposite directions: claiming the whole - * subtree highlights the overview and Browse together on the directory, while - * demanding an exact match leaves a plugin's own page with nothing selected. + * A plugin's own pages are descendants of `/admin/plugins`, so an exact match + * would leave the secondary navigation with nothing selected while reading a + * plugin's detail page. The directory is the case pulling the other way, and + * it is answered by living under a different prefix rather than by a rule + * here — which is what the last case pins. */ describe("DynamicPluginNav overview active state", () => { function overviewActive() { @@ -236,8 +237,12 @@ describe("DynamicPluginNav overview active state", () => { ["/admin/plugins", "true"], ["/admin/plugins/acme-forms", "true"], ["/admin/plugins/acme-forms/settings", "true"], - // The sibling page owns itself; Browse is the entry that highlights here. - ["/admin/plugins/browse", "false"], + // A plugin that happens to be called "browse" is still a plugin, and its + // page belongs to this entry like any other. + ["/admin/plugins/browse", "true"], + // The directory, which Browse owns. It is outside the prefix, so this is + // false without the overview having to exclude anything. + ["/admin/plugin-directory", "false"], ])("at %s the overview is active=%s", (pathname, expected) => { mockBranding = { plugins: [] } as unknown as AdminBranding; diff --git a/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx b/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx index f62ef1c9c7..fad8200700 100644 --- a/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx +++ b/packages/admin/src/components/features/dashboard/DynamicPluginNav.tsx @@ -56,13 +56,11 @@ function PluginOverviewLink({ }: { isActive: (href?: string) => boolean; }) { - // `/admin/plugins` is a prefix of `/admin/plugins/browse`, so claiming the - // whole subtree would highlight both sibling entries at once on the - // directory. Subtracting the sibling rather than demanding an exact match: - // a plugin's own pages — `/admin/plugins/` and its settings — are - // descendants too, and under an exact match they left the secondary - // navigation with nothing selected at all. - const active = isActive(ROUTES.PLUGINS) && !isActive(ROUTES.PLUGIN_BROWSE); + // The whole subtree, so a plugin's own pages — `/admin/plugins/` and + // its settings — keep the overview selected. Nothing has to be subtracted + // here: the directory lives outside this prefix, so it cannot be caught by + // a descendant match and light both entries at once. + const active = isActive(ROUTES.PLUGINS); return ( diff --git a/packages/admin/src/constants/routes.ts b/packages/admin/src/constants/routes.ts index 33fd581777..be6cf00dcc 100644 --- a/packages/admin/src/constants/routes.ts +++ b/packages/admin/src/constants/routes.ts @@ -107,7 +107,13 @@ export const ROUTES = { // Plugin routes PLUGINS: "/admin/plugins", - PLUGIN_BROWSE: "/admin/plugins/browse", + // Outside the `/admin/plugins/` namespace on purpose. A plugin's detail + // address is `/admin/plugins/`, and a slug is derived from a package + // name that may be any string, so a sibling static page there is a page a + // plugin can be named after — and whichever of the two wins, the other + // becomes unreachable. A different parent removes the collision instead of + // ranking it. + PLUGIN_BROWSE: "/admin/plugin-directory", PLUGIN_DETAIL: "/admin/plugins/[slug]", PLUGIN_SETTINGS: "/admin/plugins/[slug]/settings", } as const; diff --git a/packages/admin/src/context/providers/BrandingProvider.tsx b/packages/admin/src/context/providers/BrandingProvider.tsx index 969a84cf4a..ff3519a24e 100644 --- a/packages/admin/src/context/providers/BrandingProvider.tsx +++ b/packages/admin/src/context/providers/BrandingProvider.tsx @@ -31,8 +31,16 @@ interface BrandingState { branding: AdminBranding | undefined; /** True until the admin-meta query settles, either way. */ isPending: boolean; - /** True when it settled without an answer, so absence proves nothing. */ - isError: boolean; + /** + * True when admin-meta has never produced an answer, so absence proves + * nothing. + * + * Not "the last request failed". Once a response is cached, a failed + * BACKGROUND refetch leaves that answer intact and still valid, and treating + * it as unavailable would replace a correct page with an error for as long + * as the server stays unreachable. + */ + isUnavailable: boolean; } const BrandingContext = createContext(undefined); @@ -57,7 +65,7 @@ export function useBrandingStatus(): Omit { // here would hang it forever. return { isPending: state?.isPending ?? false, - isError: state?.isError ?? false, + isUnavailable: state?.isUnavailable ?? false, }; } @@ -176,7 +184,10 @@ export function BrandingProvider({ children }: BrandingProviderProps) { const { data: fetchedData, isPending, - isError, + // `isLoadingError`, not `isError`: the latter is also true when a + // background refetch fails while a previous response is still cached, and + // that cached response is a perfectly good answer. + isLoadingError, } = useQuery({ queryKey: ["admin-meta"], queryFn: () => publicApi.get("/admin-meta"), @@ -193,8 +204,8 @@ export function BrandingProvider({ children }: BrandingProviderProps) { // query's own stable `data` reference: without this every consumer of the // context re-renders on each render of this provider. const value = useMemo( - () => ({ branding: fetchedData, isPending, isError }), - [fetchedData, isPending, isError] + () => ({ branding: fetchedData, isPending, isUnavailable: isLoadingError }), + [fetchedData, isPending, isLoadingError] ); return ( diff --git a/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts index 0066c9bcd6..0cdbc0de2d 100644 --- a/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts +++ b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts @@ -85,13 +85,13 @@ describe("config lines", () => { expect(importStatement(plugin)).toBe( 'import { thing } from "@acme/thing";' ); - expect(pluginsArrayEntry(plugin)).toBe("plugins: [thing()]"); + expect(pluginsArrayEntry(plugin)).toBe("thing()"); }); it("leaves an uncalled export uncalled", () => { expect( pluginsArrayEntry(entry({ exportName: "thing", callArgs: null })) - ).toBe("plugins: [thing]"); + ).toBe("thing"); }); it("places required arguments inside the call", () => { @@ -99,6 +99,6 @@ describe("config lines", () => { pluginsArrayEntry( entry({ exportName: "thing", callArgs: '{ collections: ["posts"] }' }) ) - ).toBe('plugins: [thing({ collections: ["posts"] })]'); + ).toBe('thing({ collections: ["posts"] })'); }); }); diff --git a/packages/admin/src/lib/plugins/registry/entries.ts b/packages/admin/src/lib/plugins/registry/entries.ts index 8f1db5f299..b0720916c3 100644 --- a/packages/admin/src/lib/plugins/registry/entries.ts +++ b/packages/admin/src/lib/plugins/registry/entries.ts @@ -66,9 +66,15 @@ export const REGISTRY_ENTRIES: RegistryPlugin[] = [ // `collections` is required rather than illustrative: the plugin adds the // SEO group only to the collections it is given, so a bare call does not // type-check. + // + // A named placeholder, not a plausible slug like "posts". The blank + // template ships `collections: []`, and naming a collection the project + // does not have makes the eager schema fold throw + // NEXTLY_SCHEMA_EXTEND_TARGET_UNKNOWN at startup — a copied line that + // stops the app is worse than one that obviously has to be edited. config: { exportName: "seoPlugin", - callArgs: '{ collections: ["posts"] }', + callArgs: '{ collections: ["your-collection"] }', }, links: { homepage: "https://nextlyhq.com", diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts index bd5a877fbc..d636f4b3f2 100644 --- a/packages/admin/src/lib/plugins/registry/install-command.ts +++ b/packages/admin/src/lib/plugins/registry/install-command.ts @@ -73,7 +73,13 @@ export function importStatement(plugin: RegistryPlugin): string { } /** - * The entry to add inside `plugins: [...]`. + * The single element to append inside an existing `plugins: [...]`. + * + * The element alone, not the whole `plugins:` property. A reader who already + * has plugins configured — the likely one, since the directory is reached from + * the installed list — cannot use a property: pasting it inside the array is + * not valid TypeScript, and pasting it over the existing property silently + * drops every plugin already there. * * `callArgs` decides whether the binding is called at all: `null` means the * package exports a ready-made plugin value, which is how @@ -83,6 +89,5 @@ export function importStatement(plugin: RegistryPlugin): string { */ export function pluginsArrayEntry(plugin: RegistryPlugin): string { const { exportName, callArgs } = plugin.config; - const value = callArgs === null ? exportName : `${exportName}(${callArgs})`; - return `plugins: [${value}]`; + return callArgs === null ? exportName : `${exportName}(${callArgs})`; } diff --git a/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx b/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx index bcc2a61230..c59dc18afb 100644 --- a/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/[slug].test.tsx @@ -14,7 +14,7 @@ vi.mock("@admin/context/providers/BrandingProvider", () => ({ useBranding: () => mockBranding, // Settled with an answer: every case here is about a plugin the branding // payload already carries. - useBrandingStatus: () => ({ isPending: false, isError: false }), + useBrandingStatus: () => ({ isPending: false, isUnavailable: false }), })); import PluginDetailPage from "./[slug]"; diff --git a/packages/admin/src/pages/dashboard/plugins/[slug].tsx b/packages/admin/src/pages/dashboard/plugins/[slug].tsx index 1b325d8d42..e378403e53 100644 --- a/packages/admin/src/pages/dashboard/plugins/[slug].tsx +++ b/packages/admin/src/pages/dashboard/plugins/[slug].tsx @@ -181,7 +181,7 @@ function InstalledPluginsUnavailable() { function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { const branding = useBranding(); - const { isPending, isError } = useBrandingStatus(); + const { isPending, isUnavailable } = useBrandingStatus(); const plugins = branding?.plugins ?? []; const plugin = activeSlug ? plugins.find(p => pluginSlug(p.name) === activeSlug) @@ -197,7 +197,7 @@ function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { // "not installed" tells someone who HAS this plugin to go and install it. if (!plugin) { if (isPending) return ; - if (isError) return ; + if (isUnavailable) return ; return ; } diff --git a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx index 15dac9f269..d564a50708 100644 --- a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx @@ -1,8 +1,8 @@ /** * The directory joins two sources: a curated catalogue, and what the server * reports as installed. These cover that join, and the route that reaches it — - * `/admin/plugins/browse` is also a legal `/admin/plugins/[slug]`, so which - * page answers is a property worth pinning rather than assuming. + * the directory sits outside `/admin/plugins/` so that no plugin name can + * shadow it, and that separation is worth pinning rather than assuming. */ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { fireEvent, render, screen, waitFor } from "@testing-library/react"; @@ -37,15 +37,7 @@ afterEach(() => { }); describe("plugin browse route", () => { - /** - * `/admin/plugins/[slug]` matches this path too. Two independent things keep - * the directory reachable — the exact-match pass in `resolveRoute`, and the - * browse route being registered before the detail route, since the dynamic - * matcher takes the first pattern that matches. Either alone is sufficient, - * so this fails only when both are gone, which is the state that would - * actually render a plugin detail page for a plugin named "browse". - */ - it("resolves the browse path to the browse page, not the detail page", () => { + it("resolves the directory path to the browse page", () => { const resolved = resolveRoute(ROUTES.PLUGIN_BROWSE, ""); expect(resolved.Component).toBe(PluginBrowsePage); @@ -54,13 +46,31 @@ describe("plugin browse route", () => { it("still resolves a real plugin slug to the detail page", () => { // Positive control on the matcher: the dynamic route is reachable, so the - // assertion above is about precedence rather than about a detail route - // that stopped matching anything. + // assertion above is about the directory's own path rather than about a + // detail route that stopped matching anything. const resolved = resolveRoute("/admin/plugins/nextlyhq-plugin-seo", ""); expect(resolved.Component).not.toBe(PluginBrowsePage); expect(resolved.params).toEqual({ slug: "nextlyhq-plugin-seo" }); }); + + /** + * The separating case, and the reason the directory does not live at + * `/admin/plugins/browse`. + * + * `PluginDefinition.name` is an arbitrary string, so a plugin can slugify to + * any segment a static sibling might occupy — `browse` included. With the + * two sharing a parent, one of them has to win, and the loser is a page the + * UI still links to and nothing can reach. Under a different parent the + * question does not arise, so this asserts the slug the old layout would + * have swallowed still reaches its own page. + */ + it("reaches the detail page for a plugin whose slug is the old directory segment", () => { + const resolved = resolveRoute("/admin/plugins/browse", ""); + + expect(resolved.Component).not.toBe(PluginBrowsePage); + expect(resolved.params).toEqual({ slug: "browse" }); + }); }); describe("PluginBrowsePage", () => { diff --git a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx index 7849390f6a..820a354db0 100644 --- a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx @@ -23,7 +23,7 @@ import type { AdminBranding } from "@admin/types/branding"; const INSTALL_COMMAND = /^pnpm add @nextlyhq\/plugin-seo@\d/; let mockBranding: AdminBranding = { plugins: [] } as unknown as AdminBranding; -let mockBrandingStatus = { isPending: false, isError: false }; +let mockBrandingStatus = { isPending: false, isUnavailable: false }; vi.mock("@admin/context/providers/BrandingProvider", () => ({ useBranding: () => mockBranding, @@ -45,7 +45,7 @@ function renderDetail(slug: string) { afterEach(() => { mockBranding = { plugins: [] } as unknown as AdminBranding; - mockBrandingStatus = { isPending: false, isError: false }; + mockBrandingStatus = { isPending: false, isUnavailable: false }; vi.restoreAllMocks(); }); @@ -76,7 +76,12 @@ describe("plugin detail, not installed", () => { expect( screen.getByText('import { seoPlugin } from "@nextlyhq/plugin-seo";') ).toBeInTheDocument(); - expect(screen.getByText(/plugins: \[seoPlugin\(/)).toBeInTheDocument(); + // The element alone, with no `plugins:` property around it. A reader who + // already has plugins configured appends this; a property could only be + // pasted by replacing theirs. + expect( + screen.getByText('seoPlugin({ collections: ["your-collection"] })') + ).toBeInTheDocument(); }); /** @@ -86,7 +91,7 @@ describe("plugin detail, not installed", () => { * something they already have. */ it("does not offer to install while the installed list is still loading", async () => { - mockBrandingStatus = { isPending: true, isError: false }; + mockBrandingStatus = { isPending: true, isUnavailable: false }; renderDetail("nextlyhq-plugin-seo"); @@ -97,7 +102,7 @@ describe("plugin detail, not installed", () => { /** Same reasoning, permanent: a failed request is not evidence of absence. */ it("does not offer to install when the installed list could not be loaded", async () => { - mockBrandingStatus = { isPending: false, isError: true }; + mockBrandingStatus = { isPending: false, isUnavailable: true }; renderDetail("nextlyhq-plugin-seo"); diff --git a/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx index 5ebf1dd037..69bfc7f0f5 100644 --- a/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/plugin-detail.test.tsx @@ -45,7 +45,7 @@ const plugins: PluginMetadata[] = [ vi.mock("@admin/context/providers/BrandingProvider", () => ({ useBranding: () => ({ plugins }), // Settled with an answer, so a slug missing from `plugins` really is absent. - useBrandingStatus: () => ({ isPending: false, isError: false }), + useBrandingStatus: () => ({ isPending: false, isUnavailable: false }), })); describe("PluginDetailPage", () => { diff --git a/packages/admin/src/pages/registry.ts b/packages/admin/src/pages/registry.ts index 0af3262e92..848f260b21 100644 --- a/packages/admin/src/pages/registry.ts +++ b/packages/admin/src/pages/registry.ts @@ -373,12 +373,9 @@ export const routeConfig: Record = { type: "private", requiredPermission: "manage-settings", }, - // `/admin/plugins/[slug]` matches `/admin/plugins/browse` too, so which page - // answers is decided rather than incidental. Two things decide it, and - // either alone is sufficient: `resolveRoute` tries an exact-match pass before - // the dynamic matcher, and the dynamic matcher returns the first pattern that - // matches in insertion order, which is this one. Kept above PLUGIN_DETAIL so - // the ordering half stays true if the exact pass is ever reworked. + // Registered outside `/admin/plugins/`, so no ordering rule holds this in + // place: the directory and a plugin's detail page cannot match the same + // path, whatever the plugin is called. [ROUTES.PLUGIN_BROWSE]: { component: PluginBrowsePage, type: "private", From f7bcbb870d8ffc9292c4302a50aaf57f1a09e756 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 10:55:47 +0500 Subject: [PATCH 08/16] test(admin): cover the branding readiness states --- .../providers/BrandingProvider.test.tsx | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 packages/admin/src/context/providers/BrandingProvider.test.tsx diff --git a/packages/admin/src/context/providers/BrandingProvider.test.tsx b/packages/admin/src/context/providers/BrandingProvider.test.tsx new file mode 100644 index 0000000000..bf37db8377 --- /dev/null +++ b/packages/admin/src/context/providers/BrandingProvider.test.tsx @@ -0,0 +1,109 @@ +/** + * Readers that conclude something from a plugin being MISSING from branding + * need to know the list arrived. `useBranding` alone cannot tell them: it + * returns `{}` both before the request answers and when the project genuinely + * has nothing, so these cover the distinction `useBrandingStatus` adds. + */ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { AdminBranding } from "@admin/types/branding"; + +const get = vi.fn(); + +vi.mock("@admin/lib/api/publicApi", () => ({ + publicApi: { get: (...args: unknown[]) => get(...args) }, +})); + +import { + BrandingProvider, + useBrandingStatus, +} from "@admin/context/providers/BrandingProvider"; + +function Probe() { + const { isPending, isUnavailable } = useBrandingStatus(); + return ( + + {isPending ? "pending" : isUnavailable ? "unavailable" : "answered"} + + ); +} + +function renderProbe(client: QueryClient) { + return render( + + + + + + ); +} + +function status() { + return screen.getByTestId("status").textContent; +} + +afterEach(() => { + get.mockReset(); + vi.restoreAllMocks(); +}); + +describe("useBrandingStatus", () => { + it("reports pending until the request answers", async () => { + get.mockReturnValue(new Promise(() => {})); + + renderProbe( + new QueryClient({ defaultOptions: { queries: { retry: false } } }) + ); + + expect(status()).toBe("pending"); + }); + + it("reports answered once branding arrives", async () => { + get.mockResolvedValue({ plugins: [] } as AdminBranding); + + renderProbe( + new QueryClient({ defaultOptions: { queries: { retry: false } } }) + ); + + await waitFor(() => expect(status()).toBe("answered")); + }); + + it("reports unavailable when the first request fails", async () => { + get.mockRejectedValue(new Error("boom")); + + renderProbe( + new QueryClient({ defaultOptions: { queries: { retry: false } } }) + ); + + await waitFor(() => expect(status()).toBe("unavailable")); + }); + + /** + * The separating case, and the one a plain `isError` gets wrong. + * + * A background refetch can fail long after a response is cached, and that + * cached response is still a complete answer. Reporting it as unavailable + * would replace a correct page with an error for as long as the server stays + * unreachable — for a reader looking at a plugin the cached list already + * proved absent, that is an error screen instead of the install page. + */ + it("stays answered when a refetch fails after a response is cached", async () => { + get.mockResolvedValueOnce({ plugins: [] } as AdminBranding); + get.mockRejectedValue(new Error("boom")); + + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + renderProbe(client); + await waitFor(() => expect(status()).toBe("answered")); + + await client.refetchQueries({ queryKey: ["admin-meta"] }); + + // The refetch really did fail — without this the assertion below is + // satisfied by a refetch that never ran, which is the same green. + expect(get).toHaveBeenCalledTimes(2); + expect(status()).toBe("answered"); + }); +}); From a4a9c2a0ed19985914b68f9424eee3c620b92fc1 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 11:34:33 +0500 Subject: [PATCH 09/16] test(admin): cover branding readiness and state the refetch gap --- .../providers/BrandingProvider.test.tsx | 41 +++++++------------ 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/packages/admin/src/context/providers/BrandingProvider.test.tsx b/packages/admin/src/context/providers/BrandingProvider.test.tsx index bf37db8377..224ad752cb 100644 --- a/packages/admin/src/context/providers/BrandingProvider.test.tsx +++ b/packages/admin/src/context/providers/BrandingProvider.test.tsx @@ -79,31 +79,18 @@ describe("useBrandingStatus", () => { await waitFor(() => expect(status()).toBe("unavailable")); }); - - /** - * The separating case, and the one a plain `isError` gets wrong. - * - * A background refetch can fail long after a response is cached, and that - * cached response is still a complete answer. Reporting it as unavailable - * would replace a correct page with an error for as long as the server stays - * unreachable — for a reader looking at a plugin the cached list already - * proved absent, that is an error screen instead of the install page. - */ - it("stays answered when a refetch fails after a response is cached", async () => { - get.mockResolvedValueOnce({ plugins: [] } as AdminBranding); - get.mockRejectedValue(new Error("boom")); - - const client = new QueryClient({ - defaultOptions: { queries: { retry: false } }, - }); - renderProbe(client); - await waitFor(() => expect(status()).toBe("answered")); - - await client.refetchQueries({ queryKey: ["admin-meta"] }); - - // The refetch really did fail — without this the assertion below is - // satisfied by a refetch that never ran, which is the same green. - expect(get).toHaveBeenCalledTimes(2); - expect(status()).toBe("answered"); - }); }); + +/** + * NOT COVERED, stated rather than left to look covered: a background refetch + * failing while a previous response is cached must leave the status + * `answered`, since the cached response is still a complete answer. + * + * `isUnavailable` is wired to the query's `isLoadingError`, which query-core + * defines as `isError && !hasData` (`isRefetchError` is the `&& hasData` + * half), so the distinction is the library's own and holds by construction. + * A test was written for it and removed: driving a failed refetch through + * `refetchQueries` errors the cache entry without the probe observing a + * re-render, so the assertion passed against BOTH `isLoadingError` and a + * deliberately broken `isError` — coverage in appearance only. + */ From 38f7d8abea3a3f1ecba9aadbb35cfc5bb9451423 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 12:11:59 +0500 Subject: [PATCH 10/16] fix(admin): give the plugin detail page a loading boundary and report copy failures The catalogue query suspends and the nearest boundary was RootLayout's fallback={null}, so a cold request rendered a blank page. Copying now reports failure instead of doing nothing silently: the Clipboard API needs a secure context, so an admin served over plain HTTP has no navigator.clipboard at all. --- .../src/pages/dashboard/plugins/[slug].tsx | 33 +++----- .../plugins/components/NotInstalledPlugin.tsx | 50 ++++++++++--- .../plugins/components/PluginPageLoading.tsx | 32 ++++++++ .../plugins/not-installed-detail.test.tsx | 75 ++++++++++++++++++- 4 files changed, 158 insertions(+), 32 deletions(-) create mode 100644 packages/admin/src/pages/dashboard/plugins/components/PluginPageLoading.tsx diff --git a/packages/admin/src/pages/dashboard/plugins/[slug].tsx b/packages/admin/src/pages/dashboard/plugins/[slug].tsx index e378403e53..67a4045e12 100644 --- a/packages/admin/src/pages/dashboard/plugins/[slug].tsx +++ b/packages/admin/src/pages/dashboard/plugins/[slug].tsx @@ -2,6 +2,7 @@ import { Badge } from "@nextlyhq/ui"; import { useQueryClient, useSuspenseQuery } from "@tanstack/react-query"; +import { Suspense } from "react"; import { BookOpen, @@ -11,7 +12,6 @@ import { Globe, LayoutDashboard, Layers, - Loader2, Menu as MenuIcon, Package, Route, @@ -38,6 +38,7 @@ import { staticRegistrySource } from "@admin/lib/plugins/registry/static-source" import type { PluginMetadata } from "@admin/types/branding"; import { NotInstalledPlugin } from "./components/NotInstalledPlugin"; +import { PluginPageLoading } from "./components/PluginPageLoading"; import { PluginStatusPill } from "./components/PluginsTable"; const PLACEMENT_LABELS: Record = { @@ -139,25 +140,6 @@ function UninstalledOrMissing({ activeSlug }: { activeSlug?: string }) { ); } -/** - * Shown while the installed-plugin list is still in flight. - * - * The alternative — rendering the catalogue view immediately and correcting it - * when the list lands — flashes install instructions at someone who already - * has the plugin, and reads as an answer rather than as a wait. - */ -function LoadingInstalledPlugins() { - return ( -
- - Loading plugin… -
- ); -} - /** * Shown when admin-meta failed, so whether this plugin is installed is * unknown. @@ -196,9 +178,16 @@ function PluginDetailContent({ activeSlug }: { activeSlug?: string }) { // empty for a reason that says nothing about the project, and reading it as // "not installed" tells someone who HAS this plugin to go and install it. if (!plugin) { - if (isPending) return ; + if (isPending) return ; if (isUnavailable) return ; - return ; + // Its own Suspense boundary: `UninstalledOrMissing` suspends on the + // catalogue, and the nearest boundary above is RootLayout's + // `fallback={null}`, which would blank the page for the duration. + return ( + }> + + + ); } const title = plugin.appearance?.label ?? plugin.name; diff --git a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx index 0680ac0c4f..fdd391acf5 100644 --- a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx +++ b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx @@ -4,7 +4,7 @@ import { Button } from "@nextlyhq/ui"; import type React from "react"; import { useState } from "react"; -import { Check, Copy } from "@admin/components/icons"; +import { AlertCircle, Check, Copy } from "@admin/components/icons"; import { PluginIcon } from "@admin/components/shared/plugin-icon"; import { adminVersion } from "@admin/lib/admin-version"; import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; @@ -25,7 +25,33 @@ import type { RegistryPlugin } from "@admin/lib/plugins/registry/types"; * look somewhere else to learn that something local succeeded. */ function CopyLine({ value, label }: { value: string; label: string }) { - const [copied, setCopied] = useState(false); + const [outcome, setOutcome] = useState<"idle" | "copied" | "failed">("idle"); + + // The Clipboard API needs a secure context, so an admin served over plain + // HTTP — a LAN host, a colleague's dev box — has no `navigator.clipboard` at + // all, and `writeText` can be rejected by permissions even where it exists. + // Both end with the reader having to select the line by hand, so both have + // to say so: a button that silently does nothing reads as a broken page. + const copy = () => { + const clipboard = navigator.clipboard; + if (!clipboard) { + setOutcome("failed"); + return; + } + try { + void clipboard.writeText(value).then( + () => { + setOutcome("copied"); + window.setTimeout(() => setOutcome("idle"), 2000); + }, + () => setOutcome("failed") + ); + } catch { + // A synchronous throw rather than a rejection, which some + // implementations do when the document is not focused. + setOutcome("failed"); + } + }; return (
@@ -43,20 +69,26 @@ function CopyLine({ value, label }: { value: string; label: string }) { // Names the thing copied, since several of these sit on one page and // "Copy" alone would read identically to a screen reader each time. aria-label={`Copy ${label.toLowerCase()}`} - onClick={() => { - void navigator.clipboard?.writeText(value).then(() => { - setCopied(true); - window.setTimeout(() => setCopied(false), 2000); - }); - }} + onClick={copy} > - {copied ? ( + {outcome === "copied" ? ( + ) : outcome === "failed" ? ( + ) : ( )}
+ {outcome === "failed" && ( + // Announced, because the only other signal is an icon on the button + // the reader just pressed, and the instruction matters more than the + // failure: the line above is still there to be selected. +

+ Could not copy to the clipboard. Select the line above and copy it + manually. +

+ )} ); } diff --git a/packages/admin/src/pages/dashboard/plugins/components/PluginPageLoading.tsx b/packages/admin/src/pages/dashboard/plugins/components/PluginPageLoading.tsx new file mode 100644 index 0000000000..c4ff443b8e --- /dev/null +++ b/packages/admin/src/pages/dashboard/plugins/components/PluginPageLoading.tsx @@ -0,0 +1,32 @@ +"use client"; + +import type React from "react"; + +import { Loader2 } from "@admin/components/icons"; + +/** + * What a plugin page shows while the data it suspends on is in flight. + * + * A `Suspense` fallback is not optional for these pages. `QueryErrorBoundary` + * handles errors and nothing else, and the only boundary above it is + * `RootLayout`'s `fallback={null}` — which exists to hide a lazy chunk + * swapping in, not to stand in for a page. Without a local boundary a + * suspending page is a blank screen for as long as the request takes. + * + * @module pages/dashboard/plugins/components/PluginPageLoading + */ +export function PluginPageLoading({ + label = "Loading…", +}: { + label?: string; +}): React.ReactElement { + return ( +
+ + {label} +
+ ); +} diff --git a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx index 820a354db0..78df358d47 100644 --- a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx @@ -5,7 +5,7 @@ * rather than an error. */ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { AdminBranding } from "@admin/types/branding"; @@ -22,6 +22,18 @@ import type { AdminBranding } from "@admin/types/branding"; */ const INSTALL_COMMAND = /^pnpm add @nextlyhq\/plugin-seo@\d/; +/** + * jsdom's navigator has no `clipboard` property at all, which is the same + * shape a browser on a plain-HTTP origin presents — so the "unavailable" case + * below is the environment's own default rather than something simulated. + */ +function setClipboard(clipboard: Clipboard | undefined) { + Object.defineProperty(navigator, "clipboard", { + value: clipboard, + configurable: true, + }); +} + let mockBranding: AdminBranding = { plugins: [] } as unknown as AdminBranding; let mockBrandingStatus = { isPending: false, isUnavailable: false }; @@ -46,6 +58,7 @@ function renderDetail(slug: string) { afterEach(() => { mockBranding = { plugins: [] } as unknown as AdminBranding; mockBrandingStatus = { isPending: false, isUnavailable: false }; + setClipboard(undefined); vi.restoreAllMocks(); }); @@ -112,6 +125,66 @@ describe("plugin detail, not installed", () => { expect(screen.queryByText(INSTALL_COMMAND)).toBeNull(); }); + /** + * The catalogue query suspends. The only boundary above this page is + * `RootLayout`'s `fallback={null}`, which exists to hide a lazy chunk + * swapping in, so without a local one the page is blank for the duration of + * the request rather than merely slow. + */ + it("shows a loading state while the catalogue is in flight", () => { + renderDetail("nextlyhq-plugin-seo"); + + // Read synchronously, before the catalogue promise settles: this is the + // frame the missing boundary would have rendered as nothing. + expect(screen.getByRole("status")).toBeInTheDocument(); + }); + + /** + * The Clipboard API needs a secure context, so an admin on plain HTTP has no + * `navigator.clipboard` at all. Without this the button is inert and the + * page looks broken; the line is still selectable, so the fix is to say so. + */ + it("says copying failed when the clipboard is unavailable", async () => { + setClipboard(undefined); + + renderDetail("nextlyhq-plugin-seo"); + fireEvent.click( + await screen.findByRole("button", { name: /copy install command/i }) + ); + + expect(await screen.findByText(/could not copy/i)).toBeInTheDocument(); + }); + + it("says copying failed when the write is rejected", async () => { + setClipboard({ + writeText: () => Promise.reject(new Error("denied")), + } as unknown as Clipboard); + + renderDetail("nextlyhq-plugin-seo"); + fireEvent.click( + await screen.findByRole("button", { name: /copy install command/i }) + ); + + expect(await screen.findByText(/could not copy/i)).toBeInTheDocument(); + }); + + /** + * The positive control for the two above: a working clipboard must NOT show + * the failure text, or both would pass on a component that always shows it. + */ + it("stays quiet when the copy succeeds", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + setClipboard({ writeText } as unknown as Clipboard); + + renderDetail("nextlyhq-plugin-seo"); + fireEvent.click( + await screen.findByRole("button", { name: /copy install command/i }) + ); + + await waitFor(() => expect(writeText).toHaveBeenCalled()); + expect(screen.queryByText(/could not copy/i)).toBeNull(); + }); + /** * The invariant the whole surface rests on: verified content only ever * appears in the verified section. Nothing here has been observed running, From 477b2ef13a93a714b50d4198b2440ccc6ac54bf9 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 12:21:29 +0500 Subject: [PATCH 11/16] fix(admin): classify the plugin directory into the Plugins sidebar section The category classifier matched the literal substring /admin/plugins, so the directory's separate namespace fell through to dashboard and closed the Plugins panel. Both arms now ask a shared segment-safe helper for the route constants, which also stops an unrelated sibling like /admin/plugins-archive being claimed. --- .../components/layout/sidebar/DualSidebar.tsx | 8 ++- .../admin/src/hooks/useSidebarNavigation.ts | 9 ++-- .../admin/src/lib/__tests__/is-under.test.ts | 50 +++++++++++++++++++ packages/admin/src/lib/routing.ts | 18 +++++++ 4 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 packages/admin/src/lib/__tests__/is-under.test.ts diff --git a/packages/admin/src/components/layout/sidebar/DualSidebar.tsx b/packages/admin/src/components/layout/sidebar/DualSidebar.tsx index fa3ae74c6f..ef753415ef 100644 --- a/packages/admin/src/components/layout/sidebar/DualSidebar.tsx +++ b/packages/admin/src/components/layout/sidebar/DualSidebar.tsx @@ -23,6 +23,7 @@ import { import { resolveCollectionPlacement } from "@admin/lib/plugins/collection-placement"; import { pluginSlug } from "@admin/lib/plugins/plugin-slug"; import { resolvePluginIcon } from "@admin/lib/plugins/resolve-plugin-icon"; +import { isUnder } from "@admin/lib/routing"; import { cn } from "@admin/lib/utils"; import type { ApiCollection } from "@admin/types/entities"; @@ -338,8 +339,13 @@ export function DualSidebar({ isMobile }: DualSidebarProps = {}) { }); }; + // The route constants rather than their spellings. The plugin directory + // sits at its own top level so no plugin slug can shadow it, which means + // the Plugins category is not one URL prefix and a literal would silently + // stop covering it the next time either route moves. if ( - pathname.includes("/admin/plugins") || + isUnder(pathname, ROUTES.PLUGINS) || + isUnder(pathname, ROUTES.PLUGIN_BROWSE) || pathname.includes("/admin/forms") || isPluginPath(collectionsData) ) { diff --git a/packages/admin/src/hooks/useSidebarNavigation.ts b/packages/admin/src/hooks/useSidebarNavigation.ts index c8fb348694..f669407502 100644 --- a/packages/admin/src/hooks/useSidebarNavigation.ts +++ b/packages/admin/src/hooks/useSidebarNavigation.ts @@ -3,6 +3,7 @@ import { useState, useEffect } from "react"; import type { NavigationItem } from "../constants/navigation"; +import { isUnder } from "../lib/routing"; /** * Custom hook to manage sidebar navigation state @@ -61,9 +62,11 @@ export function useSidebarNavigation( if (exactMatch) return false; - // Sub-item match (e.g., /admin/users/create should match /admin/users) - // Ensure we match whole segments to avoid /admin/user matching /admin/users - return path.startsWith(target + "/") || path === target; + // Sub-item match (e.g., /admin/users/create should match /admin/users), + // asked of the shared helper rather than restated: the sidebar's category + // classifier answers the same question, and two spellings of "is this + // route under that one" agree until one of them is adjusted. + return isUnder(path, target); }; return { diff --git a/packages/admin/src/lib/__tests__/is-under.test.ts b/packages/admin/src/lib/__tests__/is-under.test.ts new file mode 100644 index 0000000000..622121d7af --- /dev/null +++ b/packages/admin/src/lib/__tests__/is-under.test.ts @@ -0,0 +1,50 @@ +/** + * "Is this route under that one" is asked by the sidebar's category + * classifier and by its active-item matcher. These cover the answers a + * substring test gets wrong, which is why both now ask one function. + */ +import { describe, expect, it } from "vitest"; + +import { ROUTES } from "@admin/constants/routes"; +import { isUnder } from "@admin/lib/routing"; + +describe("isUnder", () => { + it("matches the route itself and its descendants", () => { + expect(isUnder("/admin/plugins", "/admin/plugins")).toBe(true); + expect(isUnder("/admin/plugins/acme-forms", "/admin/plugins")).toBe(true); + expect( + isUnder("/admin/plugins/acme-forms/settings", "/admin/plugins") + ).toBe(true); + }); + + /** + * The separating case. `"/admin/plugins-archive".includes("/admin/plugins")` + * is true, so the substring test this replaced classified an unrelated + * sibling as a plugins page — and the miss is silent, because the wrong + * answer is a real category rather than an error. + */ + it("rejects a sibling that merely shares the prefix", () => { + expect(isUnder("/admin/plugins-archive", "/admin/plugins")).toBe(false); + expect(isUnder("/admin/plugin-directory", "/admin/plugins")).toBe(false); + }); + + it("ignores a trailing slash on either side", () => { + expect(isUnder("/admin/plugins/", "/admin/plugins")).toBe(true); + expect(isUnder("/admin/plugins", "/admin/plugins/")).toBe(true); + }); + + it("does not match a parent from a child route", () => { + expect(isUnder("/admin", "/admin/plugins")).toBe(false); + }); + + /** + * The directory is deliberately not under `/admin/plugins`, so anything + * classifying the Plugins section has to name it separately. Pinned here + * because the consequence of forgetting is a silently wrong sidebar rather + * than a failure. + */ + it("confirms the directory is outside the installed-plugins subtree", () => { + expect(isUnder(ROUTES.PLUGIN_BROWSE, ROUTES.PLUGINS)).toBe(false); + expect(isUnder(ROUTES.PLUGIN_BROWSE, ROUTES.PLUGIN_BROWSE)).toBe(true); + }); +}); diff --git a/packages/admin/src/lib/routing.ts b/packages/admin/src/lib/routing.ts index 2739cc2182..37b1facf31 100644 --- a/packages/admin/src/lib/routing.ts +++ b/packages/admin/src/lib/routing.ts @@ -8,6 +8,24 @@ import registry, { routeConfig } from "../pages/registry"; import { matchPluginPage } from "./plugins/plugin-route-registry"; +/** + * Whether `pathname` is `base` or sits beneath it. + * + * Whole segments, so `/admin/plugins` does not claim `/admin/plugins-archive` + * — the reason this exists rather than each caller writing `includes()`. A + * substring test is a proxy for "is this route under that one", and the + * sibling paths that motivate writing the check are exactly the ones the proxy + * gets wrong. + * + * Callers pass `ROUTES.*` rather than a spelling, so a route that moves takes + * its classification with it. + */ +export function isUnder(pathname: string, base: string): boolean { + const path = pathname.replace(/\/$/, ""); + const target = base.replace(/\/$/, ""); + return path === target || path.startsWith(`${target}/`); +} + /** Props passed to page components by the router */ export interface PageProps { params?: Record; From ef4ee0cc393556da4a5075d0939cf8c0ddf45590 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 12:33:03 +0500 Subject: [PATCH 12/16] test(admin): size the search debounce wait to the delay it waits on --- .../src/pages/dashboard/plugins/browse.test.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx index d564a50708..7e5099ffe0 100644 --- a/packages/admin/src/pages/dashboard/plugins/browse.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/browse.test.tsx @@ -133,8 +133,17 @@ describe("PluginBrowsePage", () => { // The disappearance is the control. Asserting only that SEO survives is // satisfied by a page that never filtered at all, which is the same green // a search reading the wrong field produces. - await waitFor(() => - expect(screen.queryByRole("heading", { name: "Page Builder" })).toBeNull() + // The timeout is sized to the debounce, not to taste. `waitFor` defaults + // to 1000ms, which is only ~3x a delay the component takes deliberately, + // and under a full parallel suite that margin is not enough — this passed + // alone and failed in the whole run. The property being asserted is + // unchanged; only the patience is. + await waitFor( + () => + expect( + screen.queryByRole("heading", { name: "Page Builder" }) + ).toBeNull(), + { timeout: 5000 } ); expect(screen.getByRole("heading", { name: "SEO" })).toBeInTheDocument(); }); From 0c9b264e7d1cfc1f3eae41d32ed69e8285d32acb Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 12:47:44 +0500 Subject: [PATCH 13/16] docs(admin): note that source-mode consumers get an unpinned install command --- packages/admin/src/lib/admin-version.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/admin/src/lib/admin-version.ts b/packages/admin/src/lib/admin-version.ts index 9f089e443a..4e7cf94bfe 100644 --- a/packages/admin/src/lib/admin-version.ts +++ b/packages/admin/src/lib/admin-version.ts @@ -6,6 +6,13 @@ * `package.json` read as core has: this bundle targets the browser, where * there is no module resolver to fall back to. * + * A consumer that bundles this package from SOURCE gets `undefined`, because + * neither `define` runs. That is the contributor playground, which aliases + * `@nextlyhq/admin` to `src/index.ts` — so the install command it renders is + * unpinned, and that is correct rather than broken: a dev checkout has no + * published release to name. An installed project resolves the built `dist`, + * where the constant is a literal. + * * @module lib/admin-version */ From 77d0a32adc2b7e184126bf3174fbe0e63055fc12 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 12:54:58 +0500 Subject: [PATCH 14/16] fix(admin): include the admin-module import and announce copy success Two of the three catalogue plugins ship an /admin side-effect module the app's admin route has to import; without it the plugin installs and its server half runs while its admin UI silently never registers. Copy success is now announced to assistive technology, which the button's aria-label previously hid. --- .../__tests__/install-command.test.ts | 22 +++++++- .../admin/src/lib/plugins/registry/entries.ts | 8 ++- .../lib/plugins/registry/install-command.ts | 15 ++++++ .../admin/src/lib/plugins/registry/types.ts | 11 ++++ .../plugins/components/NotInstalledPlugin.tsx | 52 ++++++++++++++++--- .../plugins/not-installed-detail.test.tsx | 25 +++++++++ 6 files changed, 123 insertions(+), 10 deletions(-) diff --git a/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts index 0cdbc0de2d..7f2f01e9d0 100644 --- a/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts +++ b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts @@ -1,5 +1,5 @@ /** - * The three lines a reader copies to add a plugin. + * The lines a reader copies to add a plugin. * * Two properties matter and they are separate: the lines must be derived from * the entry rather than stored beside it, and the install must name a version. @@ -14,6 +14,7 @@ import { adminVersion } from "@admin/lib/admin-version"; import pkg from "../../../../../package.json"; import { + adminImportStatement, importStatement, installCommand, pluginsArrayEntry, @@ -94,6 +95,25 @@ describe("config lines", () => { ).toBe("thing"); }); + /** + * A fourth edit, in the admin route page rather than the config. Skipping it + * leaves the plugin installed and its server half running while its admin UI + * silently never registers, so the recipe has to name it. + */ + it("asks for the admin module only when the package ships one", () => { + expect( + adminImportStatement( + entry({ exportName: "thing", callArgs: "", adminModule: true }) + ) + ).toBe('import "@acme/thing/admin";'); + + // The separating case: a plugin without an `/admin` export must not be + // told to import one, or the recipe names a subpath that does not resolve. + expect( + adminImportStatement(entry({ exportName: "thing", callArgs: "" })) + ).toBeUndefined(); + }); + it("places required arguments inside the call", () => { expect( pluginsArrayEntry( diff --git a/packages/admin/src/lib/plugins/registry/entries.ts b/packages/admin/src/lib/plugins/registry/entries.ts index b0720916c3..d4fedcfacc 100644 --- a/packages/admin/src/lib/plugins/registry/entries.ts +++ b/packages/admin/src/lib/plugins/registry/entries.ts @@ -31,7 +31,7 @@ export const REGISTRY_ENTRIES: RegistryPlugin[] = [ category: "content", tags: ["blocks", "editor", "pages"], icon: { lucide: "Layout" }, - config: { exportName: "pageBuilder", callArgs: "" }, + config: { exportName: "pageBuilder", callArgs: "", adminModule: true }, links: { homepage: "https://nextlyhq.com", repository: "https://github.com/nextlyhq/nextly", @@ -48,7 +48,11 @@ export const REGISTRY_ENTRIES: RegistryPlugin[] = [ // `callArgs: null` — the export goes in uncalled. The factory returns a // FormBuilderPluginResult whose definition is at `.plugin`, and the package // exports the unwrapped value for exactly this use. - config: { exportName: "formBuilderPlugin", callArgs: null }, + config: { + exportName: "formBuilderPlugin", + callArgs: null, + adminModule: true, + }, links: { homepage: "https://nextlyhq.com", repository: "https://github.com/nextlyhq/nextly", diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts index d636f4b3f2..4e4a867710 100644 --- a/packages/admin/src/lib/plugins/registry/install-command.ts +++ b/packages/admin/src/lib/plugins/registry/install-command.ts @@ -87,6 +87,21 @@ export function importStatement(plugin: RegistryPlugin): string { * object whose definition sits at `.plugin`, so calling it here would be * wrong rather than merely verbose. */ +/** + * The side-effect import the app's admin route needs, for plugins that ship + * an `/admin` module — `undefined` for those that do not. + * + * Its own line rather than part of the config recipe because it goes in a + * different file: the admin route page, not `nextly.config.ts`. Undefined + * rather than an empty string, so a caller has to decide whether to render a + * step at all instead of rendering a blank one. + */ +export function adminImportStatement( + plugin: RegistryPlugin +): string | undefined { + return plugin.config.adminModule ? `import "${plugin.id}/admin";` : undefined; +} + export function pluginsArrayEntry(plugin: RegistryPlugin): string { const { exportName, callArgs } = plugin.config; return callArgs === null ? exportName : `${exportName}(${callArgs})`; diff --git a/packages/admin/src/lib/plugins/registry/types.ts b/packages/admin/src/lib/plugins/registry/types.ts index cec1f54baa..beffda445d 100644 --- a/packages/admin/src/lib/plugins/registry/types.ts +++ b/packages/admin/src/lib/plugins/registry/types.ts @@ -53,6 +53,17 @@ export interface RegistryPlugin { * are different facts about the package's API, not two spellings of one. */ callArgs: string | null; + /** + * Whether the package ships an `/admin` side-effect module that the app's + * admin route has to import. + * + * A fourth edit, in a different file, and omitting it is not a small + * miss: the plugin installs and its server half runs, so nothing errors, + * while its admin UI silently never registers — the form builder degrades + * to plain JSON inputs. Declared rather than assumed, because it is a fact + * about the package's export map that only some plugins have. + */ + adminModule?: boolean; }; links?: { homepage?: string; repository?: string; docs?: string }; } diff --git a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx index fdd391acf5..ae202810eb 100644 --- a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx +++ b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx @@ -10,6 +10,7 @@ import { adminVersion } from "@admin/lib/admin-version"; import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; import { PACKAGE_MANAGERS, + adminImportStatement, importStatement, installCommand, pluginsArrayEntry, @@ -24,7 +25,16 @@ import type { RegistryPlugin } from "@admin/lib/plugins/registry/types"; * looking at the thing they clicked, and a toast in the corner asks them to * look somewhere else to learn that something local succeeded. */ -function CopyLine({ value, label }: { value: string; label: string }) { +function CopyLine({ + value, + label, + file, +}: { + value: string; + label: string; + /** The file this line goes in, when it is not the one the section names. */ + file?: string; +}) { const [outcome, setOutcome] = useState<"idle" | "copied" | "failed">("idle"); // The Clipboard API needs a secure context, so an admin served over plain @@ -57,6 +67,11 @@ function CopyLine({ value, label }: { value: string; label: string }) {

{label} + {file && ( + + {file} + + )}

@@ -89,6 +104,15 @@ function CopyLine({ value, label }: { value: string; label: string }) { manually.

)} + {outcome === "copied" && ( + // Success is announced too, and only to assistive technology. The + // button's `aria-label` overrides its descendants, so swapping the + // glyph changes nothing a screen reader can perceive — sighted users + // already have the tick, and this is the equivalent for everyone else. +

+ {label} copied to the clipboard. +

+ )}
); } @@ -99,8 +123,8 @@ function CopyLine({ value, label }: { value: string; label: string }) { * Deliberately thin, and the reason is the invariant the whole surface is * built on: verified content only ever appears in the verified section. Nothing * here has been observed running, so there is no contributions section, no - * permissions and no routes — only what the catalogue claims, plus the three - * lines that would make the claims checkable. + * permissions and no routes — only what the catalogue claims, plus the lines + * that would make the claims checkable. * * @module pages/dashboard/plugins/components/NotInstalledPlugin */ @@ -111,6 +135,7 @@ export function NotInstalledPlugin({ }): React.ReactElement { const [manager, setManager] = useState("pnpm"); const label = categoryLabel(plugin.category); + const adminImport = adminImportStatement(plugin); return (
@@ -173,10 +198,9 @@ export function NotInstalledPlugin({ ))}
- {/* Three lines because the reader makes three edits, each copyable on - its own: the import and the array entry land in different places in - the same file, so joining them into one block would be a snippet - nobody can paste anywhere. */} + {/* One line per edit, each copyable on its own: the import and the + array entry land in different places in the same file, so joining + them into one block would be a snippet nobody can paste anywhere. */} + {adminImport && ( +
+

+ This plugin also ships admin UI, which is registered by importing + it in your admin route page. Skip it and the plugin still loads — + its editors just fall back to plain inputs. +

+ +
+ )} {/* States the boundary rather than leaving it implied: a reader looking diff --git a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx index 78df358d47..edb8c5c7d5 100644 --- a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx @@ -125,6 +125,31 @@ describe("plugin detail, not installed", () => { expect(screen.queryByText(INSTALL_COMMAND)).toBeNull(); }); + /** + * The admin-module step, which only some plugins need. Form Builder ships an + * `/admin` side-effect module; skip it and the plugin installs, its server + * half runs, and its builder silently degrades to plain inputs. + */ + it("asks for the admin route import when the plugin ships one", async () => { + renderDetail("nextlyhq-plugin-form-builder"); + + expect( + await screen.findByText('import "@nextlyhq/plugin-form-builder/admin";') + ).toBeInTheDocument(); + }); + + /** + * The separating case. SEO has no `/admin` export, so telling a reader to + * import one would name a subpath that does not resolve. + */ + it("omits the admin route import for a plugin without one", async () => { + renderDetail("nextlyhq-plugin-seo"); + + await screen.findByRole("heading", { name: "SEO" }); + expect(screen.queryByText(/\/admin";$/)).toBeNull(); + expect(screen.queryByText("Admin route import")).toBeNull(); + }); + /** * The catalogue query suspends. The only boundary above this page is * `RootLayout`'s `fallback={null}`, which exists to hide a lazy chunk From 326d7ef59c5c4ccd3379718ac1bdb57463ab0a8f Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 12:57:18 +0500 Subject: [PATCH 15/16] test(admin): restore the original clipboard descriptor between tests --- .../plugins/not-installed-detail.test.tsx | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx index edb8c5c7d5..36269fd492 100644 --- a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx @@ -27,6 +27,11 @@ const INSTALL_COMMAND = /^pnpm add @nextlyhq\/plugin-seo@\d/; * shape a browser on a plain-HTTP origin presents — so the "unavailable" case * below is the environment's own default rather than something simulated. */ +const ORIGINAL_CLIPBOARD = Object.getOwnPropertyDescriptor( + navigator, + "clipboard" +); + function setClipboard(clipboard: Clipboard | undefined) { Object.defineProperty(navigator, "clipboard", { value: clipboard, @@ -34,6 +39,19 @@ function setClipboard(clipboard: Clipboard | undefined) { }); } +/** + * Puts back what was there, which in jsdom is no own property at all — + * defining one with `undefined` is not the same thing, and it would outlive + * these tests for anything else sharing this environment. + */ +function restoreClipboard() { + if (ORIGINAL_CLIPBOARD) { + Object.defineProperty(navigator, "clipboard", ORIGINAL_CLIPBOARD); + } else { + delete (navigator as { clipboard?: Clipboard }).clipboard; + } +} + let mockBranding: AdminBranding = { plugins: [] } as unknown as AdminBranding; let mockBrandingStatus = { isPending: false, isUnavailable: false }; @@ -58,7 +76,7 @@ function renderDetail(slug: string) { afterEach(() => { mockBranding = { plugins: [] } as unknown as AdminBranding; mockBrandingStatus = { isPending: false, isUnavailable: false }; - setClipboard(undefined); + restoreClipboard(); vi.restoreAllMocks(); }); From 6683d7de83afa0ee050e5ae91b75cc13a776dcfc Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 13:23:24 +0500 Subject: [PATCH 16/16] fix(admin): include the editor stylesheet step and raise the card hover border to 3:1 Page Builder's /admin entry registers components and imports no CSS, so a reader following the recipe got an editor with none of its layout. The card's hover border was primary/40, which measures 2.84:1 against the page surface and failed the WCAG alpha-utility guard in @nextlyhq/ui. --- .../__tests__/install-command.test.ts | 26 ++++++++++++++ .../admin/src/lib/plugins/registry/entries.ts | 7 +++- .../lib/plugins/registry/install-command.ts | 14 ++++++++ .../admin/src/lib/plugins/registry/types.ts | 12 +++++++ .../plugins/components/NotInstalledPlugin.tsx | 34 +++++++++++++------ .../plugins/components/PluginCard.tsx | 5 ++- .../plugins/not-installed-detail.test.tsx | 28 ++++++++++++++- 7 files changed, 112 insertions(+), 14 deletions(-) diff --git a/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts index 7f2f01e9d0..44988f0a44 100644 --- a/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts +++ b/packages/admin/src/lib/plugins/registry/__tests__/install-command.test.ts @@ -15,6 +15,7 @@ import { adminVersion } from "@admin/lib/admin-version"; import pkg from "../../../../../package.json"; import { adminImportStatement, + adminStylesImport, importStatement, installCommand, pluginsArrayEntry, @@ -114,6 +115,31 @@ describe("config lines", () => { ).toBeUndefined(); }); + /** + * Separate from the admin module, because they are separate facts about the + * package. Page Builder's `/admin` entry imports no CSS, so a reader who + * takes only the module gets a registered editor with no layout at all. + */ + it("asks for the stylesheet only when the package exports one", () => { + expect( + adminStylesImport( + entry({ + exportName: "thing", + callArgs: "", + adminStyles: "styles/editor.css", + }) + ) + ).toBe('import "@acme/thing/styles/editor.css";'); + + // A plugin with an admin module but no stylesheet must not be told to + // import one — the two are independent. + expect( + adminStylesImport( + entry({ exportName: "thing", callArgs: "", adminModule: true }) + ) + ).toBeUndefined(); + }); + it("places required arguments inside the call", () => { expect( pluginsArrayEntry( diff --git a/packages/admin/src/lib/plugins/registry/entries.ts b/packages/admin/src/lib/plugins/registry/entries.ts index d4fedcfacc..b01913c175 100644 --- a/packages/admin/src/lib/plugins/registry/entries.ts +++ b/packages/admin/src/lib/plugins/registry/entries.ts @@ -31,7 +31,12 @@ export const REGISTRY_ENTRIES: RegistryPlugin[] = [ category: "content", tags: ["blocks", "editor", "pages"], icon: { lucide: "Layout" }, - config: { exportName: "pageBuilder", callArgs: "", adminModule: true }, + config: { + exportName: "pageBuilder", + callArgs: "", + adminModule: true, + adminStyles: "styles/editor.css", + }, links: { homepage: "https://nextlyhq.com", repository: "https://github.com/nextlyhq/nextly", diff --git a/packages/admin/src/lib/plugins/registry/install-command.ts b/packages/admin/src/lib/plugins/registry/install-command.ts index 4e4a867710..823e0e69eb 100644 --- a/packages/admin/src/lib/plugins/registry/install-command.ts +++ b/packages/admin/src/lib/plugins/registry/install-command.ts @@ -102,6 +102,20 @@ export function adminImportStatement( return plugin.config.adminModule ? `import "${plugin.id}/admin";` : undefined; } +/** + * The stylesheet import the admin route needs, for plugins that ship one. + * + * A separate line from `adminImportStatement`, because the module and the + * stylesheet are separate imports in the same file and a plugin can ship + * either without the other. Taking only the module leaves the editor + * registered and unstyled, which looks like a broken build rather than a + * missing step. + */ +export function adminStylesImport(plugin: RegistryPlugin): string | undefined { + const subpath = plugin.config.adminStyles; + return subpath ? `import "${plugin.id}/${subpath}";` : undefined; +} + export function pluginsArrayEntry(plugin: RegistryPlugin): string { const { exportName, callArgs } = plugin.config; return callArgs === null ? exportName : `${exportName}(${callArgs})`; diff --git a/packages/admin/src/lib/plugins/registry/types.ts b/packages/admin/src/lib/plugins/registry/types.ts index beffda445d..e1c0142a15 100644 --- a/packages/admin/src/lib/plugins/registry/types.ts +++ b/packages/admin/src/lib/plugins/registry/types.ts @@ -64,6 +64,18 @@ export interface RegistryPlugin { * about the package's export map that only some plugins have. */ adminModule?: boolean; + /** + * The package subpath of a stylesheet the admin route must import, when + * the plugin ships one. + * + * Separate from `adminModule` because they are separate facts: the + * page builder's `/admin` entry registers components and imports no CSS, + * so an app that takes only the module gets a registered editor with none + * of its layout — the shell, grid, toolbar and panes all come from + * `styles/editor.css`. The subpath is stored rather than assumed, since + * only some packages export one and they need not agree on its name. + */ + adminStyles?: string; }; links?: { homepage?: string; repository?: string; docs?: string }; } diff --git a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx index ae202810eb..7415ef686b 100644 --- a/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx +++ b/packages/admin/src/pages/dashboard/plugins/components/NotInstalledPlugin.tsx @@ -11,6 +11,7 @@ import { categoryLabel } from "@admin/lib/plugins/plugin-categories"; import { PACKAGE_MANAGERS, adminImportStatement, + adminStylesImport, importStatement, installCommand, pluginsArrayEntry, @@ -136,6 +137,7 @@ export function NotInstalledPlugin({ const [manager, setManager] = useState("pnpm"); const label = categoryLabel(plugin.category); const adminImport = adminImportStatement(plugin); + const adminStyles = adminStylesImport(plugin); return (
@@ -210,18 +212,28 @@ export function NotInstalledPlugin({ label="Plugins array entry" value={pluginsArrayEntry(plugin)} /> - {adminImport && ( -
-

- This plugin also ships admin UI, which is registered by importing - it in your admin route page. Skip it and the plugin still loads — - its editors just fall back to plain inputs. + {(adminImport || adminStyles) && ( +

+

+ This plugin also ships admin UI, registered by importing it in + your admin route page. Skip these and the plugin still loads — its + editors just fall back to plain inputs + {adminStyles ? ", and render unstyled" : ""}.

- + {adminImport && ( + + )} + {adminStyles && ( + + )}
)} diff --git a/packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx b/packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx index 4991fc5351..733f7713d7 100644 --- a/packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx +++ b/packages/admin/src/pages/dashboard/plugins/components/PluginCard.tsx @@ -44,7 +44,10 @@ export function PluginCard({ return (
diff --git a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx index 36269fd492..3296510959 100644 --- a/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx +++ b/packages/admin/src/pages/dashboard/plugins/not-installed-detail.test.tsx @@ -5,7 +5,13 @@ * rather than an error. */ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { AdminBranding } from "@admin/types/branding"; @@ -156,6 +162,26 @@ describe("plugin detail, not installed", () => { ).toBeInTheDocument(); }); + /** + * Page Builder needs both the module and its stylesheet. Form Builder needs + * only the module, so a page that showed a stylesheet line for it would be + * naming a subpath that does not resolve. + */ + it("asks for the editor stylesheet only where the package exports one", async () => { + renderDetail("nextlyhq-plugin-page-builder"); + expect( + await screen.findByText( + 'import "@nextlyhq/plugin-page-builder/styles/editor.css";' + ) + ).toBeInTheDocument(); + + cleanup(); + + renderDetail("nextlyhq-plugin-form-builder"); + await screen.findByText('import "@nextlyhq/plugin-form-builder/admin";'); + expect(screen.queryByText("Editor stylesheet")).toBeNull(); + }); + /** * The separating case. SEO has no `/admin` export, so telling a reader to * import one would name a subpath that does not resolve.