diff --git a/.changeset/plugin-description-check.md b/.changeset/plugin-description-check.md new file mode 100644 index 0000000000..96ed388f64 --- /dev/null +++ b/.changeset/plugin-description-check.md @@ -0,0 +1,30 @@ +--- +"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 +--- + +Warn at boot when a plugin ships without an `admin.description`. Without one the +admin can only show the package specifier wherever it lists that plugin, and +nothing previously stopped a plugin shipping that way. diff --git a/packages/nextly/src/di/register.ts b/packages/nextly/src/di/register.ts index 6e479cfd4c..da8877d6e4 100644 --- a/packages/nextly/src/di/register.ts +++ b/packages/nextly/src/di/register.ts @@ -99,6 +99,7 @@ import { registerSingleHooks } from "../hooks/register-single-hooks"; import { createSanitizationHook } from "../hooks/sanitization-hooks"; import type { PluginPermission, PluginRole } from "../plugins/contributions"; import { getCoreVersion } from "../plugins/core-version"; +import { warnUndescribedPlugins } from "../plugins/describe-check"; import { setInitializedPlugins } from "../plugins/initialized-plugins"; import { collectCustomPermissions, @@ -574,6 +575,13 @@ export async function registerServices( if (transformedConfig.plugins && transformedConfig.plugins.length > 0) { const pluginNames = transformedConfig.plugins.map(p => p.name).join(", "); resolvedLogger.info?.(`Registered plugins: ${pluginNames}`); + + // Beside the line that names them, because that line is the symptom: a + // plugin with no description is one the admin can only ever show by its + // package specifier. Warned rather than thrown — the omission is the + // plugin author's and breaks nothing, and an operator cannot fix a + // third-party package from their own config. + warnUndescribedPlugins(transformedConfig.plugins, resolvedLogger); } // ---------------------------------------- diff --git a/packages/nextly/src/plugins/describe-check.test.ts b/packages/nextly/src/plugins/describe-check.test.ts new file mode 100644 index 0000000000..c0a5c3d64d --- /dev/null +++ b/packages/nextly/src/plugins/describe-check.test.ts @@ -0,0 +1,110 @@ +/** + * A plugin with no description is one the admin can only ever show by its + * package specifier, which is a name chosen for npm rather than for a reader. + */ +import { describe, expect, it } from "vitest"; + +import type { Logger } from "../shared/types"; + +import { + pluginsMissingDescription, + warnUndescribedPlugins, +} from "./describe-check"; +import type { PluginDefinition } from "./plugin-context"; + +function plugin(name: string, description?: string): PluginDefinition { + return { + name, + version: "1.0.0", + ...(description === undefined ? {} : { admin: { description } }), + } as PluginDefinition; +} + +describe("pluginsMissingDescription", () => { + it("names a plugin that declares none", () => { + expect(pluginsMissingDescription([plugin("@acme/bare")])).toEqual([ + "@acme/bare", + ]); + }); + + it("passes a plugin that declares one", () => { + expect( + pluginsMissingDescription([plugin("@acme/described", "Does a thing")]) + ).toEqual([]); + }); + + /** + * The separating cases. A present-but-useless value satisfies `!== undefined` + * and an optional-chain check alike, so a test that only covers "absent" + * passes on an implementation that admits an empty string. + */ + it.each([ + ["empty", ""], + ["whitespace", " "], + ])("names a plugin whose description is %s", (_why, value) => { + expect(pluginsMissingDescription([plugin("@acme/blank", value)])).toEqual([ + "@acme/blank", + ]); + }); + + it("reports only the ones missing it, in order", () => { + const result = pluginsMissingDescription([ + plugin("@acme/a"), + plugin("@acme/b", "Has one"), + plugin("@acme/c"), + ]); + + expect(result).toEqual(["@acme/a", "@acme/c"]); + }); + + it("says nothing about an empty plugin list", () => { + expect(pluginsMissingDescription([])).toEqual([]); + }); +}); + +describe("warnUndescribedPlugins", () => { + function fakeLogger() { + const warnings: string[] = []; + return { + logger: { warn: (m: string) => warnings.push(m) } as unknown as Logger, + warnings, + }; + } + + it("names every plugin missing one, in one message", () => { + const { logger, warnings } = fakeLogger(); + + warnUndescribedPlugins( + [plugin("@acme/a"), plugin("@acme/b", "Has one"), plugin("@acme/c")], + logger + ); + + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain("@acme/a"); + expect(warnings[0]).toContain("@acme/c"); + // The separating assertion: a message that simply listed every plugin + // would satisfy the two above. + expect(warnings[0]).not.toContain("@acme/b"); + }); + + /** + * Silence when there is nothing to say. A warning on every boot of a + * correctly-described install is noise that trains an operator to ignore the + * one that matters. + */ + it("says nothing when every plugin declares one", () => { + const { logger, warnings } = fakeLogger(); + + warnUndescribedPlugins([plugin("@acme/a", "Has one")], logger); + + expect(warnings).toEqual([]); + }); + + it("says nothing when there are no plugins", () => { + const { logger, warnings } = fakeLogger(); + + warnUndescribedPlugins([], logger); + + expect(warnings).toEqual([]); + }); +}); diff --git a/packages/nextly/src/plugins/describe-check.ts b/packages/nextly/src/plugins/describe-check.ts new file mode 100644 index 0000000000..3f0c4d2e40 --- /dev/null +++ b/packages/nextly/src/plugins/describe-check.ts @@ -0,0 +1,53 @@ +import type { Logger } from "../shared/types"; + +import type { PluginDefinition } from "./plugin-context"; + +/** + * Plugins that ship without an `admin.description`. + * + * A description is the only thing that tells an operator what an installed + * plugin is FOR. Without one every surface that lists plugins — the plugins + * table, the dashboard section, the detail page — can show only the package + * specifier, so `@nextlyhq/plugin-page-builder` appears where another plugin + * shows "Build pages visually from blocks". The reader is left to infer the + * purpose from a name chosen for npm rather than for them. + * + * Pure, and returns the names rather than logging them, so the caller decides + * whether this is a boot warning, a CLI report, or a test assertion. The same + * question asked from three places must not become three answers. + * + * Deliberately NOT an error. A missing description breaks nothing at runtime, + * and failing a boot over it would punish an operator for a plugin author's + * omission — they cannot fix a third-party package from their own config. + * + * @module plugins/describe-check + */ +export function pluginsMissingDescription( + plugins: readonly PluginDefinition[] +): string[] { + return plugins + .filter(plugin => !plugin.admin?.description?.trim()) + .map(plugin => plugin.name); +} + +/** + * Report the plugins that ship without a description, if any. + * + * Takes the logger rather than reaching for one, so the message it produces is + * observable in a test. Boot wiring that only exists at a call site inside a + * several-hundred-line registration function can be deleted without any test + * noticing; the message is the part worth pinning, and this is the smallest + * unit that owns it. + */ +export function warnUndescribedPlugins( + plugins: readonly PluginDefinition[], + logger: Logger +): void { + const undescribed = pluginsMissingDescription(plugins); + if (undescribed.length === 0) return; + + logger.warn?.( + `Plugins with no admin.description, so the admin can only show their ` + + `package name: ${undescribed.join(", ")}` + ); +}