Skip to content

Commit 21a486a

Browse files
committed
fix(cli): derive a parseable JS identifier for the emitted plugin symbol
`os create plugin <name>` interpolated the project name straight into an identifier position (`export const <name>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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
1 parent a5eccf9 commit 21a486a

2 files changed

Lines changed: 262 additions & 6 deletions

File tree

packages/cli/src/commands/create.ts

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,49 @@ export function validateEmittedPackageName(packageName: string): string | null {
211211
);
212212
}
213213

214-
function toCamelCase(str: string): string {
215-
return str.replace(/-([a-z])/g, (g) => g[1].toUpperCase());
214+
/**
215+
* The JavaScript identifier the emitted plugin's exported symbol is built from
216+
* — DERIVED from the project name, never copied out of it.
217+
*
218+
* ## The defect this replaces
219+
*
220+
* `validateProjectName` accepts exactly what npm accepts, and that is correct:
221+
* `foo.bar` is a legal npm package name and `@objectstack/plugin-foo.bar` is
222+
* publishable. The same string is then interpolated into an *identifier*
223+
* position (`export const <ident>Plugin`), where npm's charset is far wider
224+
* than JavaScript's. The predecessor of this function folded `-x` into `X` and
225+
* passed everything else straight through, so
226+
*
227+
* os create plugin foo.bar -> export const foo.barPlugin: Plugin = {
228+
*
229+
* exited 0 having written a property access where a binding name belongs.
230+
* `1foo` (npm-legal) reached the same position as `1fooPlugin`, and `a_b` as
231+
* `a_bPlugin` — legal, but not the camel fold the `-` case promises.
232+
*
233+
* ## The rule
234+
*
235+
* The fold is GENERALISED, not narrowed: every run of characters illegal in a
236+
* JS identifier is the separator `-` already was — dropped, with the character
237+
* after it upper-cased — and a leading digit takes the `'a'` prefix that
238+
* `sanitizeNamespace()` (imported one line away) has always used for exactly
239+
* this rule. Ordinary names are unchanged: `my-app` still yields `myApp`.
240+
*
241+
* ⛔ This normalises the CODE identifier and nothing else. The package name,
242+
* its scope and the emitted directory name stay byte-for-byte what the user
243+
* typed, and what `os create` accepts is unchanged.
244+
*
245+
* ⛔ No reserved-word handling, deliberately: every emission site appends
246+
* `Plugin`, so the identifier that lands is never a bare keyword.
247+
*/
248+
export function sanitizeIdentifier(name: string): string {
249+
const stem = name.replace(/^@[^/]+\//, ''); // drop an npm scope if present
250+
let ident = stem.replace(
251+
/[^A-Za-z0-9]+(.)?/g,
252+
(_match: string, next?: string) => (next ? next.toUpperCase() : ''),
253+
);
254+
if (!ident) ident = 'plugin';
255+
if (/^[0-9]/.test(ident)) ident = `a${ident}`;
256+
return ident;
216257
}
217258

218259
const PLUGIN_IN_REPO_DIR = 'packages/plugins';
@@ -278,7 +319,7 @@ export const templates: Record<string, CreateTemplate> = {
278319
/**
279320
* ${name} Plugin for ObjectStack
280321
*/
281-
export const ${toCamelCase(name)}Plugin: Plugin = {
322+
export const ${sanitizeIdentifier(name)}Plugin: Plugin = {
282323
name: '${name}',
283324
version: '0.1.0',
284325
@@ -293,7 +334,7 @@ export const ${toCamelCase(name)}Plugin: Plugin = {
293334
},
294335
};
295336
296-
export default ${toCamelCase(name)}Plugin;
337+
export default ${sanitizeIdentifier(name)}Plugin;
297338
`,
298339
'README.md': (name: string) => `# @objectstack/plugin-${name}
299340
@@ -307,13 +348,19 @@ pnpm add @objectstack/plugin-${name}
307348
308349
## Usage
309350
351+
The plugin is exported as \`${sanitizeIdentifier(name)}Plugin\` — a JavaScript
352+
identifier derived from the package name \`${name}\`. Characters that npm allows
353+
in a package name but JavaScript does not allow in an identifier (a dot, a
354+
hyphen, an underscore, a leading digit) are folded away, so the exported symbol
355+
can differ from the name.
356+
310357
\`\`\`typescript
311-
import { ${toCamelCase(name)}Plugin } from '@objectstack/plugin-${name}';
358+
import { ${sanitizeIdentifier(name)}Plugin } from '@objectstack/plugin-${name}';
312359
313360
// Use the plugin in your ObjectStack configuration
314361
export default {
315362
plugins: [
316-
${toCamelCase(name)}Plugin,
363+
${sanitizeIdentifier(name)}Plugin,
317364
],
318365
};
319366
\`\`\`
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* PIN (#15892) — the project `os create plugin <name>` emits must PARSE, for
5+
* every name the command accepts.
6+
*
7+
* ## The defect
8+
*
9+
* `validateProjectName` accepts exactly what npm accepts, on purpose: `.`, `_`
10+
* and a leading digit are all legal in an npm package name, and
11+
* `@objectstack/plugin-foo.bar` is publishable. The emitted identifier used to
12+
* be the same string, copied:
13+
*
14+
* os create plugin foo.bar -> export const foo.barPlugin: Plugin = {
15+
*
16+
* exit 0, on a file that is not TypeScript — `foo.bar` in a binding position
17+
* is a property access. The maintainer's ruling (#15892, decision batch #64)
18+
* is that acceptance stays as npm's and the IDENTIFIER is derived, the way
19+
* `sanitizeNamespace()` already derives a namespace.
20+
*
21+
* ## Why the instrument is TypeScript's own parser
22+
*
23+
* "Does it look like an identifier" is the judgement that produced the defect
24+
* in the first place. `ts.createSourceFile` + `getSyntacticDiagnostics` asks
25+
* the compiler instead, and asks it about the bytes the template actually
26+
* emits rather than about a restatement of them.
27+
*
28+
* ⭐ The reading is only worth something because it CAN fail. Two controls:
29+
*
30+
* - `my-app` — an ordinary name, which must still yield exactly
31+
* `myAppPlugin`. A sanitiser that changes today's correct output is a
32+
* regression, not a fix, and a green parse would not notice.
33+
* - THE CANARY — the pre-fix bytes (the raw name interpolated back into the
34+
* identifier position) must produce at least one syntactic diagnostic. A
35+
* harness that resolves nothing, or is handed the wrong text, reports zero
36+
* diagnostics and reads exactly like a pass.
37+
*
38+
* ⚠️ `a_b` is in the ruling's list but does NOT discriminate on parseability:
39+
* `a_bPlugin` was always legal TypeScript. It is asserted on the MAPPING
40+
* instead (`a_b` -> `aB`), which is the half of the ruling it can fail.
41+
*
42+
* ## What this pin deliberately does not touch
43+
*
44+
* The emitted package name, its scope and the emitted directory name are the
45+
* user's string byte-for-byte (#15530 / #15816) — asserted below, so a future
46+
* edit that "fixes" the name instead of the identifier reddens here.
47+
*/
48+
49+
import { describe, expect, it } from 'vitest';
50+
import ts from 'typescript';
51+
import {
52+
DEFAULT_PLACEMENT,
53+
sanitizeIdentifier,
54+
templates,
55+
type ScaffoldPlacement,
56+
} from '../src/commands/create.js';
57+
import { validateProjectName } from '../src/commands/init.js';
58+
59+
/**
60+
* Syntactic (parse) diagnostics only — no lib, no resolution, no type layer.
61+
* `noLib`/`noResolve` keep the verdict about the grammar of these bytes, which
62+
* is the property the defect broke.
63+
*/
64+
function syntacticDiagnostics(fileName: string, source: string): readonly ts.Diagnostic[] {
65+
const sourceFile = ts.createSourceFile(
66+
fileName,
67+
source,
68+
ts.ScriptTarget.Latest,
69+
true,
70+
ts.ScriptKind.TS,
71+
);
72+
const host: ts.CompilerHost = {
73+
getSourceFile: (requested) => (requested === fileName ? sourceFile : undefined),
74+
getDefaultLibFileName: () => 'lib.d.ts',
75+
writeFile: () => {},
76+
getCurrentDirectory: () => '/',
77+
getCanonicalFileName: (f) => f,
78+
useCaseSensitiveFileNames: () => true,
79+
getNewLine: () => '\n',
80+
fileExists: (f) => f === fileName,
81+
readFile: (f) => (f === fileName ? source : undefined),
82+
};
83+
const program = ts.createProgram(
84+
[fileName],
85+
{ noLib: true, noResolve: true, target: ts.ScriptTarget.Latest },
86+
host,
87+
);
88+
return program.getSyntacticDiagnostics(sourceFile);
89+
}
90+
91+
/** Render one file of the `plugin` template for a name and placement. */
92+
function emit(file: string, name: string, placement: ScaffoldPlacement): string {
93+
const render = templates.plugin.filesFor(placement)[file];
94+
if (!render) throw new Error(`the plugin template emits no ${file}`);
95+
const content = render(name);
96+
return typeof content === 'string' ? content : `${JSON.stringify(content, null, 2)}\n`;
97+
}
98+
99+
/** The fenced `typescript` block of the emitted README — emission sites 3 and 4. */
100+
function readmeTypescriptFence(readme: string): string {
101+
const fence = readme.match(/^```typescript\n([\s\S]*?)^```/m);
102+
if (!fence) throw new Error('the emitted README has no typescript fence');
103+
return fence[1];
104+
}
105+
106+
function occurrences(haystack: string, needle: string): number {
107+
return haystack.split(needle).length - 1;
108+
}
109+
110+
/**
111+
* The ruling's cases, plus the mapping each one is really about. `my-app` is
112+
* the control in BOTH directions — it must still produce `myAppPlugin`.
113+
*/
114+
const CASES: ReadonlyArray<{ name: string; identifier: string; why: string }> = [
115+
{ name: 'foo.bar', identifier: 'fooBar', why: 'a dot is legal for npm, illegal in an identifier' },
116+
{ name: '1foo', identifier: 'a1foo', why: 'a leading digit takes the fixed prefix' },
117+
{ name: 'a_b', identifier: 'aB', why: 'an underscore folds the way a hyphen already did' },
118+
{ name: 'my-app', identifier: 'myApp', why: 'CONTROL — today’s correct output must not move' },
119+
];
120+
121+
const PLACEMENTS: readonly ScaffoldPlacement[] = ['standalone', 'in-repo'];
122+
123+
describe('`os create plugin <name>` emits a parseable identifier', () => {
124+
it('accepts every case below — npm acceptance is unchanged by this fix', () => {
125+
for (const { name } of CASES) {
126+
expect(validateProjectName(name), name).toBeNull();
127+
}
128+
});
129+
130+
it.each(CASES)('$name -> $identifier ($why)', ({ name, identifier }) => {
131+
expect(sanitizeIdentifier(name)).toBe(identifier);
132+
});
133+
134+
it.each(CASES)('emitted src/index.ts parses for $name', ({ name, identifier }) => {
135+
for (const placement of PLACEMENTS) {
136+
const source = emit('src/index.ts', name, placement);
137+
const diagnostics = syntacticDiagnostics('index.ts', source);
138+
expect(
139+
diagnostics.map((d) => ts.flattenDiagnosticMessageText(d.messageText, ' ')),
140+
`${name} @ ${placement}`,
141+
).toEqual([]);
142+
expect(source).toContain(`export const ${identifier}Plugin: Plugin = {`);
143+
expect(source).toContain(`export default ${identifier}Plugin;`);
144+
}
145+
});
146+
147+
it.each(CASES)('emitted README.md fence parses for $name', ({ name, identifier }) => {
148+
const readme = emit('README.md', name, DEFAULT_PLACEMENT);
149+
const diagnostics = syntacticDiagnostics('readme.ts', readmeTypescriptFence(readme));
150+
expect(
151+
diagnostics.map((d) => ts.flattenDiagnosticMessageText(d.messageText, ' ')),
152+
name,
153+
).toEqual([]);
154+
expect(readme).toContain(`import { ${identifier}Plugin } from '@objectstack/plugin-${name}';`);
155+
});
156+
157+
it.each(CASES)('names the derived identifier in the README prose for $name', ({ name, identifier }) => {
158+
const readme = emit('README.md', name, DEFAULT_PLACEMENT);
159+
const prose = readme.replace(/^```[\s\S]*?^```/gm, '');
160+
expect(prose).toContain(`\`${identifier}Plugin\``);
161+
expect(prose).toContain(`\`${name}\``);
162+
});
163+
164+
/**
165+
* The ruling's four emission sites: `src/index.ts` x2, `README.md` x2 — plus
166+
* the one prose mention the ruling also asks for, which is why the README
167+
* count is three. A new site must be added here deliberately.
168+
*/
169+
it.each(CASES)('reaches every emission site for $name', ({ name, identifier }) => {
170+
const index = emit('src/index.ts', name, DEFAULT_PLACEMENT);
171+
const readme = emit('README.md', name, DEFAULT_PLACEMENT);
172+
expect(occurrences(index, `${identifier}Plugin`)).toBe(2);
173+
expect(occurrences(readme, `${identifier}Plugin`)).toBe(3);
174+
// The defect's own shape, at the two sites that carry a binding. ⛔ Not a
175+
// bare `${name}Plugin` substring test: `a1foo` legitimately CONTAINS
176+
// `1foo`, so that spelling fails on a correct emission.
177+
if (name !== identifier) {
178+
expect(index).not.toContain(`export const ${name}Plugin`);
179+
expect(readme).not.toContain(`import { ${name}Plugin }`);
180+
}
181+
});
182+
183+
it.each(CASES)('leaves the emitted package name and directory as typed for $name', ({ name }) => {
184+
const manifest = JSON.parse(emit('package.json', name, DEFAULT_PLACEMENT)) as { name: string };
185+
expect(manifest.name).toBe(`@objectstack/plugin-${name}`);
186+
expect(templates.plugin.dirName(name)).toBe(`plugin-${name}`);
187+
});
188+
189+
/**
190+
* CANARY — the pre-fix bytes. Without this, a harness that parsed the wrong
191+
* text (or nothing at all) would report zero diagnostics for every case above
192+
* and read as a pass.
193+
*/
194+
it('the parser reports the pre-fix emission as broken', () => {
195+
const fixed = emit('src/index.ts', 'foo.bar', DEFAULT_PLACEMENT);
196+
const preFix = fixed.split(`${sanitizeIdentifier('foo.bar')}Plugin`).join('foo.barPlugin');
197+
expect(preFix).toContain('export const foo.barPlugin: Plugin = {');
198+
expect(syntacticDiagnostics('index.ts', preFix).length).toBeGreaterThan(0);
199+
});
200+
201+
/**
202+
* `~` is npm-legal but `validateProjectName` does not admit it, so it never
203+
* reaches an emission site. Recorded because the card asserted it does.
204+
*/
205+
it('a tilde is refused by the validator, not by the sanitiser', () => {
206+
expect(validateProjectName('foo~bar')).not.toBeNull();
207+
expect(sanitizeIdentifier('foo~bar')).toBe('fooBar');
208+
});
209+
});

0 commit comments

Comments
 (0)