diff --git a/.changeset/create-plugin-scaffold-dead-artifacts-3755-3759.md b/.changeset/create-plugin-scaffold-dead-artifacts-3755-3759.md new file mode 100644 index 000000000..d9166c86a --- /dev/null +++ b/.changeset/create-plugin-scaffold-dead-artifacts-3755-3759.md @@ -0,0 +1,46 @@ +--- +"@object-ui/create-plugin": patch +--- + +Remove the scaffold's unused pinned icon dependency, and make its generated schema interface reachable + +Two declared-but-unreachable artifacts in the generated plugin, both on the blind side of +the import gate objectui#3733 added — that gate rejects an import nothing declares, and +never looked for a declaration nothing imports. + +**The generated `dependencies` no longer pin `lucide-react`** (objectui#3755). It was +declared at `^0.563.0` and imported by no generated source file, so every freshly +scaffolded plugin really installed lucide 0.563.x for code that never referenced it — two +majors behind the 23 in-repo declarations, all `^1.28.0`. Worse than ordinary caret drift: +a `0.x` caret does not cross minors, so `^0.563.0` is `>=0.563.0 <0.564.0` and could not +float even within `0.x`. It is removed rather than re-anchored because this repo declares +an icon library where it imports one — of the 24 manifests mentioning `lucide-react`, 23 +import it, and none pre-declares it for code not yet written. An author who wants icons +runs `pnpm add lucide-react` and lands the current version by construction, with no anchor +table to maintain for an unused entry. The generated `dependencies` is now exactly the four +`workspace:*` platform packages, which cannot drift at all. + +**The generated `src/index.tsx` now re-exports the schema interface** from `src/types.ts` +(objectui#3759). The generated `exports` map exposes exactly one key — `.` — so the entry +is a consumer's only door, and nothing walked through it to `src/types.ts`: no generated +source imported it, and the deep paths that would have reached it (`/types`, +`/dist/types`) are closed by that same map. The interface in it is the plugin's schema +contract, and it shipped dead — while the generator's own documentation page told authors to +"export your schema types … make it importable rather than internal". A named type-only +re-export, matching the four in-repo plugins that ship a `src/types.ts` and the worked +example in the plugin-development guide. + +**That interface now extends `BaseSchema` from `@object-ui/types`** instead of re-declaring +a subset of the base node. Unreachable, a hand-rolled `{ type; id?; className? }` was only +dead weight; published, it would be a second dialect of a node the protocol already defines, +silently missing everything else `BaseSchema` carries (`name`, `label`, `visible`, …). Only +the `type` literal is narrowed locally, the same shape every in-repo plugin uses. This also +makes the generated `@object-ui/types` dependency a used declaration. + +Both halves are pinned structurally rather than by string match, so the next dead artifact +fails a test instead of shipping: no versioned runtime dependency may be declared that no +generated source imports (`workspace:*` exempt — it cannot drift), and no generated `src/**` +module may be unreachable from the single entry the `exports` map exposes. Each of those +gates passes over an empty result on today's templates, so each is paired with a self-test +that plants the removed defect back and asserts the rule names it — a gate that is green +because it produces nothing is not a gate. diff --git a/packages/create-plugin/src/__tests__/templates.test.ts b/packages/create-plugin/src/__tests__/templates.test.ts index 28df66fce..485ade531 100644 --- a/packages/create-plugin/src/__tests__/templates.test.ts +++ b/packages/create-plugin/src/__tests__/templates.test.ts @@ -16,6 +16,19 @@ * dependency of the generated package (the exact defect, generalised); * - the `setupFiles` path in the generated Vitest config must name a file the * generator actually writes. + * + * Two more structural gates close the blind spot on the OTHER side of that + * import check, which only ever caught undeclared imports and never declared + * things nothing used (objectui#3755, objectui#3759): + * + * - no versioned runtime dependency may be declared that no generated source + * imports (`workspace:*` exempt — it cannot drift); + * - no generated `src/**` module may be unreachable from `src/index.tsx`, the + * single entry the generated `exports` map exposes. + * + * Both would pass over an empty result on today's templates, so each is paired + * with a self-test that plants the removed defect back and asserts the rule + * names it. A gate that is green because it produces nothing is not a gate. */ import { describe, it, expect } from 'vitest'; import { existsSync, readFileSync, readdirSync } from 'node:fs'; @@ -25,6 +38,7 @@ import { fileURLToPath } from 'node:url'; import { VITEST_SETUP_FILE, + buildIndexFile, buildPackageJson, buildPluginFiles, buildTestFile, @@ -155,6 +169,101 @@ function generatedDevDependencies(vars: PluginTemplateVars): Record }).devDependencies; } +function generatedDependencies(vars: PluginTemplateVars): Record { + return (buildPackageJson(vars) as { dependencies: Record }).dependencies; +} + +/** + * Runtime dependencies pinned to a VERSION that no generated source imports. + * + * The reverse of the `import nothing the manifest does not declare` gate below. + * That one is one-way — objectui#3733 added it against undeclared imports, so + * `'lucide-react': '^0.563.0'` sat in the generated `dependencies` for as long + * as it did precisely because nothing looked in this direction (objectui#3755). + * + * `workspace:*` entries are exempt by design, not by oversight: they resolve to + * whatever this workspace currently builds, so an unused one costs nothing and + * cannot drift. A versioned range is the opposite — it is installed as written, + * and `^0.563.0` could not even float within `0.x` (a `0.x` caret is + * `>=0.563.0 <0.564.0`), so it stayed two majors behind the repo's own + * `^1.28.0` indefinitely. + * + * Returned rather than asserted so the rule can be exercised against a manifest + * that DOES violate it — see the self-test beside its use. Without that, the + * gate would pass by producing nothing (there are no versioned runtime ranges + * left) and would keep passing if it were broken. + */ +function unusedVersionedDependencies( + dependencies: Record, + files: Record +): string[] { + const imported = new Set(); + for (const [relativePath, contents] of Object.entries(files)) { + if (!/\.tsx?$/.test(relativePath)) continue; + for (const pkg of importedPackagesOf(contents)) imported.add(pkg); + } + return Object.entries(dependencies) + .filter(([name, range]) => !range.startsWith('workspace:') && !imported.has(name)) + .map(([name]) => name) + .sort(); +} + +/** + * Generated `src/**` modules NOT reachable from the entry the `exports` map names. + * + * objectui#3759's criterion, structurally. The generated manifest exposes one + * `exports` key (`.` → `dist/*`), so `src/index.tsx` is a consumer's only door; + * a module the entry does not pull in transitively is unreachable no matter what + * it contains. `src/types.ts` was written to disk in exactly that state — the + * deep paths that would have reached it (`/types`, `/dist/types`) are + * closed by the same `exports` map, so the plugin's schema contract shipped + * dead. The pre-existing file-map test asserted only that the path EXISTS, + * which is what let it stay dead. + * + * Test files are excluded from the roots on purpose: Vitest loads them + * directly, so entry-reachability is not required of them — and if they counted + * as roots, a module reachable only from a test would read as live while still + * being unreachable for every consumer. + */ +function unreachableGeneratedSources(files: Record): string[] { + const isSource = (path: string) => /^src\/.*\.tsx?$/.test(path) && !/\.test\.tsx?$/.test(path); + + const resolveRelative = (fromPath: string, specifier: string): string | undefined => { + const fromDir = fromPath.slice(0, fromPath.lastIndexOf('/')); + const segments = `${fromDir}/${specifier}`.split('/'); + const stack: string[] = []; + for (const segment of segments) { + if (segment === '.' || segment === '') continue; + if (segment === '..') stack.pop(); + else stack.push(segment); + } + const base = stack.join('/'); + return [base, `${base}.tsx`, `${base}.ts`, `${base}/index.tsx`, `${base}/index.ts`].find( + (candidate) => files[candidate] !== undefined + ); + }; + + const reached = new Set(); + const queue = ['src/index.tsx']; + while (queue.length > 0) { + const current = queue.pop() as string; + if (reached.has(current) || files[current] === undefined) continue; + reached.add(current); + // Covers `import … from './x'`, `import './x'` and `export … from './x'`, + // including the `export type { … } from './types'` form the entry uses. + for (const match of files[current].matchAll( + /(?:^|\n)\s*(?:import|export)\s+(?:type\s+)?(?:[^;'"]*?from\s+)?'(\.[^']*)'/g + )) { + const target = resolveRelative(current, match[1]); + if (target !== undefined) queue.push(target); + } + } + + return Object.keys(files) + .filter((path) => isSource(path) && !reached.has(path)) + .sort(); +} + function declaredDependencies(vars: PluginTemplateVars): Record { const pkg = buildPackageJson(vars) as { dependencies: Record; @@ -180,6 +289,49 @@ describe('generated package.json', () => { ); }); + it('declares exactly the four workspace platform packages as runtime dependencies', () => { + // objectui#3755. The whole map, not `arrayContaining`: a versioned runtime + // range added here has to fail this test and be argued for, which is what + // `'lucide-react': '^0.563.0'` never was. `workspace:*` throughout is the + // property that makes the generated manifest undriftable by construction — + // no anchor table needed, unlike the devDependencies above. + expect(generatedDependencies(VARS)).toEqual({ + '@object-ui/components': 'workspace:*', + '@object-ui/core': 'workspace:*', + '@object-ui/react': 'workspace:*', + '@object-ui/types': 'workspace:*' + }); + }); + + it('declares no versioned runtime dependency that the generated sources never import', () => { + // The other half of objectui#3733's one-way import gate. That gate rejects + // an import nothing declares; this one rejects a versioned declaration + // nothing imports — the direction that let lucide-react be installed by + // every scaffolded plugin for code that never referenced it (objectui#3755). + expect(unusedVersionedDependencies(generatedDependencies(VARS), buildPluginFiles(VARS))).toEqual( + [] + ); + }); + + it('catches an unused versioned runtime dependency when one is present', () => { + // Self-test, because the assertion above currently passes over an EMPTY + // set — there are no versioned runtime ranges left to judge. Without this, + // that gate would go green on a broken rule just as readily as on a clean + // manifest. Plants back the exact declaration objectui#3755 removed. + const files = buildPluginFiles(VARS); + const withLucide = { ...generatedDependencies(VARS), 'lucide-react': '^0.563.0' }; + expect(unusedVersionedDependencies(withLucide, files)).toEqual(['lucide-react']); + + // And does not fire on a versioned range the sources really do import, so + // the rule rejects unused declarations rather than versioned ones. + const withImported = { ...generatedDependencies(VARS), 'lucide-react': '^1.28.0' }; + const importingFiles = { + ...files, + 'src/Icon.tsx': `import { Flame } from 'lucide-react';\nexport const Icon = Flame;\n` + }; + expect(unusedVersionedDependencies(withImported, importingFiles)).toEqual([]); + }); + it('anchors every devDependency range, leaving none unpinned', () => { // The completeness gate. objectui#3733 pinned only the three testing // ranges, and the five build ranges beside them drifted one to two majors @@ -251,6 +403,23 @@ describe('generated sources', () => { } }); + it('derives the published schema interface from the protocols BaseSchema', () => { + // objectui#3759's second half. Reachable-from-the-entry made this interface + // the plugin's published contract; a hand-rolled `{ type; id?; className? }` + // in that position is a second dialect of a base node `@object-ui/types` + // already defines (AGENTS.md #0.1). It also silently omits everything else + // `BaseSchema` carries (`name`, `label`, `visible`, …). Every in-repo plugin + // that ships a `src/types.ts` extends it; `packages/plugin-markdown` is the + // closest model. Narrowing `type` to the registry key is the only local part. + const typesFile = buildPluginFiles(VARS)['src/types.ts']; + expect(typesFile).toContain(`import type { BaseSchema } from '@object-ui/types';`); + expect(typesFile).toContain('export interface HeatmapSchema extends BaseSchema {'); + expect(typesFile).toContain(`type: 'heatmap';`); + // The copied subset is gone rather than kept alongside `extends`. + expect(typesFile).not.toContain('id?: string;'); + expect(typesFile).not.toContain('className?: string;'); + }); + it('has its jest-dom matchers registered by the setup file', () => { expect(buildTestFile(VARS)).toContain('toBeInTheDocument'); expect(buildVitestSetup()).toContain('@testing-library/jest-dom'); @@ -315,6 +484,45 @@ describe('generated file map', () => { expect(files[VITEST_SETUP_FILE]).toContain(`import '@testing-library/jest-dom/vitest';`); }); + it('writes no source file that is unreachable from the packages only entry point', () => { + // objectui#3759. `src/types.ts` used to be written and then reachable from + // nowhere: no generated source imported it, and the `exports` map exposes + // only `.`, so `/types` and `/dist/types` were closed too. The + // interface in it is the plugin's schema contract — the one thing a + // metadata author needs from the package — and it shipped dead. + // + // Structural rather than a grep for the re-export line: any future template + // file added to the map has to be wired up to the entry (or be a test), + // instead of quietly becoming the next dead artifact. + expect(unreachableGeneratedSources(buildPluginFiles(VARS))).toEqual([]); + }); + + it('pins that entry-reachability is what makes a type consumable, via the exports map', () => { + // The premise the test above rests on. If a `./types` subpath were ever + // added to `exports`, entry-reachability would stop being the criterion — + // so the premise is asserted rather than assumed. + const pkg = buildPackageJson(VARS) as { exports: Record }; + expect(Object.keys(pkg.exports)).toEqual(['.']); + expect(buildIndexFile(VARS)).toContain(`export type { HeatmapSchema } from './types';`); + }); + + it('reports the schema module as unreachable when the entry stops re-exporting it', () => { + // Self-test for the gate above, which passes over an empty result: with the + // re-export line stripped, `src/types.ts` must be NAMED. This is the + // reverse verification of objectui#3759 encoded in the suite — restore the + // pre-fix entry and the gate goes red, rather than silently green because + // it stopped looking. + const files = buildPluginFiles(VARS); + const preFixEntry = files['src/index.tsx'].replace( + `export type { HeatmapSchema } from './types';\n`, + '' + ); + expect(preFixEntry).not.toContain(`from './types'`); + expect(unreachableGeneratedSources({ ...files, 'src/index.tsx': preFixEntry })).toEqual([ + 'src/types.ts' + ]); + }); + it('keeps every path inside the generated plugin directory', () => { // The CLI joins these onto the target dir, so a `..` segment here would // escape it — the same traversal the plugin-name validation guards against. diff --git a/packages/create-plugin/src/templates.ts b/packages/create-plugin/src/templates.ts index 801fc7697..a8c7aae65 100644 --- a/packages/create-plugin/src/templates.ts +++ b/packages/create-plugin/src/templates.ts @@ -94,6 +94,39 @@ const DEV_DEPENDENCIES: Record = { vitest: '^4.1.10' }; +/** + * Runtime `dependencies` written into the generated plugin. + * + * EVERY entry is a `workspace:*` platform package — the four `@object-ui/*` the + * scaffold's own sources build against. That is not an accident of the list, it + * is the rule: a generated plugin is written into `/packages/plugin-` + * and shares this workspace, so `workspace:*` always resolves to the version the + * repo currently builds and tests with, and can never drift. + * + * A VERSIONED runtime range here can drift, and one did. Until objectui#3755 the + * map also carried `'lucide-react': '^0.563.0'` — a declaration no generated + * source file imported, nailed two majors behind the 23 in-repo declarations + * (all `^1.28.0`) and unable to float off it, because a `0.x` caret does not + * cross minors: `^0.563.0` is `>=0.563.0 <0.564.0`. Every scaffolded plugin + * really installed lucide 0.563.x for code that never referenced it. + * + * It is gone rather than re-anchored because this repo declares an icon library + * where it imports one: of the 24 manifests that mention `lucide-react`, 23 + * import it, and no package pre-declares it for code not yet written. An author + * who wants icons runs `pnpm add lucide-react` and lands the current version by + * construction — no anchor table has to be maintained to keep an unused + * declaration honest. `templates.test.ts` pins both halves: this map is exactly + * the four `workspace:*` entries, and no versioned runtime range may be + * declared without a generated source importing it (the reverse of the one-way + * import gate objectui#3733 added, which only ever caught the other direction). + */ +const DEPENDENCIES: Record = { + '@object-ui/components': 'workspace:*', + '@object-ui/core': 'workspace:*', + '@object-ui/react': 'workspace:*', + '@object-ui/types': 'workspace:*' +}; + /** The generated plugin's `package.json`, as an object (not yet serialised). */ export function buildPackageJson(vars: PluginTemplateVars): Record { return { @@ -117,13 +150,7 @@ export function buildPackageJson(vars: PluginTemplateVars): Record/types`, `/dist/types`) are closed by that same + * `exports` map. The author's schema contract — the one thing a metadata + * producer needs from a renderer package — was a dead file. + * + * Named type-only re-export rather than `export * from './types'`, matching all + * four in-repo plugins that ship a `src/types.ts` (`plugin-charts`, + * `plugin-editor`, `plugin-kanban`, `plugin-markdown` — each + * `export type { XSchema } from './types';` behind the same `.`-only exports + * map) and the worked example in `content/docs/guide/plugin-development.md`. + * A star export would make every future local type public by accident; the + * named form keeps the published surface a deliberate act. + */ export function buildIndexFile(vars: PluginTemplateVars): string { return `/** * ObjectUI @@ -281,6 +327,7 @@ import { ${vars.pascalName} } from './${vars.pascalName}Impl'; export { ${vars.pascalName} }; export type { ${vars.pascalName}Props } from './${vars.pascalName}Impl'; +export type { ${vars.pascalName}Schema } from './types'; // Register component with ComponentRegistry const ${vars.pascalName}Renderer: React.FC<{ schema: any }> = ({ schema }) => { @@ -331,7 +378,25 @@ export const ${vars.pascalName}: React.FC<${vars.pascalName}Props> = ({ classNam `; } -/** The generated plugin's `src/types.ts`. */ +/** + * The generated plugin's `src/types.ts`. + * + * Extends `BaseSchema` from `@object-ui/types` instead of re-declaring the base + * node's fields. This became mandatory the moment objectui#3759 made the file + * reachable from the entry: an unreachable interface is only dead weight, but a + * PUBLISHED one is the plugin's schema contract, and shipping a hand-rolled + * `{ type; id?; className? }` as that contract would hand every scaffolded + * plugin a second dialect of a base node the protocol already defines — + * precisely the "one strict contract beats N dialects" failure in AGENTS.md + * commandment #0.1. `BaseSchema` already carries `id?` and `className?` (plus + * `name`, `label`, `visible`, … which a copied subset silently omits), so only + * the `type` literal is narrowed here. Same shape as every in-repo plugin that + * ships one — `packages/plugin-markdown/src/types.ts` is the closest model — + * and as the anatomy table in `content/docs/guide/plugin-development.md`. + * + * This is also what makes the generated `@object-ui/types` dependency a used + * declaration rather than a third unused one beside objectui#3755's. + */ export function buildTypesFile(vars: PluginTemplateVars): string { return `/** * ObjectUI @@ -341,13 +406,16 @@ export function buildTypesFile(vars: PluginTemplateVars): string { * LICENSE file in the root directory of this source tree. */ +import type { BaseSchema } from '@object-ui/types'; + /** - * Schema definition for ${vars.pascalName} + * Schema definition for ${vars.pascalName}. + * + * This is the contract between a metadata author and the renderer: it is + * re-exported from \`src/index.tsx\`, which is the package's only entry point. */ -export interface ${vars.pascalName}Schema { +export interface ${vars.pascalName}Schema extends BaseSchema { type: '${vars.pluginName}'; - id?: string; - className?: string; // Add schema properties here } `;