From 21a486a678b32f1619d341623ab40502bbde4ac7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 06:46:01 +0000 Subject: [PATCH 1/2] fix(cli): derive a parseable JS identifier for the emitted plugin symbol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `os create plugin ` interpolated the project name straight into an identifier position (`export const Plugin`), while the shared `validateProjectName` accepts exactly what npm accepts — a dot, an underscore and a leading digit included. So `os create plugin foo.bar` exited 0 having written `export const foo.barPlugin: Plugin = {`, a property access where a binding name belongs. Acceptance is unchanged: the emitted package name, its scope and the emitted directory name stay byte-for-byte what the user typed. Only the code identifier is normalised. `toCamelCase` folded `-x` into `X` and passed everything else through; `sanitizeIdentifier` generalises that fold to every run of non-identifier characters and prefixes a leading digit with `a`, the rule `sanitizeNamespace()` already uses. `my-app` still yields `myApp`. The emitted README now names the derived identifier in prose, so the mapping from package name to exported symbol is stated once where the user reads it. The pin drives TypeScript's own parser over the emitted bytes and asserts zero syntactic diagnostics, with `my-app` as a control in both directions and a canary that asserts the pre-fix bytes DO produce a diagnostic. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ --- packages/cli/src/commands/create.ts | 59 ++++- .../create-plugin-identifier-parses.test.ts | 209 ++++++++++++++++++ 2 files changed, 262 insertions(+), 6 deletions(-) create mode 100644 packages/cli/test/create-plugin-identifier-parses.test.ts diff --git a/packages/cli/src/commands/create.ts b/packages/cli/src/commands/create.ts index 250081090a..c7acf9a9a0 100644 --- a/packages/cli/src/commands/create.ts +++ b/packages/cli/src/commands/create.ts @@ -211,8 +211,49 @@ export function validateEmittedPackageName(packageName: string): string | null { ); } -function toCamelCase(str: string): string { - return str.replace(/-([a-z])/g, (g) => g[1].toUpperCase()); +/** + * The JavaScript identifier the emitted plugin's exported symbol is built from + * — DERIVED from the project name, never copied out of it. + * + * ## The defect this replaces + * + * `validateProjectName` accepts exactly what npm accepts, and that is correct: + * `foo.bar` is a legal npm package name and `@objectstack/plugin-foo.bar` is + * publishable. The same string is then interpolated into an *identifier* + * position (`export const Plugin`), where npm's charset is far wider + * than JavaScript's. The predecessor of this function folded `-x` into `X` and + * passed everything else straight through, so + * + * os create plugin foo.bar -> export const foo.barPlugin: Plugin = { + * + * exited 0 having written a property access where a binding name belongs. + * `1foo` (npm-legal) reached the same position as `1fooPlugin`, and `a_b` as + * `a_bPlugin` — legal, but not the camel fold the `-` case promises. + * + * ## The rule + * + * The fold is GENERALISED, not narrowed: every run of characters illegal in a + * JS identifier is the separator `-` already was — dropped, with the character + * after it upper-cased — and a leading digit takes the `'a'` prefix that + * `sanitizeNamespace()` (imported one line away) has always used for exactly + * this rule. Ordinary names are unchanged: `my-app` still yields `myApp`. + * + * ⛔ This normalises the CODE identifier and nothing else. The package name, + * its scope and the emitted directory name stay byte-for-byte what the user + * typed, and what `os create` accepts is unchanged. + * + * ⛔ No reserved-word handling, deliberately: every emission site appends + * `Plugin`, so the identifier that lands is never a bare keyword. + */ +export function sanitizeIdentifier(name: string): string { + const stem = name.replace(/^@[^/]+\//, ''); // drop an npm scope if present + let ident = stem.replace( + /[^A-Za-z0-9]+(.)?/g, + (_match: string, next?: string) => (next ? next.toUpperCase() : ''), + ); + if (!ident) ident = 'plugin'; + if (/^[0-9]/.test(ident)) ident = `a${ident}`; + return ident; } const PLUGIN_IN_REPO_DIR = 'packages/plugins'; @@ -278,7 +319,7 @@ export const templates: Record = { /** * ${name} Plugin for ObjectStack */ -export const ${toCamelCase(name)}Plugin: Plugin = { +export const ${sanitizeIdentifier(name)}Plugin: Plugin = { name: '${name}', version: '0.1.0', @@ -293,7 +334,7 @@ export const ${toCamelCase(name)}Plugin: Plugin = { }, }; -export default ${toCamelCase(name)}Plugin; +export default ${sanitizeIdentifier(name)}Plugin; `, 'README.md': (name: string) => `# @objectstack/plugin-${name} @@ -307,13 +348,19 @@ pnpm add @objectstack/plugin-${name} ## Usage +The plugin is exported as \`${sanitizeIdentifier(name)}Plugin\` — a JavaScript +identifier derived from the package name \`${name}\`. Characters that npm allows +in a package name but JavaScript does not allow in an identifier (a dot, a +hyphen, an underscore, a leading digit) are folded away, so the exported symbol +can differ from the name. + \`\`\`typescript -import { ${toCamelCase(name)}Plugin } from '@objectstack/plugin-${name}'; +import { ${sanitizeIdentifier(name)}Plugin } from '@objectstack/plugin-${name}'; // Use the plugin in your ObjectStack configuration export default { plugins: [ - ${toCamelCase(name)}Plugin, + ${sanitizeIdentifier(name)}Plugin, ], }; \`\`\` diff --git a/packages/cli/test/create-plugin-identifier-parses.test.ts b/packages/cli/test/create-plugin-identifier-parses.test.ts new file mode 100644 index 0000000000..d7a652f64e --- /dev/null +++ b/packages/cli/test/create-plugin-identifier-parses.test.ts @@ -0,0 +1,209 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#15892) — the project `os create plugin ` emits must PARSE, for + * every name the command accepts. + * + * ## The defect + * + * `validateProjectName` accepts exactly what npm accepts, on purpose: `.`, `_` + * and a leading digit are all legal in an npm package name, and + * `@objectstack/plugin-foo.bar` is publishable. The emitted identifier used to + * be the same string, copied: + * + * os create plugin foo.bar -> export const foo.barPlugin: Plugin = { + * + * exit 0, on a file that is not TypeScript — `foo.bar` in a binding position + * is a property access. The maintainer's ruling (#15892, decision batch #64) + * is that acceptance stays as npm's and the IDENTIFIER is derived, the way + * `sanitizeNamespace()` already derives a namespace. + * + * ## Why the instrument is TypeScript's own parser + * + * "Does it look like an identifier" is the judgement that produced the defect + * in the first place. `ts.createSourceFile` + `getSyntacticDiagnostics` asks + * the compiler instead, and asks it about the bytes the template actually + * emits rather than about a restatement of them. + * + * ⭐ The reading is only worth something because it CAN fail. Two controls: + * + * - `my-app` — an ordinary name, which must still yield exactly + * `myAppPlugin`. A sanitiser that changes today's correct output is a + * regression, not a fix, and a green parse would not notice. + * - THE CANARY — the pre-fix bytes (the raw name interpolated back into the + * identifier position) must produce at least one syntactic diagnostic. A + * harness that resolves nothing, or is handed the wrong text, reports zero + * diagnostics and reads exactly like a pass. + * + * ⚠️ `a_b` is in the ruling's list but does NOT discriminate on parseability: + * `a_bPlugin` was always legal TypeScript. It is asserted on the MAPPING + * instead (`a_b` -> `aB`), which is the half of the ruling it can fail. + * + * ## What this pin deliberately does not touch + * + * The emitted package name, its scope and the emitted directory name are the + * user's string byte-for-byte (#15530 / #15816) — asserted below, so a future + * edit that "fixes" the name instead of the identifier reddens here. + */ + +import { describe, expect, it } from 'vitest'; +import ts from 'typescript'; +import { + DEFAULT_PLACEMENT, + sanitizeIdentifier, + templates, + type ScaffoldPlacement, +} from '../src/commands/create.js'; +import { validateProjectName } from '../src/commands/init.js'; + +/** + * Syntactic (parse) diagnostics only — no lib, no resolution, no type layer. + * `noLib`/`noResolve` keep the verdict about the grammar of these bytes, which + * is the property the defect broke. + */ +function syntacticDiagnostics(fileName: string, source: string): readonly ts.Diagnostic[] { + const sourceFile = ts.createSourceFile( + fileName, + source, + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TS, + ); + const host: ts.CompilerHost = { + getSourceFile: (requested) => (requested === fileName ? sourceFile : undefined), + getDefaultLibFileName: () => 'lib.d.ts', + writeFile: () => {}, + getCurrentDirectory: () => '/', + getCanonicalFileName: (f) => f, + useCaseSensitiveFileNames: () => true, + getNewLine: () => '\n', + fileExists: (f) => f === fileName, + readFile: (f) => (f === fileName ? source : undefined), + }; + const program = ts.createProgram( + [fileName], + { noLib: true, noResolve: true, target: ts.ScriptTarget.Latest }, + host, + ); + return program.getSyntacticDiagnostics(sourceFile); +} + +/** Render one file of the `plugin` template for a name and placement. */ +function emit(file: string, name: string, placement: ScaffoldPlacement): string { + const render = templates.plugin.filesFor(placement)[file]; + if (!render) throw new Error(`the plugin template emits no ${file}`); + const content = render(name); + return typeof content === 'string' ? content : `${JSON.stringify(content, null, 2)}\n`; +} + +/** The fenced `typescript` block of the emitted README — emission sites 3 and 4. */ +function readmeTypescriptFence(readme: string): string { + const fence = readme.match(/^```typescript\n([\s\S]*?)^```/m); + if (!fence) throw new Error('the emitted README has no typescript fence'); + return fence[1]; +} + +function occurrences(haystack: string, needle: string): number { + return haystack.split(needle).length - 1; +} + +/** + * The ruling's cases, plus the mapping each one is really about. `my-app` is + * the control in BOTH directions — it must still produce `myAppPlugin`. + */ +const CASES: ReadonlyArray<{ name: string; identifier: string; why: string }> = [ + { name: 'foo.bar', identifier: 'fooBar', why: 'a dot is legal for npm, illegal in an identifier' }, + { name: '1foo', identifier: 'a1foo', why: 'a leading digit takes the fixed prefix' }, + { name: 'a_b', identifier: 'aB', why: 'an underscore folds the way a hyphen already did' }, + { name: 'my-app', identifier: 'myApp', why: 'CONTROL — today’s correct output must not move' }, +]; + +const PLACEMENTS: readonly ScaffoldPlacement[] = ['standalone', 'in-repo']; + +describe('`os create plugin ` emits a parseable identifier', () => { + it('accepts every case below — npm acceptance is unchanged by this fix', () => { + for (const { name } of CASES) { + expect(validateProjectName(name), name).toBeNull(); + } + }); + + it.each(CASES)('$name -> $identifier ($why)', ({ name, identifier }) => { + expect(sanitizeIdentifier(name)).toBe(identifier); + }); + + it.each(CASES)('emitted src/index.ts parses for $name', ({ name, identifier }) => { + for (const placement of PLACEMENTS) { + const source = emit('src/index.ts', name, placement); + const diagnostics = syntacticDiagnostics('index.ts', source); + expect( + diagnostics.map((d) => ts.flattenDiagnosticMessageText(d.messageText, ' ')), + `${name} @ ${placement}`, + ).toEqual([]); + expect(source).toContain(`export const ${identifier}Plugin: Plugin = {`); + expect(source).toContain(`export default ${identifier}Plugin;`); + } + }); + + it.each(CASES)('emitted README.md fence parses for $name', ({ name, identifier }) => { + const readme = emit('README.md', name, DEFAULT_PLACEMENT); + const diagnostics = syntacticDiagnostics('readme.ts', readmeTypescriptFence(readme)); + expect( + diagnostics.map((d) => ts.flattenDiagnosticMessageText(d.messageText, ' ')), + name, + ).toEqual([]); + expect(readme).toContain(`import { ${identifier}Plugin } from '@objectstack/plugin-${name}';`); + }); + + it.each(CASES)('names the derived identifier in the README prose for $name', ({ name, identifier }) => { + const readme = emit('README.md', name, DEFAULT_PLACEMENT); + const prose = readme.replace(/^```[\s\S]*?^```/gm, ''); + expect(prose).toContain(`\`${identifier}Plugin\``); + expect(prose).toContain(`\`${name}\``); + }); + + /** + * The ruling's four emission sites: `src/index.ts` x2, `README.md` x2 — plus + * the one prose mention the ruling also asks for, which is why the README + * count is three. A new site must be added here deliberately. + */ + it.each(CASES)('reaches every emission site for $name', ({ name, identifier }) => { + const index = emit('src/index.ts', name, DEFAULT_PLACEMENT); + const readme = emit('README.md', name, DEFAULT_PLACEMENT); + expect(occurrences(index, `${identifier}Plugin`)).toBe(2); + expect(occurrences(readme, `${identifier}Plugin`)).toBe(3); + // The defect's own shape, at the two sites that carry a binding. ⛔ Not a + // bare `${name}Plugin` substring test: `a1foo` legitimately CONTAINS + // `1foo`, so that spelling fails on a correct emission. + if (name !== identifier) { + expect(index).not.toContain(`export const ${name}Plugin`); + expect(readme).not.toContain(`import { ${name}Plugin }`); + } + }); + + it.each(CASES)('leaves the emitted package name and directory as typed for $name', ({ name }) => { + const manifest = JSON.parse(emit('package.json', name, DEFAULT_PLACEMENT)) as { name: string }; + expect(manifest.name).toBe(`@objectstack/plugin-${name}`); + expect(templates.plugin.dirName(name)).toBe(`plugin-${name}`); + }); + + /** + * CANARY — the pre-fix bytes. Without this, a harness that parsed the wrong + * text (or nothing at all) would report zero diagnostics for every case above + * and read as a pass. + */ + it('the parser reports the pre-fix emission as broken', () => { + const fixed = emit('src/index.ts', 'foo.bar', DEFAULT_PLACEMENT); + const preFix = fixed.split(`${sanitizeIdentifier('foo.bar')}Plugin`).join('foo.barPlugin'); + expect(preFix).toContain('export const foo.barPlugin: Plugin = {'); + expect(syntacticDiagnostics('index.ts', preFix).length).toBeGreaterThan(0); + }); + + /** + * `~` is npm-legal but `validateProjectName` does not admit it, so it never + * reaches an emission site. Recorded because the card asserted it does. + */ + it('a tilde is refused by the validator, not by the sanitiser', () => { + expect(validateProjectName('foo~bar')).not.toBeNull(); + expect(sanitizeIdentifier('foo~bar')).toBe('fooBar'); + }); +}); From b96b0620df27108306f9dd687fdd0700e4175d86 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 06:50:38 +0000 Subject: [PATCH 2/2] chore(changeset): patch @objectstack/cli for the derived plugin identifier Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ --- .changeset/quiet-donkeys-smoke.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/quiet-donkeys-smoke.md diff --git a/.changeset/quiet-donkeys-smoke.md b/.changeset/quiet-donkeys-smoke.md new file mode 100644 index 0000000000..cf48c5fcf8 --- /dev/null +++ b/.changeset/quiet-donkeys-smoke.md @@ -0,0 +1,9 @@ +--- +'@objectstack/cli': patch +--- + +`os create plugin ` now derives the exported plugin symbol as a JavaScript identifier rather than copying the project name into an identifier position. + +`validateProjectName` accepts exactly what npm accepts — a dot, an underscore and a leading digit included — so `os create plugin foo.bar` used to exit 0 having written `export const foo.barPlugin: Plugin = {`, a property access where a binding name belongs. The scaffolded project did not parse. + +What the command accepts is unchanged, and so is what it emits as a name: the package name, its scope and the project directory stay byte-for-byte what was typed. Only the code identifier is normalised — every run of characters that is legal in an npm name but illegal in a JavaScript identifier now folds the way `-` already did, and a leading digit takes an `a` prefix. Ordinary names are unaffected (`my-app` still exports `myAppPlugin`). The emitted README names the derived symbol in prose, so the mapping is stated where it is read.