From 9f3c53501e1a2360c63de42ac54ce62cac5a41e6 Mon Sep 17 00:00:00 2001 From: aromko Date: Thu, 23 Jul 2026 16:49:52 +0200 Subject: [PATCH 01/11] feat(DST-1543): add data-driven codemod engine with theme primitives Version-agnostic primitives driven by a per-version manifest, anchored on ThemeComponent<'X'> declarations verifiably imported from @marigold/system: - restructure-to-slots: single-cva components become slot objects, the consumer's cva moves verbatim into the primary slot - swap-exact-classes: baseline slots swap to the new baseline only on a byte-exact per-slot match (proof the slot was never customized), with a -/+ token diff so renamed tokens are visible at a glance - stub-missing-slots: missing slot keys become cva({}) stubs; dropped and spread-hidden slots are reported, never guessed at - scaffold-component: theme files for new components, fully derived from the manifest slot list, with load-bearing layout classes injected - report primitives for dead theme keys and DOM-structure changes The v18 manifest is hand-written (delivery step 2); slot sets extracted from the Theme type, swap class strings extracted from the published v17.9.1 theme-rui, links pinned to commit 946dc9f30. Codegen (DST-1650) will generate this file for future majors. All edits are byte-preserving (magic-string): untouched consumer code is never re-printed, and re-running a migration is a no-op. --- packages/cli/src/lib/codemod/anchor.ts | 100 ++++ packages/cli/src/lib/codemod/codemod.test.ts | 335 +++++++++++ packages/cli/src/lib/codemod/engine.ts | 171 ++++++ packages/cli/src/lib/codemod/manifests/v18.ts | 544 ++++++++++++++++++ .../cli/src/lib/codemod/primitives/report.ts | 52 ++ .../primitives/restructure-to-slots.ts | 55 ++ .../codemod/primitives/scaffold-component.ts | 72 +++ .../codemod/primitives/stub-missing-slots.ts | 87 +++ .../codemod/primitives/swap-exact-classes.ts | 116 ++++ packages/cli/src/lib/codemod/test-helpers.ts | 10 + packages/cli/src/lib/codemod/types.ts | 133 +++++ packages/cli/src/lib/doctor/format.ts | 8 +- packages/cli/src/lib/edit-tsx.ts | 2 +- packages/cli/src/lib/format.ts | 7 + packages/cli/src/lib/tsx-ast.test.ts | 4 +- packages/cli/src/lib/tsx-ast.ts | 5 +- 16 files changed, 1690 insertions(+), 11 deletions(-) create mode 100644 packages/cli/src/lib/codemod/anchor.ts create mode 100644 packages/cli/src/lib/codemod/codemod.test.ts create mode 100644 packages/cli/src/lib/codemod/engine.ts create mode 100644 packages/cli/src/lib/codemod/manifests/v18.ts create mode 100644 packages/cli/src/lib/codemod/primitives/report.ts create mode 100644 packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts create mode 100644 packages/cli/src/lib/codemod/primitives/scaffold-component.ts create mode 100644 packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts create mode 100644 packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts create mode 100644 packages/cli/src/lib/codemod/test-helpers.ts create mode 100644 packages/cli/src/lib/codemod/types.ts diff --git a/packages/cli/src/lib/codemod/anchor.ts b/packages/cli/src/lib/codemod/anchor.ts new file mode 100644 index 0000000000..8fc913beac --- /dev/null +++ b/packages/cli/src/lib/codemod/anchor.ts @@ -0,0 +1,100 @@ +import { type AnyNode, collectImports, walk } from '../tsx-ast.js'; + +// Locating theme definitions in consumer code. The stable anchor is the +// `ThemeComponent<'X'>` type from @marigold/system — not file names or +// directory layout — so the codemods work regardless of how a consumer +// organizes their theme. Anything we cannot find this way is reported as a +// warning, never guessed at. + +export const MARIGOLD_SYSTEM = '@marigold/system'; +export const MARIGOLD_COMPONENTS = '@marigold/components'; + +/** + * Local name under which `exported` is imported from a Marigold package in + * this file (handles aliased imports), or null if not imported. + */ +export const marigoldLocalName = ( + file: AnyNode, + exported: string, + pkg: string = MARIGOLD_SYSTEM +): string | null => { + for (const imp of collectImports(file)) { + const src = (imp.source as { value?: string } | undefined)?.value; + if (src !== pkg) continue; + for (const s of (imp.specifiers as AnyNode[] | undefined) ?? []) { + if (s.type !== 'ImportSpecifier') continue; + const imported = (s.imported as { name?: string } | undefined)?.name; + const local = (s.local as { name?: string } | undefined)?.name; + if (imported === exported && local) return local; + } + } + return null; +}; + +export interface ThemeComponentDecl { + /** the component name, e.g. 'Checkbox' from ThemeComponent<'Checkbox'> */ + component: string; + /** the initializer (object literal of slots, or a single cva call) */ + init: AnyNode; +} + +// Babel 8 stores type arguments as `typeArguments`; Babel 7 used +// `typeParameters`. Accept both so a parser bump can't silently break us. +const typeArgs = (ref: AnyNode): AnyNode[] => + ( + (ref.typeArguments ?? ref.typeParameters) as + | { params?: AnyNode[] } + | undefined + )?.params ?? []; + +/** + * All `const X: ThemeComponent<'Name'> = ...` declarations in a file whose + * `ThemeComponent` verifiably comes from @marigold/system. + */ +export const findThemeComponents = (file: AnyNode): ThemeComponentDecl[] => { + const localName = marigoldLocalName(file, 'ThemeComponent'); + if (!localName) return []; + + const decls: ThemeComponentDecl[] = []; + walk(file, n => { + if (n.type !== 'VariableDeclarator') return; + const id = n.id as AnyNode | undefined; + const annotation = (id?.typeAnnotation as AnyNode | undefined) + ?.typeAnnotation as AnyNode | undefined; + if (!annotation || annotation.type !== 'TSTypeReference') return; + if ((annotation.typeName as { name?: string })?.name !== localName) return; + const [arg] = typeArgs(annotation); + const literal = arg?.literal as AnyNode | undefined; + if (arg?.type !== 'TSLiteralType' || literal?.type !== 'StringLiteral') { + return; + } + const init = n.init as AnyNode | undefined; + if (!init) return; + decls.push({ component: literal.value as string, init }); + }); + return decls; +}; + +export const objectProperties = (obj: AnyNode): AnyNode[] => + obj.type === 'ObjectExpression' ? ((obj.properties as AnyNode[]) ?? []) : []; + +/** key name of an ObjectProperty (identifier or string-literal key) */ +export const propertyName = (prop: AnyNode): string | null => { + if (prop.type !== 'ObjectProperty') return null; + const key = prop.key as AnyNode | undefined; + if (key?.type === 'Identifier') + return (key as { name?: string }).name ?? null; + if (key?.type === 'StringLiteral') { + return (key as { value?: string }).value ?? null; + } + return null; +}; + +/** render a slot name as an object key, quoting when not an identifier */ +export const asPropertyKey = (name: string): string => + /^[A-Za-z_$][\w$]*$/.test(name) ? name : `'${name}'`; + +export const isCvaCall = (node: AnyNode, cvaLocal: string | null): boolean => + node.type === 'CallExpression' && + (node.callee as AnyNode | undefined)?.type === 'Identifier' && + ((node.callee as { name?: string }).name ?? '') === cvaLocal; diff --git a/packages/cli/src/lib/codemod/codemod.test.ts b/packages/cli/src/lib/codemod/codemod.test.ts new file mode 100644 index 0000000000..20504b4167 --- /dev/null +++ b/packages/cli/src/lib/codemod/codemod.test.ts @@ -0,0 +1,335 @@ +import { type AnyNode, parseTsx } from '../tsx-ast.js'; +import { asPropertyKey, findThemeComponents } from './anchor.js'; +import { classTokens, detectIndentUnit, reindent } from './engine.js'; +import { v18 } from './manifests/v18.js'; +import { reportDeadKeys, reportStructure } from './primitives/report.js'; +import { restructureToSlots } from './primitives/restructure-to-slots.js'; +import { + addIndexExport, + generateScaffold, +} from './primitives/scaffold-component.js'; +import { stubMissingSlots } from './primitives/stub-missing-slots.js'; +import { swapExactClasses } from './primitives/swap-exact-classes.js'; +import { assertEdited } from './test-helpers.js'; +import type { CodemodOutcome } from './types.js'; + +// Fixtures mirror the b2c portal theme's style: 4-space indent, single +// quotes, one ThemeComponent<'X'> per file — the acceptance target of +// DST-1543. The byte-preserving invariant ("existing class strings survive +// byte-for-byte") is asserted on the portal's own class strings. + +const parse = (source: string): AnyNode => + parseTsx(source) as unknown as AnyNode; + +const PORTAL_CARD = `import { cva, ThemeComponent } from '@marigold/system'; + +export const Card: ThemeComponent<'Card'> = cva({ + base: ['bg-white shadow-[0_1px_6px_rgba(0,_0,_0,_0.117647)] rounded-xs'], + variants: { + variant: { + default: 'p-2', + dynamicPadding: 'p-4 sm:px-6' + } + }, + defaultVariants: { + variant: 'default' + } +}); +`; + +const PORTAL_SWITCH = `import { cva, ThemeComponent } from '@marigold/system'; + +export const Switch: ThemeComponent<'Switch'> = { + container: cva({ + base: 'disabled:cursor-not-allowed disabled:text-disabled-foreground' + }), + track: cva({ + base: [ + 'flex h-6 w-10 shrink-0 cursor-pointer items-center rounded-full', + 'group-selected/switch:bg-brand bg-input' + ] + }), + thumb: cva({ + base: 'pointer-events-none block size-5 rounded-full' + }) +}; +`; + +describe('anchor', () => { + test('finds ThemeComponent declarations with verified import origin', () => { + const decls = findThemeComponents(parse(PORTAL_SWITCH)); + expect(decls.map(d => d.component)).toEqual(['Switch']); + }); + + test('ignores ThemeComponent from other packages', () => { + const source = `import { cva, ThemeComponent } from 'other-system'; +export const Switch: ThemeComponent<'Switch'> = { container: cva({}) }; +`; + expect(findThemeComponents(parse(source))).toEqual([]); + }); + + test('resolves aliased imports', () => { + const source = `import { ThemeComponent as TC, cva } from '@marigold/system'; +export const Badge: TC<'Badge'> = cva({}); +`; + expect(findThemeComponents(parse(source)).map(d => d.component)).toEqual([ + 'Badge', + ]); + }); + + test('quotes non-identifier slot names', () => { + expect(asPropertyKey('container')).toBe('container'); + expect(asPropertyKey('bottom-left')).toBe(`'bottom-left'`); + }); +}); + +describe('engine', () => { + test('detects the indentation unit of a file', () => { + expect(detectIndentUnit(PORTAL_SWITCH)).toBe(' '); + expect(detectIndentUnit('const a = 1;\n')).toBe(' '); + }); + + test('reindents 2-space manifest source to the target unit and base', () => { + const out = reindent(`{\n base: [\n 'grid',\n ],\n}`, ' ', ' '); + expect(out).toBe(`{\n base: [\n 'grid',\n ],\n }`); + }); + + test('splits class strings into utility tokens', () => { + expect([...classTokens(['grid gap-x-2', 'grid'])]).toEqual([ + 'grid', + 'gap-x-2', + ]); + }); +}); + +describe('restructure-to-slots', () => { + test('wraps a single cva into the primary slot and stubs the rest', () => { + const result = restructureToSlots(v18).apply(PORTAL_CARD); + + assertEdited(result); + // the consumer's cva moved verbatim — byte-for-byte, incl. their classes + expect(result.output).toContain( + `'bg-white shadow-[0_1px_6px_rgba(0,_0,_0,_0.117647)] rounded-xs'` + ); + expect(result.output).toContain(' container: cva({'); + for (const slot of [ + 'header', + 'title', + 'description', + 'content', + 'footer', + 'media', + ]) { + expect(result.output).toContain(` ${slot}: cva({}),`); + } + expect(result.changes).toHaveLength(1); + }); + + test('produces parseable output that is a slot object', () => { + const result = restructureToSlots(v18).apply(PORTAL_CARD); + + assertEdited(result); + const [decl] = findThemeComponents(parse(result.output)); + expect(decl.init.type).toBe('ObjectExpression'); + }); + + test('leaves already-slotted components unchanged', () => { + const result = restructureToSlots(v18).apply(PORTAL_SWITCH); + expect(result.kind).toBe('unchanged'); + }); +}); + +describe('stub-missing-slots', () => { + test('adds missing slot keys as cva({}) stubs, preserving existing ones', () => { + const source = `import { cva, ThemeComponent } from '@marigold/system'; + +export const Input: ThemeComponent<'Input'> = { + input: cva({ base: 'border-input' }) +}; +`; + const result = stubMissingSlots(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain(`input: cva({ base: 'border-input' })`); + expect(result.output).toContain(' icon: cva({}),'); + expect(result.output).toContain(' action: cva({}),'); + }); + + test('warns about slot keys the target version dropped', () => { + const source = `import { cva, ThemeComponent } from '@marigold/system'; + +export const Tag: ThemeComponent<'Tag'> = { + container: cva({}), + tag: cva({}), + listItems: cva({}), + closeButton: cva({}), + showMore: cva({}), + removeAll: cva({ base: 'text-brand' }) +}; +`; + const result = stubMissingSlots(v18).apply(source); + + expect(result.kind).toBe('unchanged'); + expect( + (result as Extract).warnings[0] + ).toContain('slot `removeAll` no longer exists'); + }); + + test('bails to a warning naming the unverifiable slots when a spread is present', () => { + const source = `import { cva, ThemeComponent } from '@marigold/system'; +const shared = { icon: cva({}) }; +export const Input: ThemeComponent<'Input'> = { + ...shared, + input: cva({}) +}; +`; + const result = stubMissingSlots(v18).apply(source); + + expect(result.kind).toBe('unchanged'); + const [warning] = (result as Extract) + .warnings; + expect(warning).toContain('not visible in this file: `icon`, `action`'); + expect(warning).toContain( + 'themes/theme-rui/src/components/Input.styles.ts' + ); + }); + + test('stays silent when a spread is present but all slots are visible', () => { + const source = `import { cva, ThemeComponent } from '@marigold/system'; +const shared = { extra: cva({}) }; +export const Select: ThemeComponent<'Select'> = { + ...shared, + select: cva({}), + icon: cva({}) +}; +`; + const result = stubMissingSlots(v18).apply(source); + + expect(result.kind).toBe('unchanged'); + expect( + (result as Extract).warnings + ).toEqual([]); + }); + + test('handles single-line object literals', () => { + const source = `import { cva, ThemeComponent } from '@marigold/system'; +export const Select: ThemeComponent<'Select'> = { select: cva({}) }; +`; + const result = stubMissingSlots(v18).apply(source); + + assertEdited(result); + const [decl] = findThemeComponents(parse(result.output)); + expect(decl.init.type).toBe('ObjectExpression'); + expect(result.output).toContain('icon: cva({}),'); + }); + + test('leaves complete slot objects unchanged', () => { + const result = stubMissingSlots(v18).apply(PORTAL_SWITCH); + expect(result.kind).toBe('unchanged'); + }); +}); + +describe('swap-exact-classes', () => { + test('swaps a slot that matches the old baseline byte-for-byte', () => { + const result = swapExactClasses(v18).apply(PORTAL_SWITCH); + + assertEdited(result); + expect(result.output).toContain(`'grid gap-x-2 items-center'`); + // reindented to the portal's 4-space style + expect(result.output).toContain(` base: [`); + // the customized track/thumb slots stay byte-identical + expect(result.output).toContain( + `'group-selected/switch:bg-brand bg-input'` + ); + expect(result.changes).toEqual([ + 'Switch.container: swapped baseline styles to the v18 baseline', + ]); + }); + + test('emits a token diff of removed vs added utilities', () => { + const result = swapExactClasses(v18).apply(PORTAL_SWITCH); + + assertEdited(result); + const tokenWarning = result.warnings.find(w => w.includes('resolve')); + expect(tokenWarning).toContain('\n- disabled:text-disabled-foreground'); + expect(tokenWarning).toContain('\n+ '); + expect(tokenWarning).toContain('disabled:text-disabled '); + expect(tokenWarning).toContain('grid'); + // defaultVariants values are variant names, not classes — never listed + expect(tokenWarning).not.toMatch(/[\s+-]default(\s|$)/); + }); + + test('warns instead of editing when the slot was customized', () => { + const customized = PORTAL_SWITCH.replace( + 'disabled:cursor-not-allowed disabled:text-disabled-foreground', + 'disabled:cursor-not-allowed my-custom-class' + ); + const result = swapExactClasses(v18).apply(customized); + + expect(result.kind).toBe('unchanged'); + expect( + (result as Extract).warnings[0] + ).toContain('does not match the old Marigold baseline'); + }); + + test('ignores components without swap entries', () => { + const result = swapExactClasses(v18).apply(PORTAL_CARD); + expect(result.kind).toBe('unchanged'); + expect( + (result as Extract).warnings + ).toEqual([]); + }); +}); + +describe('scaffold-component', () => { + test('derives file content from the slot list with load-bearing classes', () => { + const entry = v18.scaffolds[0]; + const content = generateScaffold(entry, v18, ' '); + + expect(content).toContain( + `export const BooleanField: ThemeComponent<'BooleanField'> = {` + ); + expect(content).toContain(`'grid gap-x-2'`); // load-bearing layout + expect(content).toContain(`'col-start-2'`); + // parseable and anchored like hand-written theme files + expect(findThemeComponents(parse(content)).map(d => d.component)).toEqual([ + 'BooleanField', + ]); + }); + + test('adds the barrel export after the last export line, idempotently', () => { + const index = `export * from './Card.styles';\nexport * from './Switch.styles';\n`; + const codemod = addIndexExport('BooleanField.styles'); + + const first = codemod.apply(index); + assertEdited(first); + expect(first.output).toContain( + `export * from './Switch.styles';\nexport * from './BooleanField.styles';` + ); + + expect(codemod.apply(first.output).kind).toBe('unchanged'); + }); +}); + +describe('reports', () => { + test('flags removed components as dead keys', () => { + const source = `import { cva, ThemeComponent } from '@marigold/system'; +export const MultiSelect: ThemeComponent<'MultiSelect'> = cva({}); +`; + const result = reportDeadKeys(v18).apply(source); + + expect(result.kind).toBe('unchanged'); + expect( + (result as Extract).warnings[0] + ).toContain('removed in v18'); + }); + + test('emits structure warnings for themed components with DOM changes', () => { + const result = reportStructure(v18).apply(PORTAL_SWITCH); + + expect(result.kind).toBe('unchanged'); + expect( + (result as Extract).warnings[0] + ).toContain('Switch DOM'); + }); +}); diff --git a/packages/cli/src/lib/codemod/engine.ts b/packages/cli/src/lib/codemod/engine.ts new file mode 100644 index 0000000000..f9a88158bb --- /dev/null +++ b/packages/cli/src/lib/codemod/engine.ts @@ -0,0 +1,171 @@ +import MagicString from 'magic-string'; +import { insertImport } from '../edit-tsx.js'; +import { type AnyNode, collectImports, parseTsx, walk } from '../tsx-ast.js'; +import { + asPropertyKey, + findThemeComponents, + marigoldLocalName, +} from './anchor.js'; +import type { Codemod, CodemodOutcome, MigrationManifest } from './types.js'; + +// Source-text utilities shared by the codemod primitives. Everything here is +// formatting-aware but byte-preserving: consumer code outside an edited span +// is never re-printed (magic-string), and moved code is moved verbatim. + +/** parse, or return the uniform skipped outcome every primitive must honor */ +export const parseOr = ( + source: string, + run: (file: AnyNode) => CodemodOutcome +): CodemodOutcome => { + let ast; + try { + ast = parseTsx(source); + } catch (err) { + return { + kind: 'skipped', + reason: `parse error: ${(err as Error).message}`, + }; + } + return run(ast as unknown as AnyNode); +}; + +export interface ThemeVisit { + component: string; + /** the initializer (object literal of slots, or a single cva call) */ + init: AnyNode; + file: AnyNode; + source: string; + s: MagicString; + unit: string; + changes: string[]; + warnings: string[]; +} + +/** + * The shared frame of every theme-editing primitive: cheap pre-check (the + * anchor requires a literal `ThemeComponent` import), parse-or-skip, one + * MagicString, a visit per anchored component, and the uniform + * unchanged/edited outcome. `ensureCva` adds the cva import when edits + * introduced stubs. + */ +export const themeCodemod = ( + name: string, + visit: (ctx: ThemeVisit) => void, + options: { ensureCva?: boolean } = {} +): Codemod => ({ + name, + apply: source => { + if (!source.includes('ThemeComponent')) { + return { kind: 'unchanged', warnings: [] }; + } + return parseOr(source, file => { + const s = new MagicString(source); + const unit = detectIndentUnit(source); + const changes: string[] = []; + const warnings: string[] = []; + for (const { component, init } of findThemeComponents(file)) { + visit({ component, init, file, source, s, unit, changes, warnings }); + } + if (changes.length === 0) return { kind: 'unchanged', warnings }; + if (options.ensureCva) ensureCvaImport(s, file); + return { kind: 'edited', output: s.toString(), changes, warnings }; + }); + }, +}); + +/** + * Class-string literals of a cva() argument, in source order — skipping the + * `defaultVariants` subtree, whose values are variant names (`'default'`), + * not classes. + */ +export const classStringsIn = (node: AnyNode): string[] => { + const out: string[] = []; + walk(node, n => { + if ( + n.type === 'ObjectProperty' && + (n.key as AnyNode | undefined as { name?: string } | undefined)?.name === + 'defaultVariants' + ) { + return false; + } + if (n.type === 'StringLiteral') out.push(n.value as string); + }); + return out; +}; + +/** individual class utilities used across a set of class strings */ +export const classTokens = (classes: string[]): Set => + new Set(classes.flatMap(s => s.split(/\s+/)).filter(Boolean)); + +/** first indentation unit found in the file; two spaces when none found */ +export const detectIndentUnit = (source: string): string => + /\n([ \t]+)\S/.exec(source)?.[1].slice(0, 4) ?? ' '; + +/** leading whitespace of the line containing `offset` */ +export const lineIndentAt = (source: string, offset: number): string => { + const lineStart = source.lastIndexOf('\n', offset - 1) + 1; + const match = /^[ \t]*/.exec(source.slice(lineStart, offset)); + return match ? match[0] : ''; +}; + +/** + * Re-indent manifest source (written with 2-space indentation) to the + * consumer file's indentation: depth n at 2 spaces becomes `base` + + * `unit` * n. The first line is left as-is (it lands mid-line). + */ +export const reindent = (text: string, unit: string, base: string): string => { + const [first, ...rest] = text.split('\n'); + if (rest.length === 0) return text; + const relined = rest.map(line => { + const leading = /^ */.exec(line)![0].length; + return base + unit.repeat(Math.floor(leading / 2)) + line.slice(leading); + }); + return [first, ...relined].join('\n'); +}; + +/** + * Parse a manifest `newSource` cva-argument expression so its class strings + * can be inspected. Returns null when the manifest source does not parse + * (a manifest authoring error surfaced as a warning, not a crash). + */ +export const parseExpression = (expr: string): AnyNode | null => { + try { + const ast = parseTsx(`(${expr})`) as unknown as AnyNode; + const body = (ast.program as { body?: AnyNode[] } | undefined)?.body; + const statement = body?.[0]; + if (statement?.type !== 'ExpressionStatement') return null; + return (statement.expression as AnyNode) ?? null; + } catch { + return null; + } +}; + +/** ensure `cva` is imported from @marigold/system, inserting if needed */ +export const ensureCvaImport = (s: MagicString, file: AnyNode): string => { + const local = marigoldLocalName(file, 'cva'); + if (local) return local; + insertImport( + s, + file, + collectImports(file), + `import { cva } from '@marigold/system';` + ); + return 'cva'; +}; + +/** render a list of names as `code` spans for the report */ +export const codeList = (names: string[]): string => + names.map(name => `\`${name}\``).join(', '); + +/** the empty-stub property line shared by restructure and stubbing */ +export const stubSlotLine = (slot: string, indent: string): string => + `${indent}${asPropertyKey(slot)}: cva({}),`; + +/** deep link to the default theme's styles for a component, if configured */ +export const stylesReference = ( + manifest: MigrationManifest, + component: string +): string | null => + manifest.stylesReferenceUrl + ? `${manifest.stylesReferenceUrl}/${component}.styles.ts` + : null; diff --git a/packages/cli/src/lib/codemod/manifests/v18.ts b/packages/cli/src/lib/codemod/manifests/v18.ts new file mode 100644 index 0000000000..e2913e6d8f --- /dev/null +++ b/packages/cli/src/lib/codemod/manifests/v18.ts @@ -0,0 +1,544 @@ +import type { MigrationManifest } from '../types.js'; + +// Hand-written v18 manifest (DST-1543 delivery step 2). Codegen (step 3) will +// derive `slots`, `restructures`, `removedComponents` and the swap class +// strings from the Theme-type/theme-rui diff between the previous major and +// the current one; regenerating this file and diffing it against this +// hand-written version is codegen's own acceptance test. +// +// Data provenance: +// - slots: extracted from packages/system/src/types/theme.ts (v18) +// - swap oldClasses: theme-rui at git tag @marigold/components@17.9.1 +// - swap newSource / scaffold loadBearing: current theme-rui +// - warning links: GitHub permalinks pinned to commit 946dc9f30 (on +// beta-release; line numbers verified against it). Codegen should +// re-resolve these against the final release tag. +// Calendar and RangeCalendar share the same slot set in v18. +const calendarSlots = [ + 'calendar', + 'calendarContainer', + 'calendarMonth', + 'calendarListboxButton', + 'calendarCell', + 'calendarControllers', + 'calendarHeader', + 'calendarGrid', + 'calendarHeading', + 'calendarPresets', + 'select', +]; + +export const v18: MigrationManifest = { + schemaVersion: 1, + version: 'v18', + stylesReferenceUrl: + 'https://github.com/marigold-ui/marigold/blob/946dc9f30/themes/theme-rui/src/components', + slots: { + Accordion: [ + 'container', + 'item', + 'header', + 'panel', + 'content', + 'icon', + 'actions', + ], + ActionBar: ['container', 'selection', 'count', 'toolbar', 'clearButton'], + Badge: null, + Breadcrumbs: ['container', 'item', 'link', 'current'], + Button: null, + Card: [ + 'container', + 'header', + 'title', + 'description', + 'content', + 'footer', + 'media', + ], + CloseButton: null, + Collapsible: ['container', 'trigger', 'content'], + ContextualHelp: ['trigger', 'container', 'title', 'description', 'content'], + DateField: ['segment', 'field', 'input', 'action'], + Dialog: [ + 'closeButton', + 'container', + 'header', + 'content', + 'actions', + 'title', + 'description', + ], + Divider: null, + Drawer: [ + 'overlay', + 'closeButton', + 'container', + 'header', + 'title', + 'description', + 'content', + 'actions', + ], + Tray: [ + 'overlay', + 'container', + 'dragHandle', + 'header', + 'title', + 'description', + 'content', + 'actions', + ], + Field: null, + BooleanField: ['container', 'description'], + Headline: null, + Popover: null, + HelpText: ['container', 'icon'], + IconButton: null, + Checkbox: ['container', 'label', 'checkbox', 'group'], + Switch: ['container', 'track', 'thumb'], + Input: ['input', 'icon', 'action'], + Keyboard: null, + Label: null, + List: ['ol', 'ul', 'item'], + Link: null, + ListBox: [ + 'container', + 'list', + 'item', + 'section', + 'header', + 'label', + 'description', + ], + Menu: [ + 'container', + 'section', + 'item', + 'button', + 'label', + 'description', + 'keyboard', + ], + Modal: null, + Panel: [ + 'root', + 'header', + 'title', + 'description', + 'actions', + 'content', + 'collapsible', + 'collapsibleHeader', + 'collapsibleTitle', + 'collapsibleDescription', + 'collapsibleContent', + 'collapsibleIcon', + 'footer', + ], + Page: ['root', 'header', 'title', 'description', 'actions', 'content'], + Pagination: [ + 'container', + 'navigationButton', + 'pageButton', + 'icon', + 'ellipsis', + ], + ProgressCircle: ['container', 'loader', 'label'], + Radio: ['container', 'label', 'radio', 'group'], + Slider: ['container', 'track', 'thumb', 'output', 'selectedTrack'], + Select: ['select', 'icon'], + SelectList: [ + 'container', + 'list', + 'item', + 'label', + 'description', + 'indicator', + 'action', + ], + NumberField: ['group', 'stepper', 'input'], + SectionMessage: ['container', 'icon', 'title', 'description', 'content'], + Table: [ + 'table', + 'head', + 'column', + 'body', + 'footer', + 'row', + 'cell', + 'dragHandle', + 'dragPreview', + 'dragPreviewCounter', + 'dropIndicator', + 'editablePopover', + 'editTrigger', + 'editCancel', + 'editSave', + ], + LegacyTable: [ + 'table', + 'headerRow', + 'header', + 'thead', + 'body', + 'row', + 'cell', + ], + Tag: ['container', 'tag', 'listItems', 'closeButton', 'showMore'], + TagField: ['trigger', 'tagGroup', 'listItems', 'container'], + Text: null, + TextArea: null, + Tooltip: ['container', 'arrow'], + Toast: [ + 'toast', + 'title', + 'description', + 'closeButton', + 'icon', + 'content', + 'bottom-left', + 'bottom-right', + 'top-left', + 'top-right', + 'top', + 'bottom', + 'action', + ], + Tabs: [ + 'container', + 'tabsList', + 'tabsListScroll', + 'tabpanel', + 'tab', + 'tabIndicator', + ], + Underlay: null, + Calendar: calendarSlots, + RangeCalendar: calendarSlots, + DatePicker: null, + DateRangePicker: null, + ComboBox: ['icon', 'mobileTrigger'], + Autocomplete: ['mobileTrigger'], + Loader: ['container', 'loader', 'label'], + FileField: [ + 'container', + 'dropZone', + 'dropZoneContent', + 'dropZoneLabel', + 'item', + 'itemLabel', + 'itemDescription', + 'itemRemove', + ], + EmptyState: ['container', 'title', 'description', 'action'], + ToggleButton: ['group', 'button'], + SegmentedControl: ['group', 'list', 'field', 'option', 'indicator'], + Sidebar: [ + 'root', + 'overlay', + 'modal', + 'closeButton', + 'header', + 'nav', + 'footer', + 'toggle', + 'separator', + 'groupLabel', + 'navPanel', + 'navLink', + 'backButton', + 'content', + ], + TopNavigation: ['container', 'start', 'middle', 'end'], + }, + restructures: [{ component: 'Card', primarySlot: 'container' }], + scaffolds: [ + { + name: 'BooleanField', + requiredBy: ['Checkbox', 'Switch'], + reason: + 'new in v18: `Checkbox` and `Switch` render a `BooleanField` wrapper internally (label/description grid), and a theme without its styles throws at runtime', + // BooleanField is purely structural (grid columns, col-start), so both + // slots are load-bearing and carry the v18 layout verbatim. + loadBearing: { + container: `{ + base: 'grid gap-x-2', + variants: { + variant: { + default: 'grid-cols-[auto_1fr]', + settings: 'grid-cols-[1fr_auto]', + }, + }, + defaultVariants: { + variant: 'default', + }, +}`, + description: `{ + base: 'mt-0.5', + variants: { + variant: { + default: 'col-start-2', + settings: 'col-start-1', + }, + }, + defaultVariants: { + variant: 'default', + }, +}`, + }, + }, + ], + removedComponents: ['MultiSelect'], + swaps: [ + { + component: 'Checkbox', + slot: 'container', + oldClasses: ['cursor-pointer read-only:cursor-default gap-2'], + newSource: `{ + base: [ + 'grid grid-cols-[auto_1fr] gap-x-2 items-start', + 'cursor-pointer read-only:cursor-default', + 'group-data-[booleanfield]/booleanfield:grid-cols-subgrid group-data-[booleanfield]/booleanfield:col-span-full', + 'group-data-[orientation=vertical]/checkboxgroup:py-1', + 'group-data-[orientation=horizontal]/checkboxgroup:px-1.5', + ], +}`, + }, + { + component: 'Switch', + slot: 'container', + oldClasses: [ + 'disabled:cursor-not-allowed disabled:text-disabled-foreground', + ], + newSource: `{ + base: [ + 'grid gap-x-2 items-center', + 'disabled:cursor-not-allowed disabled:text-disabled', + 'group-data-booleanfield/booleanfield:grid-cols-subgrid group-data-booleanfield/booleanfield:col-span-full', + ], + variants: { + variant: { + default: 'grid-cols-[auto_1fr]', + settings: 'grid-cols-[1fr_auto]', + }, + }, + defaultVariants: { + variant: 'default', + }, +}`, + }, + ], + structureWarnings: [ + { + component: 'Switch', + text: 'v18 changes the Switch DOM: the toggle/label order and wrapper structure changed (grid layout, BooleanField wrapper for descriptions). CSS selectors targeting the old DOM (e.g. generated BEM selectors) must be reviewed. New DOM: https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Switch/Switch.tsx', + }, + { + component: 'Card', + text: 'v18 restructures the Card DOM into compound slots (Card.Header/Title/Description/Content/Footer/Media). CSS selectors targeting the old single-container DOM must be reviewed. New DOM: https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Card/Card.tsx', + }, + ], + // Application-code changes. Only lexically decidable renames/removals are + // edits; everything needing a structural JSX move (Tooltip open, Card + // compound) or a design decision stays a warning. + jsx: { + renames: [ + { component: 'Inset', from: 'space', to: 'p' }, + { component: 'Inset', from: 'spaceX', to: 'px' }, + { component: 'Inset', from: 'spaceY', to: 'py' }, + { + component: 'FileField', + from: 'acceptedFileType', + to: 'acceptedFileTypes', + wrapInArray: true, + }, + { + component: 'FileTrigger', + from: 'acceptedFileType', + to: 'acceptedFileTypes', + wrapInArray: true, + }, + ], + memberRenames: [ + { object: 'Tabs', from: 'TabPanel', to: 'Panel' }, + { object: 'SelectList', from: 'Item', to: 'Option' }, + ], + // v18 icon migration (@marigold/icons became a lucide-react proxy). + // Mapping extracted from the official table in + // .changeset/iconography-docs.md — never hand-typed. Identity mappings + // (Search, Check, ...) are omitted; unaliased imports are rewritten as + // `New as Old` per the release-notes recommendation. + importRenames: [ + { package: '@marigold/icons', from: 'Add', to: 'Plus' }, + { package: '@marigold/icons', from: 'BurgerMenu', to: 'Menu' }, + { package: '@marigold/icons', from: 'CircleChecked', to: 'CircleDot' }, + { package: '@marigold/icons', from: 'CircleUnchecked', to: 'Circle' }, + { package: '@marigold/icons', from: 'Delete', to: 'Trash2' }, + { package: '@marigold/icons', from: 'Filter', to: 'ListFilter' }, + { package: '@marigold/icons', from: 'IconMore', to: 'Ellipsis' }, + { package: '@marigold/icons', from: 'Remove', to: 'Minus' }, + { + package: '@marigold/icons', + from: 'SettingDots', + to: 'EllipsisVertical', + }, + { package: '@marigold/icons', from: 'SquareChecked', to: 'SquareCheck' }, + { package: '@marigold/icons', from: 'SquareUnchecked', to: 'Square' }, + { package: '@marigold/icons', from: 'Accessible', to: 'Accessibility' }, + { package: '@marigold/icons', from: 'AutoRenew', to: 'RefreshCcw' }, + { package: '@marigold/icons', from: 'Banned', to: 'Ban' }, + { package: '@marigold/icons', from: 'BatteryEmpty', to: 'BatteryLow' }, + { package: '@marigold/icons', from: 'BatteryHalf', to: 'BatteryMedium' }, + { package: '@marigold/icons', from: 'Bus', to: 'BusFront' }, + { + package: '@marigold/icons', + from: 'Direction', + to: 'SquareArrowUpRight', + }, + { package: '@marigold/icons', from: 'Email', to: 'Mail' }, + { package: '@marigold/icons', from: 'EventDate', to: 'Calendar1' }, + { package: '@marigold/icons', from: 'Exclamation', to: 'TriangleAlert' }, + { package: '@marigold/icons', from: 'Feedback', to: 'MessageSquareMore' }, + { package: '@marigold/icons', from: 'Food', to: 'Utensils' }, + { package: '@marigold/icons', from: 'Home', to: 'House' }, + { package: '@marigold/icons', from: 'Marker', to: 'MapPin' }, + { package: '@marigold/icons', from: 'MobilePhone', to: 'Smartphone' }, + { package: '@marigold/icons', from: 'MobileSignal', to: 'SignalHigh' }, + { + package: '@marigold/icons', + from: 'Notification', + to: 'MessageSquareWarning', + }, + { package: '@marigold/icons', from: 'Parking', to: 'CircleParking' }, + { package: '@marigold/icons', from: 'Reports', to: 'FileText' }, + { package: '@marigold/icons', from: 'Required', to: 'Asterisk' }, + { package: '@marigold/icons', from: 'ResaleLogbook', to: 'BookOpenText' }, + { package: '@marigold/icons', from: 'Spinner', to: 'Loader' }, + { package: '@marigold/icons', from: 'Thumb', to: 'ThumbsUp' }, + { package: '@marigold/icons', from: 'Cancel', to: 'CircleX' }, + { package: '@marigold/icons', from: 'Edit', to: 'Pencil' }, + { + package: '@marigold/icons', + from: 'ExportFile', + to: 'SquareArrowOutUpRight', + }, + { package: '@marigold/icons', from: 'FormatBold', to: 'Bold' }, + { package: '@marigold/icons', from: 'FormatItalic', to: 'Italic' }, + { package: '@marigold/icons', from: 'FormatSize', to: 'ALargeSmall' }, + { package: '@marigold/icons', from: 'HighlightOff', to: 'Power' }, + { package: '@marigold/icons', from: 'Location', to: 'LocateFixed' }, + { package: '@marigold/icons', from: 'Logout', to: 'LogOut' }, + { package: '@marigold/icons', from: 'Picture', to: 'Image' }, + { package: '@marigold/icons', from: 'ResaleEdit', to: 'Cog' }, + { package: '@marigold/icons', from: 'Restart', to: 'RotateCcw' }, + { package: '@marigold/icons', from: 'RotateLeft', to: 'RotateCcw' }, + { package: '@marigold/icons', from: 'RotateRight', to: 'RotateCw' }, + { package: '@marigold/icons', from: 'Sort', to: 'ChevronsUpDown' }, + { package: '@marigold/icons', from: 'SortDown', to: 'ChevronDown' }, + { package: '@marigold/icons', from: 'SortUp', to: 'ChevronUp' }, + { package: '@marigold/icons', from: 'Stop', to: 'CircleStop' }, + { package: '@marigold/icons', from: 'Underlined', to: 'Underline' }, + { package: '@marigold/icons', from: 'Zoom', to: 'ZoomIn' }, + { package: '@marigold/icons', from: 'Deal', to: 'BadgePercent' }, + { package: '@marigold/icons', from: 'Membership', to: 'IdCardLanyard' }, + { package: '@marigold/icons', from: 'Pickup', to: 'Store' }, + { package: '@marigold/icons', from: 'Price', to: 'Euro' }, + { package: '@marigold/icons', from: 'Seat', to: 'Armchair' }, + { package: '@marigold/icons', from: 'Selling', to: 'Tag' }, + { package: '@marigold/icons', from: 'Cart', to: 'ShoppingCart' }, + { package: '@marigold/icons', from: 'Group', to: 'UsersRound' }, + { package: '@marigold/icons', from: 'Id', to: 'IdCard' }, + { package: '@marigold/icons', from: 'SmilieDissatisfied', to: 'Frown' }, + { package: '@marigold/icons', from: 'SmilieNeutral', to: 'Meh' }, + { package: '@marigold/icons', from: 'SmilieSatisfied', to: 'Smile' }, + { + package: '@marigold/icons', + from: 'SmilieVeryDissatisfied', + to: 'Angry', + }, + { package: '@marigold/icons', from: 'SmilieVerySatisfied', to: 'Laugh' }, + { package: '@marigold/icons', from: 'User', to: 'UserRound' }, + { package: '@marigold/icons', from: 'Share', to: 'Share2' }, + { + package: '@marigold/icons', + from: 'Print', + to: 'Printer', + note: 'missing from the official mapping table — Printer is the Lucide equivalent', + }, + ], + removals: [ + { + component: 'TextField', + prop: 'min', + note: 'unsupported in v18; use NumberField for numeric constraints, see https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/NumberField/NumberField.tsx#L22', + }, + { + component: 'TextField', + prop: 'max', + note: 'unsupported in v18; use NumberField for numeric constraints, see https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/NumberField/NumberField.tsx#L22', + }, + ], + warnings: [ + { + component: 'Switch', + prop: 'size', + value: 'large', + text: 'the default v18 theme removed size="large". If the intent was a settings-style switch, use variant="settings"; a standalone theme defining its own size variants can keep the prop. See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Switch/Switch.tsx#L111', + }, + { + component: 'Tooltip', + prop: 'open', + text: 'the open prop moved to Tooltip.Trigger in v18 — move it manually. See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Tooltip/TooltipTrigger.tsx#L21', + }, + { + component: 'Select', + prop: 'width', + value: 'fit', + text: 'width="fit" was removed in v18 — choose an explicit width (design decision). See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Select/Select.tsx#L52-L59', + }, + { + component: 'ComboBox', + prop: 'width', + value: 'fit', + text: 'width="fit" was removed in v18 — choose an explicit width (design decision). See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/ComboBox/ComboBox.tsx#L43', + }, + { + component: 'Autocomplete', + prop: 'width', + value: 'fit', + text: 'width="fit" was removed in v18 — choose an explicit width (design decision). See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Autocomplete/Autocomplete.tsx#L124', + }, + { + component: 'Multiselect', + text: 'Multiselect was removed in v18 — use TagField (different API), migrate manually. See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/TagField/TagField.tsx#L43', + }, + { + component: 'SelectList', + prop: 'onChange', + text: 'the onChange signature changed in v18: single mode passes (key: Key | null), multiple mode passes (keys: Key[]) instead of a Selection set — update the handler. See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/SelectList/SelectList.tsx#L137-L145', + }, + { + component: 'Card', + prop: 'pt', + text: 'one-sided Card paddings were removed in v18 — only p/px/py remain; move the padding into the card content (e.g. an Inset or the compound Card slots). See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Card/Card.tsx#L64-L77', + }, + { + component: 'Card', + prop: 'pb', + text: 'one-sided Card paddings were removed in v18 — only p/px/py remain; move the padding into the card content (e.g. an Inset or the compound Card slots). See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Card/Card.tsx#L64-L77', + }, + { + component: 'Card', + prop: 'pl', + text: 'one-sided Card paddings were removed in v18 — only p/px/py remain; move the padding into the card content (e.g. an Inset or the compound Card slots). See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Card/Card.tsx#L64-L77', + }, + { + component: 'Card', + prop: 'pr', + text: 'one-sided Card paddings were removed in v18 — only p/px/py remain; move the padding into the card content (e.g. an Inset or the compound Card slots). See https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/Card/Card.tsx#L64-L77', + }, + ], + }, +}; diff --git a/packages/cli/src/lib/codemod/primitives/report.ts b/packages/cli/src/lib/codemod/primitives/report.ts new file mode 100644 index 0000000000..15d5a731ec --- /dev/null +++ b/packages/cli/src/lib/codemod/primitives/report.ts @@ -0,0 +1,52 @@ +import { findThemeComponents } from '../anchor.js'; +import { parseOr } from '../engine.js'; +import type { Codemod, CodemodOutcome, MigrationManifest } from '../types.js'; + +const analyze = ( + source: string, + collect: (component: string, warnings: string[]) => void +): CodemodOutcome => { + if (!source.includes('ThemeComponent')) { + return { kind: 'unchanged', warnings: [] }; + } + return parseOr(source, file => { + const warnings: string[] = []; + for (const { component } of findThemeComponents(file)) { + collect(component, warnings); + } + return { kind: 'unchanged', warnings }; + }); +}; + +/** + * Theme keys for components the target version no longer knows: they + * silently no-op at runtime and are dead weight in the consumer theme. + */ +export const reportDeadKeys = (manifest: MigrationManifest): Codemod => ({ + name: 'report-dead-keys', + apply: source => + analyze(source, (component, warnings) => { + if (component in manifest.slots) return; + const removed = manifest.removedComponents.includes(component); + warnings.push( + removed + ? `${component}: component was removed in ${manifest.version} — these styles are dead` + : `${component}: not a themeable component in ${manifest.version} — these styles are silently unused` + ); + }), +}); + +/** + * HTML-structure changes are not auto-fixable from here (the consumer may + * target the old DOM with their own CSS, e.g. generated BEM selectors), so + * they surface as structured warnings on the components actually themed. + */ +export const reportStructure = (manifest: MigrationManifest): Codemod => ({ + name: 'report-structure', + apply: source => + analyze(source, (component, warnings) => { + for (const entry of manifest.structureWarnings) { + if (entry.component === component) warnings.push(entry.text); + } + }), +}); diff --git a/packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts b/packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts new file mode 100644 index 0000000000..9031c31e55 --- /dev/null +++ b/packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts @@ -0,0 +1,55 @@ +import { asPropertyKey } from '../anchor.js'; +import { + codeList, + lineIndentAt, + stubSlotLine, + themeCodemod, +} from '../engine.js'; +import type { Codemod, MigrationManifest } from '../types.js'; + +/** + * Wraps a single-style-function theme component into the slot object the + * target version requires: the consumer's existing expression moves verbatim + * into the primary slot, every other slot is stubbed as `cva({})`. + * (v18 example: Card single-cva becomes `{ container: , ... }`.) + */ +export const restructureToSlots = (manifest: MigrationManifest): Codemod => + themeCodemod( + 'restructure-to-slots', + ({ component, init, source, s, unit, changes, warnings }) => { + const slots = manifest.slots[component]; + if (!Array.isArray(slots)) return; + if (init.type === 'ObjectExpression') return; // already slotted + const start = init.start as number; + const end = init.end as number; + + const primary = + manifest.restructures.find(r => r.component === component) + ?.primarySlot ?? 'container'; + if (!slots.includes(primary)) { + warnings.push( + `${component}: manifest primary slot '${primary}' is not a valid slot — restructure skipped` + ); + return; + } + + const base = lineIndentAt(source, start); + const inner = base + unit; + const stubs = slots + .filter(slot => slot !== primary) + .map(slot => stubSlotLine(slot, inner)) + .join('\n'); + const original = source.slice(start, end); + s.overwrite( + start, + end, + `{\n${inner}${asPropertyKey(primary)}: ${original},\n${stubs}\n${base}}` + ); + changes.push( + `${component}: moved existing styles into \`${primary}\`, stubbed ${codeList( + slots.filter(slot => slot !== primary) + )}` + ); + }, + { ensureCva: true } + ); diff --git a/packages/cli/src/lib/codemod/primitives/scaffold-component.ts b/packages/cli/src/lib/codemod/primitives/scaffold-component.ts new file mode 100644 index 0000000000..43a3c5ff34 --- /dev/null +++ b/packages/cli/src/lib/codemod/primitives/scaffold-component.ts @@ -0,0 +1,72 @@ +import { asPropertyKey } from '../anchor.js'; +import { reindent } from '../engine.js'; +import type { + Codemod, + CodemodOutcome, + MigrationManifest, + ScaffoldEntry, +} from '../types.js'; + +/** + * Generates the content of a theme style file for a component the target + * version added (the runner names the file `.styles.ts`). Content is + * fully derived from the manifest slot list: `cva({})` per slot, except + * slots whose layout is load-bearing, which carry the extracted baseline + * classes. Visual styling stays the consumer's job. + */ +export const generateScaffold = ( + entry: ScaffoldEntry, + manifest: MigrationManifest, + indentUnit: string +): string => { + const slots = manifest.slots[entry.name]; + const header = `import { cva, ThemeComponent } from '@marigold/system';\n\n`; + + if (!Array.isArray(slots)) { + return `${header}export const ${entry.name}: ThemeComponent<'${entry.name}'> = cva({});\n`; + } + + const body = slots + .map(slot => { + const loadBearing = entry.loadBearing?.[slot]; + const arg = loadBearing + ? reindent(loadBearing, indentUnit, indentUnit) + : '{}'; + return `${indentUnit}${asPropertyKey(slot)}: cva(${arg}),`; + }) + .join('\n'); + + return `${header}export const ${entry.name}: ThemeComponent<'${entry.name}'> = {\n${body}\n};\n`; +}; + +/** + * Adds `export * from './X.styles';` to a barrel file, idempotently. + * `context` explains the why in the report (e.g. which scaffold this + * registers). + */ +export const addIndexExport = ( + moduleBase: string, + context?: string +): Codemod => ({ + name: 'add-index-export', + apply: (source): CodemodOutcome => { + const exportLine = `export * from './${moduleBase}';`; + if (source.includes(`'./${moduleBase}'`)) { + return { kind: 'unchanged', warnings: [] }; + } + const matches = [...source.matchAll(/^export \* from .*$/gm)]; + const last = matches.at(-1); + const output = last + ? source.slice(0, last.index + last[0].length) + + '\n' + + exportLine + + source.slice(last.index + last[0].length) + : source.trimEnd() + '\n' + exportLine + '\n'; + return { + kind: 'edited', + output, + changes: [`added \`${exportLine}\`${context ? ` — ${context}` : ''}`], + warnings: [], + }; + }, +}); diff --git a/packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts b/packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts new file mode 100644 index 0000000000..b297369d22 --- /dev/null +++ b/packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts @@ -0,0 +1,87 @@ +import { objectProperties, propertyName } from '../anchor.js'; +import { + codeList, + lineIndentAt, + stubSlotLine, + stylesReference, + themeCodemod, +} from '../engine.js'; +import type { Codemod, MigrationManifest } from '../types.js'; + +/** + * Adds slot keys the target version requires but the consumer theme lacks, + * as `cva({})` stubs — never touching existing slots. Slot keys the target + * version dropped are reported as warnings (they fail the exhaustive-Record + * typecheck and are dead weight). + */ +export const stubMissingSlots = (manifest: MigrationManifest): Codemod => + themeCodemod( + 'stub-missing-slots', + ({ component, init, source, s, unit, changes, warnings }) => { + const slots = manifest.slots[component]; + if (!Array.isArray(slots) || init.type !== 'ObjectExpression') return; + + const props = objectProperties(init); + const present = props + .map(propertyName) + .filter((n): n is string => n !== null); + + if (props.some(p => p.type === 'SpreadElement')) { + // Stubbing here would be unsafe: a stub inserted after the spread + // silently overrides any slot the spread already provides (later key + // wins). Name exactly what could not be seen instead. + const unverified = slots.filter(slot => !present.includes(slot)); + if (unverified.length > 0) { + const reference = stylesReference(manifest, component); + warnings.push( + `${component}: a spread hides slot definitions — not visible in this file: ${codeList( + unverified + )}. Verify they exist in the spread source and add missing ones there (never in both places).${ + reference ? ` Reference styles: ${reference}` : '' + }` + ); + } + return; + } + + const missing = slots.filter(slot => !present.includes(slot)); + const dead = present.filter(name => !slots.includes(name)); + for (const name of dead) { + warnings.push( + `${component}: slot \`${name}\` no longer exists in ${manifest.version} — its styles are unused and will fail the typecheck` + ); + } + if (missing.length === 0) return; + + const base = lineIndentAt(source, init.start as number); + const inner = base + unit; + const stubs = missing.map(slot => stubSlotLine(slot, inner)).join('\n'); + + if (props.length === 0) { + s.overwrite( + init.start as number, + init.end as number, + `{\n${stubs}\n${base}}` + ); + } else { + const last = props[props.length - 1]; + const lastEnd = last.end as number; + const tail = source.slice(lastEnd, init.end as number); + const comma = tail.includes(',') ? '' : ','; + s.appendLeft(lastEnd, comma); + const closeBrace = (init.end as number) - 1; + if (tail.includes('\n')) { + // multiline object: insert before the closing brace's line + const lineStart = source.lastIndexOf('\n', closeBrace - 1) + 1; + s.appendLeft(lineStart, `${stubs}\n`); + } else { + // single-line object: break it open before the closing brace + s.appendLeft(closeBrace, `\n${stubs}\n${base}`); + } + } + changes.push( + `${component}: stubbed missing slot(s) ${codeList(missing)}` + ); + }, + { ensureCva: true } + ); diff --git a/packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts b/packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts new file mode 100644 index 0000000000..64b71c927f --- /dev/null +++ b/packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts @@ -0,0 +1,116 @@ +import { type AnyNode } from '../../tsx-ast.js'; +import { + isCvaCall, + marigoldLocalName, + objectProperties, + propertyName, +} from '../anchor.js'; +import { + classStringsIn, + classTokens, + lineIndentAt, + parseExpression, + reindent, + themeCodemod, +} from '../engine.js'; +import type { Codemod, MigrationManifest, SwapEntry } from '../types.js'; + +const sameList = (a: string[], b: string[]): boolean => + a.length === b.length && a.every((v, i) => v === b[i]); + +/** + * Replaces load-bearing baseline styles with the target version's baseline — + * but ONLY when the consumer's class strings equal the old baseline + * byte-for-byte, which proves the slot was never customized. Any deviation + * means customized: warn, never touch. Runs per slot, not per file. + * + * Every applied swap emits a -/+ token diff warning: the v18 strings may + * reference design tokens a standalone theme does not define, so the + * utilities that changed relative to the old baseline are listed for manual + * verification. + */ +export const swapExactClasses = (manifest: MigrationManifest): Codemod => { + // manifest data is constant for the run: resolve each entry's target + // classes and token diff once, not per visited file + const resolved = new Map< + string, + (SwapEntry & { target: string[]; removed: string[]; added: string[] })[] + >(); + for (const entry of manifest.swaps) { + const parsed = parseExpression(entry.newSource); + const target = parsed ? classStringsIn(parsed) : []; + const oldTokens = classTokens(entry.oldClasses); + const newTokens = classTokens(target); + const removed = [...oldTokens].filter(t => !newTokens.has(t)).sort(); + const added = [...newTokens].filter(t => !oldTokens.has(t)).sort(); + const list = resolved.get(entry.component) ?? []; + list.push({ ...entry, target, removed, added }); + resolved.set(entry.component, list); + } + + return themeCodemod( + 'swap-exact-classes', + ({ component, init, file, source, s, unit, changes, warnings }) => { + const entries = resolved.get(component); + if (!entries || init.type !== 'ObjectExpression') return; + const cvaLocal = marigoldLocalName(file, 'cva'); + + for (const entry of entries) { + const prop = objectProperties(init).find( + p => propertyName(p) === entry.slot + ); + // absent slot: stub-missing-slots covers it; the consumer never had + // the baseline, so there is nothing to swap or warn about here. + if (!prop) continue; + + const value = prop.value as AnyNode | undefined; + const arg = (value?.arguments as AnyNode[] | undefined)?.[0]; + + const customized = () => + warnings.push( + `${component}.${entry.slot}: does not match the old Marigold baseline (customized) — migrate manually. ${manifest.version} baseline: ${entry.newSource.replace(/\s+/g, ' ')}` + ); + + if (!value || !isCvaCall(value, cvaLocal) || !arg) { + customized(); + continue; + } + const actual = classStringsIn(arg); + + // already on the target baseline (e.g. a re-run): nothing to do + if (sameList(actual, entry.target)) continue; + + if (!sameList(actual, entry.oldClasses)) { + customized(); + continue; + } + + const base = lineIndentAt(source, prop.start as number); + s.overwrite( + arg.start as number, + arg.end as number, + reindent(entry.newSource, unit, base) + ); + changes.push( + `${component}.${entry.slot}: swapped baseline styles to the ${manifest.version} baseline` + ); + + // Render the change as a token diff (the report colorizes -/+ lines) + // so renamed tokens and new layout utilities are visible at a glance. + // ponytail: added utilities are flagged for manual verification; the + // upgrade path is resolving them against the consumer's actual CSS. + if (entry.added.length > 0 || entry.removed.length > 0) { + warnings.push( + [ + `${component}.${entry.slot}: classes changed vs the old baseline — verify the added utilities resolve in your CSS:`, + ...(entry.removed.length > 0 + ? [`- ${entry.removed.join(' ')}`] + : []), + ...(entry.added.length > 0 ? [`+ ${entry.added.join(' ')}`] : []), + ].join('\n') + ); + } + } + } + ); +}; diff --git a/packages/cli/src/lib/codemod/test-helpers.ts b/packages/cli/src/lib/codemod/test-helpers.ts new file mode 100644 index 0000000000..b08eb5d87a --- /dev/null +++ b/packages/cli/src/lib/codemod/test-helpers.ts @@ -0,0 +1,10 @@ +import type { CodemodOutcome } from './types.js'; + +/** test-only: narrow an outcome to `edited` or fail loudly */ +export function assertEdited( + result: CodemodOutcome +): asserts result is Extract { + if (result.kind !== 'edited') { + throw new Error(`expected edited, got ${result.kind}`); + } +} diff --git a/packages/cli/src/lib/codemod/types.ts b/packages/cli/src/lib/codemod/types.ts new file mode 100644 index 0000000000..6f3ef3f69d --- /dev/null +++ b/packages/cli/src/lib/codemod/types.ts @@ -0,0 +1,133 @@ +export interface Codemod { + name: string; + apply: (source: string) => CodemodOutcome; +} + +export type CodemodOutcome = + | { kind: 'edited'; output: string; changes: string[]; warnings: string[] } + | { kind: 'unchanged'; warnings: string[] } + | { kind: 'skipped'; reason: string }; + +export interface RestructureEntry { + component: string; + /** slot that receives the consumer's existing single-cva styles */ + primarySlot: string; +} + +export interface ScaffoldEntry { + name: string; + /** + * Components whose presence in the consumer theme makes this one required + * at runtime (e.g. Checkbox renders BooleanField internally). + */ + requiredBy: string[]; + /** shown in the report: why this component must exist in the theme */ + reason?: string; + /** + * Per-slot cva() argument source for slots whose classes are load-bearing + * layout (2-space indented). Slots not listed are stubbed as `cva({})`. + */ + loadBearing?: Record; +} + +export interface SwapEntry { + component: string; + slot: string; + /** + * Class-string literals of the old-baseline cva() argument, in source + * order. The swap fires only when the consumer's literals equal these + * byte-for-byte — proof the slot was never customized. + */ + oldClasses: string[]; + /** Replacement source for the whole cva() argument (2-space indented). */ + newSource: string; +} + +export interface StructureWarning { + component: string; + text: string; +} + +export interface JsxRenameEntry { + component: string; + from: string; + to: string; + /** additionally wrap the prop value in an array (e.g. acceptedFileTypes) */ + wrapInArray?: boolean; +} + +export interface JsxMemberRenameEntry { + /** the compound root, e.g. 'Tabs' for Tabs.TabPanel */ + object: string; + from: string; + to: string; +} + +export interface JsxRemovalEntry { + component: string; + prop: string; + /** appended to the change message, e.g. what replaces the prop */ + note?: string; +} + +export interface JsxImportRenameEntry { + /** package the export moved in, e.g. '@marigold/icons' */ + package: string; + from: string; + to: string; + /** appended to the change message, e.g. provenance of the mapping */ + note?: string; +} + +export interface JsxUsageWarning { + component: string; + /** without `prop`, warns when the component is imported at all */ + prop?: string; + /** with `value`, warns only when the prop has this literal value (or one + * that cannot be verified statically) */ + value?: string; + text: string; +} + +/** + * Safe application-code transforms: lexically decidable prop renames and + * removals on components verifiably imported from @marigold/components. + * Anything requiring structural JSX moves or a design decision belongs in + * `warnings`, never here. + */ +export interface JsxChanges { + renames: JsxRenameEntry[]; + memberRenames: JsxMemberRenameEntry[]; + removals: JsxRemovalEntry[]; + /** + * Renamed exports (e.g. the v18 icon migration). Applied via the + * release-notes-blessed alias strategy: `import { Store as Pickup }` keeps + * every call site untouched and cannot collide or shadow. + */ + importRenames: JsxImportRenameEntry[]; + warnings: JsxUsageWarning[]; +} + +export interface MigrationManifest { + schemaVersion: 1; + version: string; + /** + * Slot sets of the target version, keyed by component. `null` marks a + * single style function (no slots). Source of truth: the `Theme` type in + * @marigold/system — codegen will derive this; the v18 file is hand-written. + */ + slots: Record; + restructures: RestructureEntry[]; + scaffolds: ScaffoldEntry[]; + /** Formerly valid component keys that no longer exist. */ + removedComponents: string[]; + swaps: SwapEntry[]; + structureWarnings: StructureWarning[]; + jsx: JsxChanges; + /** + * Pinned base URL of the default theme's component style sources + * (`/.styles.ts`), used in reports as the reference for + * what a slot's styles look like in the target version. + */ + stylesReferenceUrl?: string; +} diff --git a/packages/cli/src/lib/doctor/format.ts b/packages/cli/src/lib/doctor/format.ts index 5b757c5174..8e1399f8f6 100644 --- a/packages/cli/src/lib/doctor/format.ts +++ b/packages/cli/src/lib/doctor/format.ts @@ -1,4 +1,5 @@ import pc from 'picocolors'; +import { highlightCode } from '../format.js'; export interface RenderRow { title: string; @@ -21,13 +22,6 @@ const glyph = (status: RenderRow['status']): string => { return pc.yellow('!'); }; -// Colorize `inline code` spans while keeping the surrounding backticks, so -// piped / non-TTY output (tests, AI agents, files) stays byte-for-byte the same -// and only interactive terminals gain color. picocolors is TTY-aware, so every -// helper below degrades to plain text automatically when color is unsupported. -const highlightCode = (text: string): string => - text.replace(/`[^`]+`/g, match => pc.cyan(match)); - // A single check can carry several findings (e.g. the Tailwind, freshness, and // React-peer checks). For humans we render them as a bulleted list under an // optional "headline:" lead-in, working from the structured `findings` array diff --git a/packages/cli/src/lib/edit-tsx.ts b/packages/cli/src/lib/edit-tsx.ts index 08c4210d5f..55b931dede 100644 --- a/packages/cli/src/lib/edit-tsx.ts +++ b/packages/cli/src/lib/edit-tsx.ts @@ -15,7 +15,7 @@ export type TsxEditOutcome = | { kind: 'unchanged'; reason: string } | { kind: 'skipped'; reason: string }; -const insertImport = ( +export const insertImport = ( s: MagicString, file: AnyNode, imports: AnyNode[], diff --git a/packages/cli/src/lib/format.ts b/packages/cli/src/lib/format.ts index 8635386a02..a9cc06380c 100644 --- a/packages/cli/src/lib/format.ts +++ b/packages/cli/src/lib/format.ts @@ -13,6 +13,13 @@ import { stripAnsi } from './strip-ansi.js'; export type OutputFormat = 'markdown' | 'json' | 'plain'; +// Colorize `inline code` spans while keeping the surrounding backticks, so +// piped / non-TTY output (tests, AI agents, files) stays byte-for-byte the +// same and only interactive terminals gain color. Shared by the doctor and +// migrate reports. +export const highlightCode = (text: string): string => + text.replace(/`[^`]+`/g, match => pc.cyan(match)); + const renderMarkdownToTerminal = (md: string): string => { const lines = md.split('\n'); const out: string[] = []; diff --git a/packages/cli/src/lib/tsx-ast.test.ts b/packages/cli/src/lib/tsx-ast.test.ts index cff11b85f9..fe27121b07 100644 --- a/packages/cli/src/lib/tsx-ast.test.ts +++ b/packages/cli/src/lib/tsx-ast.test.ts @@ -152,7 +152,9 @@ describe('findRenderArgument', () => { describe('walk', () => { test('ignores non-object nodes without throwing', () => { const seen: string[] = []; - walk(ast(`const a = 1;`), n => seen.push(n.type)); + walk(ast(`const a = 1;`), n => { + seen.push(n.type); + }); expect(seen).toContain('VariableDeclaration'); // primitives (numbers, strings) inside the tree are skipped, not visited expect(seen).not.toContain('1'); diff --git a/packages/cli/src/lib/tsx-ast.ts b/packages/cli/src/lib/tsx-ast.ts index 556d81ef7e..e3c5814fea 100644 --- a/packages/cli/src/lib/tsx-ast.ts +++ b/packages/cli/src/lib/tsx-ast.ts @@ -32,12 +32,13 @@ const SKIP_KEYS = new Set([ export const walk = ( node: unknown, - visitor: (n: AnyNode, parent: AnyNode | null) => void, + visitor: (n: AnyNode, parent: AnyNode | null) => void | boolean, parent: AnyNode | null = null ): void => { if (!node || typeof node !== 'object') return; const n = node as AnyNode; - if (typeof n.type === 'string') visitor(n, parent); + // a visitor may return false to skip the node's children + if (typeof n.type === 'string' && visitor(n, parent) === false) return; for (const key of Object.keys(n)) { if (SKIP_KEYS.has(key)) continue; const value = n[key]; From 8a2ee9ecb896a6c75a85ab7f79748b38c04e5d4a Mon Sep 17 00:00:00 2001 From: aromko Date: Thu, 23 Jul 2026 16:50:15 +0200 Subject: [PATCH 02/11] feat(DST-1543): add safe application-code codemods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSX transforms anchored on imports from @marigold/components — a local component that happens to share a Marigold name is never touched: - rename-jsx-props: Inset space/spaceX/spaceY to p/px/py, acceptedFileType to acceptedFileTypes (with array wrap) - rename-jsx-members: Tabs.TabPanel to Tabs.Panel, SelectList.Item to SelectList.Option (opening and closing tags) - remove-jsx-props: TextField min/max (dropped in v18) - rename-imports: the v18 icon migration, driven by the official mapping table in .changeset/iconography-docs.md. Renames import + usages directly when the file provably allows it; falls back to the release-notes-blessed alias form (Store as Pickup) on shadowing, shorthand properties, or name collisions, and names the reason - report-jsx-usage: warnings for changes needing a human decision (Tooltip open, width="fit", Multiselect, Switch size="large", Card one-sided paddings, SelectList onChange signature) Only lexically decidable changes are edits; everything requiring a structural JSX move or a design decision stays a warning. --- packages/cli/src/lib/codemod/jsx.test.ts | 285 ++++++++++++ .../cli/src/lib/codemod/primitives/jsx.ts | 431 ++++++++++++++++++ 2 files changed, 716 insertions(+) create mode 100644 packages/cli/src/lib/codemod/jsx.test.ts create mode 100644 packages/cli/src/lib/codemod/primitives/jsx.ts diff --git a/packages/cli/src/lib/codemod/jsx.test.ts b/packages/cli/src/lib/codemod/jsx.test.ts new file mode 100644 index 0000000000..e76684eb0b --- /dev/null +++ b/packages/cli/src/lib/codemod/jsx.test.ts @@ -0,0 +1,285 @@ +import { v18 } from './manifests/v18.js'; +import { + removeJsxProps, + renameImports, + renameJsxMembers, + renameJsxProps, + reportJsxUsage, +} from './primitives/jsx.js'; +import { assertEdited } from './test-helpers.js'; +import type { CodemodOutcome } from './types.js'; + +const warningsOf = (result: CodemodOutcome): string[] => + result.kind === 'skipped' ? [] : result.warnings; + +describe('rename-jsx-props', () => { + test('renames Inset spacing props', () => { + const source = `import { Inset } from '@marigold/components'; +export const App = () => ( + + content + +); +`; + const result = renameJsxProps(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain(''); + expect(result.output).toContain(''); + expect(result.changes).toHaveLength(3); + }); + + test('anchors on the import: other packages and local components stay untouched', () => { + const source = `import { Inset } from './my-components'; +export const App = () => ; +`; + expect(renameJsxProps(v18).apply(source).kind).toBe('unchanged'); + }); + + test('follows aliased imports', () => { + const source = `import { Inset as Spacing } from '@marigold/components'; +export const App = () => ; +`; + const result = renameJsxProps(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain(''); + }); + + test('wraps acceptedFileType values in an array', () => { + const source = `import { FileField } from '@marigold/components'; +export const A = () => ; +export const B = ({ types }: { types: string }) => ( + +); +export const C = () => ; +`; + const result = renameJsxProps(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain(`acceptedFileTypes={['image/png']} />`); + expect(result.output).toContain('acceptedFileTypes={[types]}'); + // already an array: renamed but not double-wrapped + expect(result.output).not.toContain(`{[['image/png']]}`); + }); +}); + +describe('rename-jsx-members', () => { + test('renames Tabs.TabPanel to Tabs.Panel on opening and closing tags', () => { + const source = `import { Tabs } from '@marigold/components'; +export const App = () => ( + + content + +); +`; + const result = renameJsxMembers(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + 'content' + ); + expect(result.output).not.toContain('TabPanel'); + }); + + test('renames SelectList.Item to SelectList.Option', () => { + const source = `import { SelectList } from '@marigold/components'; +export const App = () => ( + + A + +); +`; + const result = renameJsxMembers(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + 'A' + ); + }); + + test('leaves foreign Tabs alone', () => { + const source = `import { Tabs } from 'other-lib'; +export const App = () => content; +`; + expect(renameJsxMembers(v18).apply(source).kind).toBe('unchanged'); + }); +}); + +describe('remove-jsx-props', () => { + test('removes dropped TextField props and keeps the rest intact', () => { + const source = `import { TextField } from '@marigold/components'; +export const App = () => ( + +); +`; + const result = removeJsxProps(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain(''); + expect(result.changes[0]).toContain('use NumberField'); + }); +}); + +describe('rename-imports', () => { + test('renames the import and every usage directly when safe', () => { + const source = `import { Pickup, Clock } from '@marigold/icons'; +const icon = Pickup; +export const App = () => ; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + `import { Store, Clock } from '@marigold/icons';` + ); + expect(result.output).toContain('const icon = Store;'); + expect(result.output).toContain(''); + expect(result.changes[0]).toContain('import + 2 usages'); + }); + + test('does not touch member accesses or object keys with the old name', () => { + const source = `import { Pickup } from '@marigold/icons'; +const config = { Pickup: 1, render: () => }; +export const x = config.Pickup; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain('{ Pickup: 1,'); + expect(result.output).toContain('config.Pickup'); + expect(result.output).toContain(''); + }); + + test('falls back to an alias when the new name already exists in the file', () => { + const source = `import { Pickup } from '@marigold/icons'; +import { Store } from './my-store'; +export const App = () => ; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + `import { Store as Pickup } from '@marigold/icons';` + ); + expect(result.output).toContain(''); + expect(result.changes[0]).toContain('already used in this file'); + }); + + test('falls back to an alias when the old name is shadowed or shorthand', () => { + const source = `import { Pickup } from '@marigold/icons'; +export const options = { Pickup }; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + `import { Store as Pickup } from '@marigold/icons';` + ); + expect(result.output).toContain('{ Pickup }'); + }); + + test('only swaps the imported name when already aliased', () => { + const source = `import { Email as MailIcon } from '@marigold/icons'; +export const App = () => ; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + `import { Mail as MailIcon } from '@marigold/icons';` + ); + expect(result.output).toContain(''); + }); + + test('follows the official mapping, not lookalike names', () => { + // the TS suggestion for CircleChecked is CircleCheck — the official + // mapping says CircleDot (it was a radio indicator) + const source = `import { CircleChecked } from '@marigold/icons'; +export const App = () => ; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + `import { CircleDot } from '@marigold/icons';` + ); + expect(result.output).toContain(''); + }); + + test('leaves kept exports and other packages alone', () => { + const source = `import { Clock, Search, Stadium } from '@marigold/icons'; +import { Email } from 'other-icons'; +export const App = () => ; +`; + expect(renameImports(v18).apply(source).kind).toBe('unchanged'); + }); + + test('is idempotent: renamed and aliased output is not rewritten again', () => { + const direct = `import { Store } from '@marigold/icons'; +export const App = () => ; +`; + const aliased = `import { Store as Pickup } from '@marigold/icons'; +export const App = () => ; +`; + expect(renameImports(v18).apply(direct).kind).toBe('unchanged'); + expect(renameImports(v18).apply(aliased).kind).toBe('unchanged'); + }); +}); + +describe('report-jsx-usage', () => { + test('warns on removed components when imported', () => { + const source = `import { Multiselect } from '@marigold/components'; +export const App = () => ; +`; + const warnings = warningsOf(reportJsxUsage(v18).apply(source)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('TagField'); + }); + + test('warns on value-conditional props only for the named value', () => { + const source = `import { Select } from '@marigold/components'; +export const A = () => ; +`; + const warnings = warningsOf(reportJsxUsage(v18).apply(source)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('width="fit"'); + }); + + test('warns when a conditional value cannot be verified statically', () => { + const source = `import { Switch } from '@marigold/components'; +export const App = ({ size }: { size: string }) => ; +`; + const warnings = warningsOf(reportJsxUsage(v18).apply(source)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('variant="settings"'); + }); + + test('warns on removed one-sided Card paddings', () => { + const source = `import { Card } from '@marigold/components'; +export const App = () => content; +`; + const warnings = warningsOf(reportJsxUsage(v18).apply(source)); + expect(warnings).toHaveLength(2); + expect(warnings[0]).toContain('only p/px/py remain'); + }); + + test('warns on the changed SelectList onChange signature', () => { + const source = `import { SelectList } from '@marigold/components'; +export const App = () => {}} />; +`; + const warnings = warningsOf(reportJsxUsage(v18).apply(source)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('Selection set'); + }); + + test('warns on Tooltip open regardless of value', () => { + const source = `import { Tooltip } from '@marigold/components'; +export const App = () => hint; +`; + const warnings = warningsOf(reportJsxUsage(v18).apply(source)); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('Tooltip.Trigger'); + }); +}); diff --git a/packages/cli/src/lib/codemod/primitives/jsx.ts b/packages/cli/src/lib/codemod/primitives/jsx.ts new file mode 100644 index 0000000000..6062df9877 --- /dev/null +++ b/packages/cli/src/lib/codemod/primitives/jsx.ts @@ -0,0 +1,431 @@ +import MagicString from 'magic-string'; +import { + type AnyNode, + collectImports, + jsxOpeningName, + walk, +} from '../../tsx-ast.js'; +import { MARIGOLD_COMPONENTS } from '../anchor.js'; +import { parseOr } from '../engine.js'; +import type { Codemod, CodemodOutcome, MigrationManifest } from '../types.js'; + +// Safe application-code transforms. The anchor is the import: a JSX element +// only counts as a Marigold component when its (possibly aliased) name is +// verifiably imported from @marigold/components. Only explicit JSX attributes +// are touched; props hidden behind spreads are out of reach by design, and +// the consumer's typecheck catches whatever we cannot see. + +const jsxElements = (file: AnyNode): AnyNode[] => { + const out: AnyNode[] = []; + walk(file, n => { + if (n.type === 'JSXElement') out.push(n); + }); + return out; +}; + +const jsxAttributes = (opening: AnyNode | undefined): AnyNode[] => + ((opening?.attributes as AnyNode[] | undefined) ?? []).filter( + a => a.type === 'JSXAttribute' + ); + +const attrName = (attr: AnyNode): string | null => + (attr.name as AnyNode | undefined as { name?: string } | undefined)?.name ?? + null; + +/** local names for the manifest's component names, only when imported */ +const localsFor = (file: AnyNode, components: Iterable) => { + const wanted = new Set(components); + const locals = new Map(); // local -> canonical name + for (const imp of collectImports(file)) { + const src = (imp.source as { value?: string } | undefined)?.value; + if (src !== MARIGOLD_COMPONENTS) continue; + for (const spec of (imp.specifiers as AnyNode[] | undefined) ?? []) { + if (spec.type !== 'ImportSpecifier') continue; + const imported = (spec.imported as { name?: string } | undefined)?.name; + const local = (spec.local as { name?: string } | undefined)?.name; + if (imported && local && wanted.has(imported)) { + locals.set(local, imported); + } + } + } + return locals; +}; + +/** Renames props on Marigold components, e.g. Inset `space` to `p`. */ +export const renameJsxProps = (manifest: MigrationManifest): Codemod => ({ + name: 'rename-jsx-props', + apply: source => + parseOr(source, file => { + const locals = localsFor( + file, + manifest.jsx.renames.map(e => e.component) + ); + if (locals.size === 0) return { kind: 'unchanged', warnings: [] }; + + const s = new MagicString(source); + const changes: string[] = []; + for (const el of jsxElements(file)) { + const component = locals.get(jsxOpeningName(el) ?? ''); + if (!component) continue; + const opening = el.openingElement as AnyNode | undefined; + for (const entry of manifest.jsx.renames) { + if (entry.component !== component) continue; + for (const attr of jsxAttributes(opening)) { + if (attrName(attr) !== entry.from) continue; + const nameNode = attr.name as AnyNode; + s.overwrite( + nameNode.start as number, + nameNode.end as number, + entry.to + ); + if (entry.wrapInArray) wrapValueInArray(s, source, attr); + changes.push( + `${component}: renamed \`${entry.from}\` to \`${entry.to}\`` + ); + } + } + } + if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; + return { kind: 'edited', output: s.toString(), changes, warnings: [] }; + }), +}); + +const wrapValueInArray = ( + s: MagicString, + source: string, + attr: AnyNode +): void => { + const value = attr.value as AnyNode | undefined; + if (!value) return; // boolean attribute, nothing to wrap + if (value.type === 'StringLiteral') { + const text = source.slice(value.start as number, value.end as number); + s.overwrite(value.start as number, value.end as number, `{[${text}]}`); + return; + } + if (value.type === 'JSXExpressionContainer') { + const expr = value.expression as AnyNode | undefined; + if (!expr || expr.type === 'ArrayExpression') return; // already an array + s.appendLeft(expr.start as number, '['); + s.appendRight(expr.end as number, ']'); + } +}; + +// Identifier positions that are names/keys, not references to a binding. +// Renaming these would change object shapes, member accesses, or attribute +// names instead of the import usage. +const isNamePosition = (n: AnyNode, parent: AnyNode | null): boolean => { + if (!parent) return false; + const computed = (parent as { computed?: boolean }).computed === true; + switch (parent.type) { + case 'MemberExpression': + case 'OptionalMemberExpression': + return parent.property === n && !computed; + case 'ObjectProperty': + case 'ObjectMethod': + case 'ClassProperty': + case 'ClassMethod': + case 'TSPropertySignature': + case 'TSMethodSignature': + return parent.key === n && !computed; + case 'TSQualifiedName': + return parent.right === n; + case 'JSXAttribute': + return parent.name === n; + case 'ImportSpecifier': + case 'ExportSpecifier': + return true; + case 'LabeledStatement': + case 'BreakStatement': + case 'ContinueStatement': + return true; + default: + return false; + } +}; + +// Positions that (re)declare a binding with this name — a shadow of the +// import, which makes a whole-file usage rename unsafe. +const isBindingPosition = (n: AnyNode, parent: AnyNode | null): boolean => { + if (!parent) return false; + switch (parent.type) { + case 'VariableDeclarator': + return parent.id === n; + case 'FunctionDeclaration': + case 'FunctionExpression': + case 'ClassDeclaration': + case 'ClassExpression': + return parent.id === n; + case 'ArrowFunctionExpression': + return ((parent.params as AnyNode[]) ?? []).includes(n); + case 'CatchClause': + return parent.param === n; + // destructuring targets and function params inside patterns + case 'ObjectPattern': + case 'ArrayPattern': + case 'RestElement': + case 'AssignmentPattern': + return true; + default: + if ( + (parent.type === 'FunctionDeclaration' || + parent.type === 'FunctionExpression') && + ((parent.params as AnyNode[]) ?? []).includes(n) + ) { + return true; + } + return false; + } +}; + +/** + * Rewrites imports whose exported name changed (e.g. the v18 icon + * migration). When the file provably allows it, the import AND every usage + * are renamed directly (`` becomes ``). When it does not + * — the old name is shadowed or used in a shorthand property, or the new + * name already exists in the file — the specifier falls back to the + * release-notes-blessed alias form (`Store as Pickup`), which keeps every + * call site valid, and the report names the reason. Already aliased + * specifiers only swap the imported name. + */ +export const renameImports = (manifest: MigrationManifest): Codemod => { + // manifest data is constant for the run: index it once + const byPackage = new Map< + string, + Map + >(); + for (const entry of manifest.jsx.importRenames) { + const forPackage = byPackage.get(entry.package) ?? new Map(); + forPackage.set(entry.from, entry); + byPackage.set(entry.package, forPackage); + } + + return { + name: 'rename-imports', + apply: source => + parseOr(source, file => { + const s = new MagicString(source); + const changes: string[] = []; + + // one walk collects everything needed to decide direct vs alias + const usages = new Map(); // renameable references + const shadowed = new Set(); + const namesInFile = new Set(); + walk(file, (n, parent) => { + if (n.type !== 'Identifier' && n.type !== 'JSXIdentifier') return; + const name = (n as { name?: string }).name; + if (!name) return; + namesInFile.add(name); + if (n.type === 'Identifier' && isBindingPosition(n, parent)) { + shadowed.add(name); + return; + } + if ( + parent?.type === 'ObjectProperty' && + (parent as { shorthand?: boolean }).shorthand === true + ) { + // `{ Pickup }` — renaming the value would change the key + shadowed.add(name); + return; + } + if (isNamePosition(n, parent)) return; + const list = usages.get(name) ?? []; + list.push(n); + usages.set(name, list); + }); + + for (const imp of collectImports(file)) { + const src = (imp.source as { value?: string } | undefined)?.value; + const renames = src ? byPackage.get(src) : undefined; + if (!renames) continue; + for (const spec of (imp.specifiers as AnyNode[] | undefined) ?? []) { + if (spec.type !== 'ImportSpecifier') continue; + const imported = spec.imported as AnyNode; + const importedName = (imported as { name?: string }).name; + const localName = ( + spec.local as AnyNode as { name?: string } | undefined + )?.name; + const entry = importedName ? renames.get(importedName) : undefined; + if (!entry) continue; + const note = entry.note ? ` (${entry.note})` : ''; + + if (importedName !== localName) { + // already aliased: only the imported name changes + s.overwrite( + imported.start as number, + imported.end as number, + entry.to + ); + changes.push( + `${src}: \`${entry.from}\` is now \`${entry.to}\` (existing alias \`${localName}\` kept)${note}` + ); + continue; + } + + const aliasReason = namesInFile.has(entry.to) + ? `\`${entry.to}\` is already used in this file` + : shadowed.has(entry.from) + ? `\`${entry.from}\` is re-declared or used as a shorthand property here` + : null; + + if (aliasReason) { + s.overwrite( + spec.start as number, + spec.end as number, + `${entry.to} as ${entry.from}` + ); + changes.push( + `${src}: \`${entry.from}\` is now \`${entry.to}\` — imported as \`${entry.to} as ${entry.from}\` because ${aliasReason}${note}` + ); + continue; + } + + // safe to rename directly: the import specifier + every usage + // (specifier identifiers sit in name position and are not part + // of `usages`, so the specifier is rewritten explicitly) + s.overwrite(spec.start as number, spec.end as number, entry.to); + const refs = usages.get(entry.from) ?? []; + for (const ref of refs) { + s.overwrite(ref.start as number, ref.end as number, entry.to); + } + changes.push( + `${src}: renamed \`${entry.from}\` to \`${entry.to}\` (import + ${refs.length} usage${refs.length === 1 ? '' : 's'})${note}` + ); + } + } + if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; + return { kind: 'edited', output: s.toString(), changes, warnings: [] }; + }), + }; +}; + +/** Renames compound members, e.g. Tabs.TabPanel to Tabs.Panel. */ +export const renameJsxMembers = (manifest: MigrationManifest): Codemod => ({ + name: 'rename-jsx-members', + apply: source => + parseOr(source, file => { + const locals = localsFor( + file, + manifest.jsx.memberRenames.map(e => e.object) + ); + if (locals.size === 0) return { kind: 'unchanged', warnings: [] }; + + const s = new MagicString(source); + const changes: string[] = []; + const renameTag = (tag: AnyNode | undefined, to: string): void => { + const property = (tag?.name as AnyNode | undefined)?.property as + | AnyNode + | undefined; + if (property) { + s.overwrite(property.start as number, property.end as number, to); + } + }; + for (const el of jsxElements(file)) { + const opening = el.openingElement as AnyNode | undefined; + const name = opening?.name as AnyNode | undefined; + if (name?.type !== 'JSXMemberExpression') continue; + const object = name.object as AnyNode | undefined; + const property = name.property as AnyNode | undefined; + if (object?.type !== 'JSXIdentifier') continue; + const component = locals.get((object as { name?: string }).name ?? ''); + if (!component) continue; + for (const entry of manifest.jsx.memberRenames) { + if ( + entry.object !== component || + (property as { name?: string })?.name !== entry.from + ) { + continue; + } + renameTag(opening, entry.to); + renameTag(el.closingElement as AnyNode | undefined, entry.to); + changes.push( + `\`${component}.${entry.from}\`: renamed to \`${component}.${entry.to}\`` + ); + } + } + if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; + return { kind: 'edited', output: s.toString(), changes, warnings: [] }; + }), +}); + +/** Removes props the target version dropped, e.g. TextField min/max. */ +export const removeJsxProps = (manifest: MigrationManifest): Codemod => ({ + name: 'remove-jsx-props', + apply: source => + parseOr(source, file => { + const locals = localsFor( + file, + manifest.jsx.removals.map(e => e.component) + ); + if (locals.size === 0) return { kind: 'unchanged', warnings: [] }; + + const s = new MagicString(source); + const changes: string[] = []; + for (const el of jsxElements(file)) { + const component = locals.get(jsxOpeningName(el) ?? ''); + if (!component) continue; + const opening = el.openingElement as AnyNode | undefined; + for (const entry of manifest.jsx.removals) { + if (entry.component !== component) continue; + for (const attr of jsxAttributes(opening)) { + if (attrName(attr) !== entry.prop) continue; + let start = attr.start as number; + while (start > 0 && /\s/.test(source[start - 1])) start--; + s.remove(start, attr.end as number); + changes.push( + `${component}: removed \`${entry.prop}\`${entry.note ? ` (${entry.note})` : ''}` + ); + } + } + } + if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; + return { kind: 'edited', output: s.toString(), changes, warnings: [] }; + }), +}); + +/** + * Report-only: usages that need a human decision (removed components, + * props that moved structurally, design decisions). Value-conditional + * entries also warn when the value cannot be verified statically. + */ +export const reportJsxUsage = (manifest: MigrationManifest): Codemod => ({ + name: 'report-jsx-usage', + apply: (source): CodemodOutcome => + parseOr(source, file => { + const warnings: string[] = []; + const locals = localsFor( + file, + manifest.jsx.warnings.map(e => e.component) + ); + if (locals.size === 0) return { kind: 'unchanged', warnings }; + + const imported = new Set(locals.values()); + for (const entry of manifest.jsx.warnings) { + if (!entry.prop && imported.has(entry.component)) { + warnings.push(`${entry.component}: ${entry.text}`); + } + } + + for (const el of jsxElements(file)) { + const component = locals.get(jsxOpeningName(el) ?? ''); + if (!component) continue; + const opening = el.openingElement as AnyNode | undefined; + for (const entry of manifest.jsx.warnings) { + if (entry.component !== component || !entry.prop) continue; + for (const attr of jsxAttributes(opening)) { + if (attrName(attr) !== entry.prop) continue; + const value = attr.value as AnyNode | undefined; + if (entry.value !== undefined) { + const literal = + value?.type === 'StringLiteral' + ? ((value as { value?: string }).value ?? null) + : null; + // a non-literal value cannot be ruled out statically: warn too + if (literal !== null && literal !== entry.value) continue; + } + warnings.push(`${component}[${entry.prop}]: ${entry.text}`); + } + } + } + return { kind: 'unchanged', warnings }; + }), +}); From bc14603240674b60ee48ad644df96911ad66d2aa Mon Sep 17 00:00:00 2001 From: aromko Date: Thu, 23 Jul 2026 16:50:36 +0200 Subject: [PATCH 03/11] feat(DST-1543): add the marigold migrate command `marigold migrate [path] [--dry-run]` runs the codemod pipeline over a consumer repo: one read per file, a theme-component inventory pass, the ordered per-file pipeline (restructure, swap, stub, JSX transforms, reports), and a scaffold pass that creates missing theme components next to the files that require them and registers them in the local barrel. The report is TTY-aware (plain when piped): green ~ changes, yellow ! warnings with cyan code spans and clickable pinned source links, red/ green -/+ token diffs for baseline swaps, and a closing reminder that the consumer's typecheck is the completeness check. Lazy-loaded in the bin like init/doctor so @babel/parser and magic-string stay off the docs/list hot path. --- .changeset/migrate-codemods.md | 5 + packages/cli/src/bin/marigold.ts | 38 +++ packages/cli/src/commands/migrate.test.ts | 185 +++++++++++++ packages/cli/src/commands/migrate.ts | 299 ++++++++++++++++++++++ packages/cli/src/lib/commands-spec.ts | 5 + packages/cli/src/lib/telemetry.ts | 1 + 6 files changed, 533 insertions(+) create mode 100644 .changeset/migrate-codemods.md create mode 100644 packages/cli/src/commands/migrate.test.ts create mode 100644 packages/cli/src/commands/migrate.ts diff --git a/.changeset/migrate-codemods.md b/.changeset/migrate-codemods.md new file mode 100644 index 0000000000..9ea2c087d9 --- /dev/null +++ b/.changeset/migrate-codemods.md @@ -0,0 +1,5 @@ +--- +'@marigold/cli': minor +--- + +feat(DST-1543): add `marigold migrate ` codemods for breaking Marigold releases. The v18 migration restructures theme files to the new slot shapes (never overriding consumer classes), swaps exact-baseline layout classes with a token diff report, scaffolds missing theme components, applies safe application-code renames (icon imports per the official mapping, `Tabs.TabPanel`/`SelectList.Item`, `Inset` spacing props, `TextField` min/max), and reports everything that needs a human decision with pinned source links. Run `npx marigold migrate v18 --dry-run` first. diff --git a/packages/cli/src/bin/marigold.ts b/packages/cli/src/bin/marigold.ts index 0541bf2d71..cfcf782bda 100644 --- a/packages/cli/src/bin/marigold.ts +++ b/packages/cli/src/bin/marigold.ts @@ -68,6 +68,7 @@ ${pc.bold('Commands:')} examples Browse application patterns (list | get ) init Set up Marigold in a project doctor Diagnose a project's Marigold setup + migrate Apply codemods for a breaking Marigold release telemetry Manage telemetry (status|enable|disable) completion Print shell completion script (bash|zsh|fish) @@ -102,6 +103,10 @@ ${pc.bold('Doctor options:')} --format text | json (default: text) --offline Skip the network; use only the local cache +${pc.bold('Migrate options:')} + [path] Directory to migrate (default: current directory) + --dry-run Report what would change without writing files + ${pc.bold('Environment:')} MARIGOLD_DOCS_URL Override docs site base URL MARIGOLD_CACHE_TTL_MS Override cache TTL in milliseconds @@ -202,6 +207,15 @@ const parseDoctorCommand = (argv: string[]) => }, }); +const parseMigrateCommand = (argv: string[]) => + parseArgs({ + args: argv, + allowPositionals: true, + options: { + 'dry-run': { type: 'boolean', default: false }, + }, + }); + const isExamplesSub = (v: string): v is ExamplesSubcommand => (EXAMPLES_SUBCOMMANDS as readonly string[]).includes(v); @@ -419,6 +433,30 @@ export const main = async ( writeOutput(result.output); if (result.hasErrors) exitCode = 1; + } else if (command === 'migrate') { + const { positionals, values } = parseMigrateCommand(rest); + const [version, targetPath] = positionals; + + telemetryArgs = { + version: version ?? '', + ...(values['dry-run'] ? { dryRun: 'true' } : {}), + }; + + if (!version || positionals.length > 2) { + fail('Usage: marigold migrate [path] [--dry-run]'); + } + + // Lazy-load: migrate pulls in @babel/parser and magic-string, which we + // keep off the docs/list hot path. + const { runMigrate } = await import('../commands/migrate.js'); + const result = await runMigrate({ + // accept both `18` and `v18` + version: version.startsWith('v') ? version : `v${version}`, + targetPath: targetPath ?? process.cwd(), + dryRun: values['dry-run'], + }); + + writeOutput(result.output); } else if (command === 'telemetry') { const [sub] = rest; telemetryArgs = sub ? { sub } : {}; diff --git a/packages/cli/src/commands/migrate.test.ts b/packages/cli/src/commands/migrate.test.ts new file mode 100644 index 0000000000..43d260322e --- /dev/null +++ b/packages/cli/src/commands/migrate.test.ts @@ -0,0 +1,185 @@ +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; +import { existsSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runMigrate } from './migrate.js'; + +// End-to-end run against a miniature portal-shaped theme tree: standalone +// theme, 4-space indent, one style file per component, barrel index. + +const CARD = `import { cva, ThemeComponent } from '@marigold/system'; + +export const Card: ThemeComponent<'Card'> = cva({ + base: ['bg-white rounded-xs'], + variants: { + variant: { + default: 'p-2' + } + } +}); +`; + +const SWITCH = `import { cva, ThemeComponent } from '@marigold/system'; + +export const Switch: ThemeComponent<'Switch'> = { + container: cva({ + base: 'disabled:cursor-not-allowed disabled:text-disabled-foreground' + }), + track: cva({ base: 'flex h-6 w-10' }), + thumb: cva({ base: 'block size-5' }) +}; +`; + +const INDEX = `export * from './Card.styles'; +export * from './Switch.styles'; +`; + +const setupFixture = (): string => { + const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-migrate-')); + const components = path.join(root, 'theme', 'components'); + mkdirSync(components, { recursive: true }); + writeFileSync(path.join(components, 'Card.styles.ts'), CARD); + writeFileSync(path.join(components, 'Switch.styles.ts'), SWITCH); + writeFileSync(path.join(components, 'index.ts'), INDEX); + // decoy that must be ignored: no @marigold/system import + writeFileSync( + path.join(root, 'theme', 'unrelated.ts'), + `export const x = 1;\n` + ); + return root; +}; + +describe('runMigrate', () => { + test('rejects unknown migration versions', () => { + expect(() => + runMigrate({ version: 'v99', targetPath: '.', dryRun: true }) + ).toThrow(/Unknown migration 'v99'/); + }); + + test('dry run reports changes without writing anything', async () => { + const root = setupFixture(); + const cardPath = path.join(root, 'theme', 'components', 'Card.styles.ts'); + const before = readFileSync(cardPath, 'utf8'); + + const { output } = await runMigrate({ + version: 'v18', + targetPath: root, + dryRun: true, + }); + + expect(output).toContain('(dry run)'); + expect(output).toContain('Card: moved existing styles into'); + expect(output).toContain('Switch.container: swapped baseline styles'); + expect(output).toContain('would create 1 file(s)'); + expect(readFileSync(cardPath, 'utf8')).toBe(before); + expect( + existsSync( + path.join(root, 'theme', 'components', 'BooleanField.styles.ts') + ) + ).toBe(false); + }); + + test('applies edits, scaffolds required components, updates the barrel', async () => { + const root = setupFixture(); + const components = path.join(root, 'theme', 'components'); + + const { output } = await runMigrate({ + version: 'v18', + targetPath: root, + dryRun: false, + }); + + const card = readFileSync(path.join(components, 'Card.styles.ts'), 'utf8'); + expect(card).toContain('container: cva({'); + expect(card).toContain('media: cva({}),'); + + const switchSource = readFileSync( + path.join(components, 'Switch.styles.ts'), + 'utf8' + ); + expect(switchSource).toContain(`'grid gap-x-2 items-center'`); + + const scaffold = readFileSync( + path.join(components, 'BooleanField.styles.ts'), + 'utf8' + ); + expect(scaffold).toContain(`ThemeComponent<'BooleanField'>`); + + const index = readFileSync(path.join(components, 'index.ts'), 'utf8'); + expect(index).toContain(`export * from './BooleanField.styles';`); + + expect(output).toContain('created'); + expect(output).toContain('Run your typechecker'); + }); + + test('is idempotent: a second run changes nothing', async () => { + const root = setupFixture(); + await runMigrate({ version: 'v18', targetPath: root, dryRun: false }); + const components = path.join(root, 'theme', 'components'); + const snapshot = ['Card.styles.ts', 'Switch.styles.ts', 'index.ts'].map(f => + readFileSync(path.join(components, f), 'utf8') + ); + + const { output } = await runMigrate({ + version: 'v18', + targetPath: root, + dryRun: false, + }); + + expect( + ['Card.styles.ts', 'Switch.styles.ts', 'index.ts'].map(f => + readFileSync(path.join(components, f), 'utf8') + ) + ).toEqual(snapshot); + expect(output).toContain('Edited 0 file(s)'); + }); + + test('reports when no Marigold imports are found', async () => { + const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-migrate-')); + writeFileSync(path.join(root, 'app.ts'), `export const x = 1;\n`); + + const { output } = await runMigrate({ + version: 'v18', + targetPath: root, + dryRun: true, + }); + + expect(output).toContain('No files importing'); + }); + + test('applies safe application-code codemods alongside theme codemods', async () => { + const root = setupFixture(); + const appFile = path.join(root, 'app', 'Profile.tsx'); + mkdirSync(path.dirname(appFile), { recursive: true }); + writeFileSync( + appFile, + `import { Inset, Tabs, TextField, Tooltip } from '@marigold/components'; + +export const Profile = () => ( + + + + + hint + + + +); +` + ); + + const { output } = await runMigrate({ + version: 'v18', + targetPath: root, + dryRun: false, + }); + + const app = readFileSync(appFile, 'utf8'); + expect(app).toContain(''); + expect(app).toContain(''); + expect(app).toContain(''); + expect(app).toContain(''); + expect(app).toContain(''); // warning only, never edited + expect(output).toContain('Tooltip[open]'); + }); +}); diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts new file mode 100644 index 0000000000..c055ad0960 --- /dev/null +++ b/packages/cli/src/commands/migrate.ts @@ -0,0 +1,299 @@ +import pc from 'picocolors'; +import { existsSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import { findThemeComponents } from '../lib/codemod/anchor.js'; +import { + codeList, + detectIndentUnit, + stylesReference, +} from '../lib/codemod/engine.js'; +import { v18 } from '../lib/codemod/manifests/v18.js'; +import { + removeJsxProps, + renameImports, + renameJsxMembers, + renameJsxProps, + reportJsxUsage, +} from '../lib/codemod/primitives/jsx.js'; +import { + reportDeadKeys, + reportStructure, +} from '../lib/codemod/primitives/report.js'; +import { restructureToSlots } from '../lib/codemod/primitives/restructure-to-slots.js'; +import { + addIndexExport, + generateScaffold, +} from '../lib/codemod/primitives/scaffold-component.js'; +import { stubMissingSlots } from '../lib/codemod/primitives/stub-missing-slots.js'; +import { swapExactClasses } from '../lib/codemod/primitives/swap-exact-classes.js'; +import type { Codemod, MigrationManifest } from '../lib/codemod/types.js'; +import { highlightCode } from '../lib/format.js'; +import type { AnyNode } from '../lib/tsx-ast.js'; +import { parseTsx } from '../lib/tsx-ast.js'; + +const MANIFESTS: Record = { v18 }; + +export interface MigrateOptions { + version: string; + targetPath: string; + dryRun: boolean; +} + +export interface MigrateResult { + output: string; +} + +const IGNORED_DIRS = new Set([ + 'node_modules', + 'dist', + 'build', + 'coverage', + 'storybook-static', + '.git', + '.next', + '.turbo', +]); + +const collectSourceFiles = (dir: string): string[] => { + const out: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.isDirectory()) { + if (IGNORED_DIRS.has(entry.name) || entry.name.startsWith('.')) continue; + out.push(...collectSourceFiles(path.join(dir, entry.name))); + } else if (/\.tsx?$/.test(entry.name) && !entry.name.endsWith('.d.ts')) { + out.push(path.join(dir, entry.name)); + } + } + return out; +}; + +interface FileReport { + file: string; + changes: string[]; + warnings: string[]; + skips: string[]; + output: string; +} + +const edited = (report: FileReport): boolean => report.changes.length > 0; + +const applyPipeline = ( + codemods: Codemod[], + source: string, + file: string +): FileReport => { + const report: FileReport = { + file, + changes: [], + warnings: [], + skips: [], + output: source, + }; + for (const codemod of codemods) { + // each transform re-parses its input when chained; babel parse is fast + // and shared-AST plumbing is not worth it + const outcome = codemod.apply(report.output); + if (outcome.kind === 'skipped') { + report.skips.push(`${codemod.name}: ${outcome.reason}`); + continue; + } + report.warnings.push(...outcome.warnings); + if (outcome.kind === 'edited') { + report.output = outcome.output; + report.changes.push(...outcome.changes); + } + } + return report; +}; + +export const runMigrate = (options: MigrateOptions): MigrateResult => { + const manifest = MANIFESTS[options.version]; + if (!manifest) { + throw new Error( + `Unknown migration '${options.version}' (available: ${Object.keys(MANIFESTS).join(', ')})` + ); + } + + // read each candidate once; every later pass works on this snapshot + const root = path.resolve(options.targetPath); + // '@marigold/' covers system (themes), components (JSX) and icons (imports) + const sources = new Map(); + for (const file of collectSourceFiles(root).sort()) { + const source = readFileSync(file, 'utf8'); + if (source.includes('@marigold/')) { + sources.set(file, source); + } + } + + // pass 1: inventory of theme components across the consumer tree + const inventory = new Map(); + for (const [file, source] of sources) { + let ast; + try { + ast = parseTsx(source); + } catch { + continue; // unparseable files are reported by the pipeline pass + } + for (const decl of findThemeComponents(ast as unknown as AnyNode)) { + if (!inventory.has(decl.component)) inventory.set(decl.component, file); + } + } + + // pass 2: per-file pipeline. Order matters: restructure first (single-cva + // becomes a slot object), swap before stubbing (a stubbed cva({}) must not + // be mistaken for a customized slot), reports last. The JSX transforms are + // independent of the theme ones; they anchor on @marigold/components. + const codemods = [ + restructureToSlots(manifest), + swapExactClasses(manifest), + stubMissingSlots(manifest), + renameJsxMembers(manifest), + renameJsxProps(manifest), + removeJsxProps(manifest), + renameImports(manifest), + reportDeadKeys(manifest), + reportStructure(manifest), + reportJsxUsage(manifest), + ]; + const reports: FileReport[] = []; + for (const [file, source] of sources) { + const report = applyPipeline(codemods, source, file); + if ( + edited(report) || + report.warnings.length > 0 || + report.skips.length > 0 + ) { + reports.push(report); + } + if (edited(report) && !options.dryRun) { + writeFileSync(file, report.output); + } + } + + // pass 3: scaffold components the target version added, next to the theme + // files that require them, and register them in the local barrel file. + const created: string[] = []; + const scaffoldWarnings: string[] = []; + for (const entry of manifest.scaffolds) { + if (inventory.has(entry.name)) continue; + const host = entry.requiredBy.map(c => inventory.get(c)).find(Boolean); + if (!host) continue; + const moduleBase = `${entry.name}.styles`; + const dir = path.dirname(host); + const target = path.join(dir, `${moduleBase}.ts`); + if (existsSync(target)) { + scaffoldWarnings.push( + `${path.relative(root, target)} already exists but does not define ${entry.name} — check manually` + ); + continue; + } + const content = generateScaffold( + entry, + manifest, + detectIndentUnit(sources.get(host) ?? '') + ); + if (!options.dryRun) writeFileSync(target, content); + const found = entry.requiredBy.filter(c => inventory.has(c)); + const because = entry.reason + ? ` — ${entry.reason} (your theme defines ${codeList(found)})` + : ''; + const reference = stylesReference(manifest, entry.name); + created.push( + `${path.relative(root, target)}${because}${ + reference ? ` — reference styles: ${reference}` : '' + }` + ); + + const index = path.join(dir, 'index.ts'); + if (existsSync(index)) { + const outcome = addIndexExport( + moduleBase, + `registers the scaffolded \`${entry.name}\` styles in the theme barrel` + ).apply(readFileSync(index, 'utf8')); + if (outcome.kind === 'edited') { + if (!options.dryRun) writeFileSync(index, outcome.output); + reports.push({ + file: index, + changes: outcome.changes, + warnings: [], + skips: [], + output: outcome.output, + }); + } + } else { + scaffoldWarnings.push( + `${path.relative(root, dir)}/index.ts not found — export ${entry.name} from your theme manually` + ); + } + } + + // render report. picocolors is TTY-aware: piped / test / agent output stays + // byte-for-byte plain, only interactive terminals gain color. + // `code` spans render cyan (same convention as doctor), URLs cyan+underline. + const decorate = (text: string): string => + highlightCode( + text.replace(/https?:\/\/\S+/g, url => pc.underline(pc.cyan(url))) + ); + // Multi-line warnings carry a token diff; continuation lines starting with + // -/+ render like a git diff. + const pushWarning = (lines: string[], warning: string): void => { + const [first, ...rest] = warning.split('\n'); + lines.push(` ${pc.yellow('!')} ${decorate(first)}`); + for (const cont of rest) { + const colored = cont.startsWith('+') + ? pc.green(cont) + : cont.startsWith('-') + ? pc.red(cont) + : cont; + lines.push(` ${colored}`); + } + }; + const lines: string[] = [ + pc.bold( + `marigold migrate ${manifest.version}${options.dryRun ? ' (dry run)' : ''}` + ), + '', + ]; + if (sources.size === 0) { + lines.push( + `No files importing @marigold/system or @marigold/components found under ${root}.` + ); + return { output: lines.join('\n') }; + } + for (const report of reports) { + lines.push(pc.bold(path.relative(root, report.file))); + for (const change of report.changes) { + lines.push(` ${pc.green('~')} ${decorate(change)}`); + } + for (const warning of report.warnings) pushWarning(lines, warning); + for (const skip of report.skips) lines.push(pc.dim(` - ${skip}`)); + lines.push(''); + } + for (const file of created) { + lines.push(`${pc.green('+')} created ${decorate(file)}`); + } + for (const warning of scaffoldWarnings) { + lines.push(`${pc.yellow('!')} ${decorate(warning)}`); + } + if (created.length > 0 || scaffoldWarnings.length > 0) lines.push(''); + + const editedCount = reports.filter(edited).length; + const warningCount = + reports.reduce((n, r) => n + r.warnings.length, 0) + + scaffoldWarnings.length; + lines.push( + pc.bold( + `${options.dryRun ? 'Would edit' : 'Edited'} ${editedCount} file(s), ` + + `${options.dryRun ? 'would create' : 'created'} ${created.length} file(s), ` + + `${warningCount} warning(s).` + ) + ); + if (!options.dryRun) { + lines.push( + pc.dim( + `Run your typechecker now — the exhaustive slot Records in @marigold/system make it the completeness check.` + ) + ); + } + return { output: lines.join('\n') }; +}; diff --git a/packages/cli/src/lib/commands-spec.ts b/packages/cli/src/lib/commands-spec.ts index 1d47d868df..db27c49ece 100644 --- a/packages/cli/src/lib/commands-spec.ts +++ b/packages/cli/src/lib/commands-spec.ts @@ -47,6 +47,7 @@ export type SubcommandName = | 'examples' | 'init' | 'doctor' + | 'migrate' | 'telemetry' | 'completion'; @@ -104,6 +105,10 @@ export const SUBCOMMANDS: readonly SubcommandSpec[] = [ { name: '--offline', type: 'boolean' }, ], }, + { + name: 'migrate', + flags: [{ name: '--dry-run', type: 'boolean' }], + }, { name: 'telemetry', positionalKind: 'telemetry-sub', diff --git a/packages/cli/src/lib/telemetry.ts b/packages/cli/src/lib/telemetry.ts index 336f0aa953..37bb18100a 100644 --- a/packages/cli/src/lib/telemetry.ts +++ b/packages/cli/src/lib/telemetry.ts @@ -15,6 +15,7 @@ export type CommandName = | 'examples' | 'init' | 'doctor' + | 'migrate' | 'telemetry'; export interface TelemetryEvent { From 55fe815c7a7f81ae7e6e755d58e080ac1bc8d30f Mon Sep 17 00:00:00 2001 From: aromko Date: Thu, 23 Jul 2026 16:50:38 +0200 Subject: [PATCH 04/11] docs(DST-1543): document the codemod engine Usage and report legend, the pipeline order and why it matters, the invariants (never override, byte-preserving, warn-never-guess, idempotent, Theme contract as a stable API), how to author a v19 manifest, when to add a primitive vs a one-off transform, and known limitations. --- packages/cli/src/lib/codemod/README.md | 179 +++++++++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 packages/cli/src/lib/codemod/README.md diff --git a/packages/cli/src/lib/codemod/README.md b/packages/cli/src/lib/codemod/README.md new file mode 100644 index 0000000000..3b9c852f8d --- /dev/null +++ b/packages/cli/src/lib/codemod/README.md @@ -0,0 +1,179 @@ +# Marigold migration codemods + +Codemods for breaking style and HTML-structure changes in consumer themes +(DST-1543). The design goal: **per-version work is data, not code**. The +transforms in `primitives/` are version-agnostic and should never need +touching for a new release; everything release-specific lives in a manifest. + +## Usage + +```sh +npx marigold migrate v18 [path] [--dry-run] +``` + +- `path` defaults to the current directory; point it at the consumer repo + (or its theme directory) you want to migrate. +- **Always run `--dry-run` first** and read the report. +- After a real run, run the consumer's typechecker: the slot Records in + `@marigold/system`'s `Theme` type are exhaustive, so the typecheck is the + built-in completeness check for anything the codemod could not fix. + +Report legend: + +| Prefix | Meaning | +| ------ | ------------------------------------------------------------- | +| `~` | change applied (or would be applied, in dry-run) | +| `!` | warning: needs a human decision, the codemod did not touch it | +| `-` | file skipped (e.g. parse error) | +| `+` | file created (scaffolded component styles) | + +## How it works + +``` +commands/migrate.ts runner: file scan, inventory, pipeline, scaffolds +lib/codemod/ + types.ts Codemod / CodemodOutcome / MigrationManifest + anchor.ts finds ThemeComponent<'X'> declarations + engine.ts indent detection, reindent, class-string helpers + primitives/ version-agnostic transforms (see below) + manifests/v18.ts per-version data driving the primitives +``` + +Consumer code is located via the **type anchor**: a +`const X: ThemeComponent<'Name'> = ...` declaration whose `ThemeComponent` +verifiably comes from `@marigold/system`. File names and directory layout do +not matter. Anything that cannot be found or verified becomes a warning, +never a guess. + +The pipeline order matters and is fixed in `commands/migrate.ts`: + +1. `restructure-to-slots`: single-cva components become slot objects; the + consumer's cva moves **verbatim** into the primary slot. +2. `swap-exact-classes`: baseline slots are swapped to the new baseline, + but only on a byte-exact match (runs before stubbing so a fresh `cva({})` + stub is never mistaken for a customized slot). +3. `stub-missing-slots`: missing slot keys are added as `cva({})` stubs; + dropped slot keys are reported. +4. `rename-jsx-members` / `rename-jsx-props` / `remove-jsx-props`: safe + application-code edits (see below). +5. `report-dead-keys` / `report-structure` / `report-jsx-usage`: report-only + passes. + +Scaffolding (new components like `BooleanField`) runs last, per manifest +entry, next to the theme file of a component that requires it, and registers +the new file in the local barrel `index.ts`. + +## Application code + +Besides theme files, the pipeline applies **safe** codemods to application +code (`primitives/jsx.ts`, driven by the manifest's `jsx` section). Safe +means lexically decidable, no judgment involved: + +- prop renames (`Inset` `space`/`spaceX`/`spaceY` to `p`/`px`/`py`, + `acceptedFileType` to `acceptedFileTypes` including the array wrap) +- compound-member renames (`Tabs.TabPanel` to `Tabs.Panel`, opening and + closing tags) +- removals of props the target version dropped (`TextField` `min`/`max`) + +The anchor here is the **import**: a JSX element only counts as a Marigold +component when its (possibly aliased) name is imported from +`@marigold/components`. Only explicit JSX attributes are touched; props +hidden behind spreads are unreachable by design and left to the consumer's +typecheck. + +**Renamed exports** (the v18 icon migration, driven by the manifest's +`jsx.importRenames` and the official mapping table in +`.changeset/iconography-docs.md`) are renamed **directly** — import and +every usage (`` becomes ``) — when the file provably +allows it. Member accesses, object keys, and attribute names with the same +spelling are never touched. When a direct rename is not provably safe (the +old name is shadowed or used as a shorthand property, or the new name +already exists in the file), the specifier falls back to the +release-notes-blessed alias form (`Store as Pickup`), which keeps every +call site valid, and the report names the reason. Re-running is a no-op +either way. + +Everything that needs a structural JSX move or a design decision is a +warning, never an edit: `Tooltip open` (moves to `Tooltip.Trigger`), +`width="fit"` (removed, needs a chosen width), `Multiselect` (removed, the +`TagField` replacement has a different API), and `Switch size="large"`. That +last one is deliberately NOT auto-removed: the `size` prop still exists in +v18 and a standalone theme may define its own size variants; only the +default theme dropped `large`. + +## What is important (invariants) + +1. **Never override, only add.** Consumer class strings survive + byte-for-byte. The only exception is `swap-exact-classes`, and it fires + only when the consumer's class strings equal the old Marigold baseline + exactly, which proves the slot was never customized. The check is per + slot, not per file. Any deviation: warn, do not touch. +2. **Byte-preserving edits.** Everything is `magic-string`; untouched code + is never re-printed, so a consumer's differing Prettier style produces no + diff churn. +3. **Warn, never guess.** Spread elements, non-cva helpers, unmatched + baselines: all bail to structured warnings. +4. **Idempotent.** Re-running a migration is a no-op; already-swapped slots + are recognized as being on the target baseline. +5. **The Theme contract is a stable API of this system.** The primitives and + the manifest format assume optional component keys, exhaustive slot + Records, and cva-based style functions. Changing that contract in + `@marigold/system` is itself a breaking change: bump the manifest + `schemaVersion`, keep the old interpreter so chained migrations + (v17 to v19) still work. + +## Adding a new migration (e.g. v19) + +Per-version work should be data entry, not transform code: + +1. Create `manifests/v19.ts`. Follow the provenance comments in `v18.ts`: + - `slots`: extract from the `Theme` type of the target version (see the + type-diff approach documented in `v18.ts`; a codegen script is the + planned step 3 of DST-1543 and will generate this file). + - `swaps`: hand-author only the `{ component, slot }` targets. The + `oldClasses` strings must be extracted from the **previous major's + published theme-rui** and `newSource` from the current one. Never type + class strings by hand; a single whitespace difference silently disables + the swap. + - `restructures`: components whose type changed from a single style + function to a slot Record, plus the slot that receives the existing + styles (`container` is the default). + - `structureWarnings`: hand-written texts for DOM changes that break + consumer CSS selectors. Not auto-fixable, but must not go unannounced. + - Warning links: point at the **source in this repo**, as GitHub + permalinks pinned to a commit SHA or release tag (never a branch name: + line numbers rot). Link the line that answers "what do I replace this + with", e.g. the prop type that excludes the removed value. Codegen + should re-resolve line numbers against the release tag. +2. Register it in `MANIFESTS` in `commands/migrate.ts`. +3. Acceptance test: `--dry-run` against a real consumer repo and read every + line of the report. + +A change that no primitive can express gets a **one-off transform** for that +version (same `Codemod` interface, added to the pipeline). Promote it to a +primitive only when the same shape shows up in a second major; designing a +general abstraction from a single example is how wrong abstractions happen. + +## Adding or changing a primitive + +- A primitive is a factory: `(manifest: MigrationManifest) => Codemod`. It + must return `edited | unchanged | skipped` plus warnings, re-parse its own + input (transforms are chained by re-parsing; babel parse is fast), and + uphold the invariants above. +- Add fixture tests in `codemod.test.ts`. Fixtures are consumer-shaped + (4-space indent, single quotes, portal-style files), and byte-preservation + is asserted on the fixture's class strings. +- Wire it into the pipeline in `commands/migrate.ts`; think about ordering + (see above). + +## Known limitations + +- The swap token warning lists every string literal that is new relative to + the old baseline, which includes variant names like `default`. Noise, but + safe. Upgrade path: resolve utilities against the consumer's actual CSS. +- Only annotated `ThemeComponent<'X'>` declarations are anchored. A theme + assembled as one inline `Theme`-typed object without per-component + annotations is not found yet (no known consumer does this). +- HTML-structure changes are report-only by design: consumers with their own + CSS against Marigold's DOM (e.g. generated BEM selectors) must review the + named components manually. From 09e60574ca3d024e4331e8de9a44875761f89781 Mon Sep 17 00:00:00 2001 From: aromko Date: Thu, 23 Jul 2026 16:50:40 +0200 Subject: [PATCH 05/11] docs(DST-1213): add the missing Print mapping to the icon table v17 exported a custom Print icon (info/Print) but the migration table in the iconography changeset has no row for it; Printer is the Lucide equivalent. Found while generating the DST-1543 icon-rename codemod from this table. --- .changeset/iconography-docs.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/iconography-docs.md b/.changeset/iconography-docs.md index 8b2d35582d..9a6ab50c92 100644 --- a/.changeset/iconography-docs.md +++ b/.changeset/iconography-docs.md @@ -68,6 +68,7 @@ Old names from the legacy `@marigold/icons` set and their Lucide replacement. Na | Notification | `MessageSquareWarning` | | Parking | `CircleParking` | | PDF | `PDF` _(custom)_ | +| Print | `Printer` | | Reports | `FileText` | | Required | `Asterisk` | | ResaleLogbook | `BookOpenText` | From 2d7d8a0a1e3a4279b88f2c0c2a3edcd7dab81278 Mon Sep 17 00:00:00 2001 From: aromko Date: Thu, 23 Jul 2026 17:21:35 +0200 Subject: [PATCH 06/11] feat(DST-1543): detect the installed version when migrate has none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `marigold migrate` (version omitted) walks up from the target path to find @marigold/components — installed node_modules first, the declared package.json range as fallback — proposes the applicable migration(s) in order, and runs them after an interactive confirm (Enter accepts). The proposal includes the installed major itself (>=, not >): the documented procedure is upgrade-then-migrate, so a consumer on 18.0.0-beta is exactly who needs the v18 migration, and re-running is a no-op. Non-interactive sessions get the detected proposal plus the explicit command to run instead of a hanging prompt; a repo with no Marigold at all fails with guidance. --- packages/cli/src/bin/marigold.ts | 75 +++++++++++++++++++---- packages/cli/src/commands/migrate.test.ts | 66 +++++++++++++++++++- packages/cli/src/commands/migrate.ts | 54 ++++++++++++++++ packages/cli/src/lib/codemod/README.md | 7 ++- 4 files changed, 188 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/bin/marigold.ts b/packages/cli/src/bin/marigold.ts index cfcf782bda..512708aa6b 100644 --- a/packages/cli/src/bin/marigold.ts +++ b/packages/cli/src/bin/marigold.ts @@ -68,7 +68,7 @@ ${pc.bold('Commands:')} examples Browse application patterns (list | get ) init Set up Marigold in a project doctor Diagnose a project's Marigold setup - migrate Apply codemods for a breaking Marigold release + migrate [version] Apply codemods for a breaking Marigold release telemetry Manage telemetry (status|enable|disable) completion Print shell completion script (bash|zsh|fish) @@ -104,6 +104,9 @@ ${pc.bold('Doctor options:')} --offline Skip the network; use only the local cache ${pc.bold('Migrate options:')} + [version] Migration to run (e.g. v18). When omitted, the + installed @marigold/components version is detected + and the proposed migration confirmed interactively [path] Directory to migrate (default: current directory) --dry-run Report what would change without writing files @@ -435,28 +438,76 @@ export const main = async ( if (result.hasErrors) exitCode = 1; } else if (command === 'migrate') { const { positionals, values } = parseMigrateCommand(rest); - const [version, targetPath] = positionals; + // the version positional is optional: `migrate ./src` treats the first + // positional as a path, `migrate v18 ./src` as version + path + const looksLikeVersion = (p: string | undefined): p is string => + p !== undefined && /^v?\d+$/.test(p); + const [first, second] = positionals; + const explicitVersion = looksLikeVersion(first) ? first : undefined; + const targetPath = (explicitVersion ? second : first) ?? process.cwd(); telemetryArgs = { - version: version ?? '', + version: explicitVersion ?? 'auto', ...(values['dry-run'] ? { dryRun: 'true' } : {}), }; - if (!version || positionals.length > 2) { - fail('Usage: marigold migrate [path] [--dry-run]'); + if (positionals.length > (explicitVersion ? 2 : 1)) { + fail('Usage: marigold migrate [version] [path] [--dry-run]'); } // Lazy-load: migrate pulls in @babel/parser and magic-string, which we // keep off the docs/list hot path. - const { runMigrate } = await import('../commands/migrate.js'); - const result = await runMigrate({ + const { detectMigration, runMigrate } = + await import('../commands/migrate.js'); + + let versions: string[]; + if (explicitVersion) { // accept both `18` and `v18` - version: version.startsWith('v') ? version : `v${version}`, - targetPath: targetPath ?? process.cwd(), - dryRun: values['dry-run'], - }); + versions = [ + explicitVersion.startsWith('v') + ? explicitVersion + : `v${explicitVersion}`, + ]; + } else { + const detected = detectMigration(targetPath); + if (!detected) { + fail( + `Could not find @marigold/components under ${targetPath} — pass the migration explicitly: marigold migrate v18 [path]` + ); + } + if (detected.versions.length === 0) { + writeOutput( + `Detected @marigold/components ${detected.installed} (${detected.source}) — already up to date, no migration to run.` + ); + return 0; + } + if (!process.stdout.isTTY) { + fail( + `Detected @marigold/components ${detected.installed} — would run ${detected.versions.join(', then ')}. ` + + `Non-interactive session: confirm by passing the version explicitly, e.g. marigold migrate ${detected.versions[0]} [path]` + ); + } + const { confirm, isCancel } = await import('@clack/prompts'); + const proceed = await confirm({ + message: + `Detected @marigold/components ${detected.installed} (${detected.source}). ` + + `Run the ${detected.versions.join(', then the ')} migration${values['dry-run'] ? ' (dry run)' : ''}?`, + }); + if (isCancel(proceed) || proceed !== true) { + writeOutput('Aborted — nothing changed.'); + return 130; + } + versions = detected.versions; + } - writeOutput(result.output); + for (const version of versions) { + const result = await runMigrate({ + version, + targetPath, + dryRun: values['dry-run'], + }); + writeOutput(result.output); + } } else if (command === 'telemetry') { const [sub] = rest; telemetryArgs = sub ? { sub } : {}; diff --git a/packages/cli/src/commands/migrate.test.ts b/packages/cli/src/commands/migrate.test.ts index 43d260322e..0f414626c5 100644 --- a/packages/cli/src/commands/migrate.test.ts +++ b/packages/cli/src/commands/migrate.test.ts @@ -2,7 +2,7 @@ import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; import { existsSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { runMigrate } from './migrate.js'; +import { detectMigration, runMigrate } from './migrate.js'; // End-to-end run against a miniature portal-shaped theme tree: standalone // theme, 4-space indent, one style file per component, barrel index. @@ -49,6 +49,70 @@ const setupFixture = (): string => { return root; }; +describe('detectMigration', () => { + const setupRepo = (version: string, installed = true): string => { + const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-detect-')); + if (installed) { + const pkgDir = path.join(root, 'node_modules', '@marigold', 'components'); + mkdirSync(pkgDir, { recursive: true }); + writeFileSync( + path.join(pkgDir, 'package.json'), + JSON.stringify({ name: '@marigold/components', version }) + ); + } else { + writeFileSync( + path.join(root, 'package.json'), + JSON.stringify({ dependencies: { '@marigold/components': version } }) + ); + } + // the migration target usually sits far below the repo root + const target = path.join(root, 'src', 'theme'); + mkdirSync(target, { recursive: true }); + return target; + }; + + test('walks up to node_modules and proposes the applicable migration', () => { + const detected = detectMigration(setupRepo('17.9.1')); + expect(detected).toEqual({ + installed: '17.9.1', + source: 'node_modules', + versions: ['v18'], + }); + }); + + test('falls back to the declared range when nothing is installed', () => { + const detected = detectMigration(setupRepo('^17.0.0', false)); + expect(detected).toEqual({ + installed: '17.0.0', + source: 'package.json', + versions: ['v18'], + }); + }); + + test('proposes the same-major migration (upgrade first, then migrate)', () => { + const detected = detectMigration(setupRepo('18.0.0-beta.4')); + expect(detected).toEqual({ + installed: '18.0.0-beta.4', + source: 'node_modules', + versions: ['v18'], + }); + }); + + test('reports up to date when the installed major is past every migration', () => { + const detected = detectMigration(setupRepo('19.0.0')); + expect(detected).toEqual({ + installed: '19.0.0', + source: 'node_modules', + versions: [], + }); + }); + + test('returns null when no @marigold/components exists anywhere', () => { + const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-detect-')); + expect(detectMigration(root)).toBeNull(); + }); +}); + describe('runMigrate', () => { test('rejects unknown migration versions', () => { expect(() => diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index c055ad0960..1e8e8d5e29 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -27,12 +27,66 @@ import { import { stubMissingSlots } from '../lib/codemod/primitives/stub-missing-slots.js'; import { swapExactClasses } from '../lib/codemod/primitives/swap-exact-classes.js'; import type { Codemod, MigrationManifest } from '../lib/codemod/types.js'; +import { + declaredVersion, + installedVersion, + readPackageJson, +} from '../lib/doctor/package-json.js'; +import { minVersionFromRange } from '../lib/doctor/version.js'; import { highlightCode } from '../lib/format.js'; import type { AnyNode } from '../lib/tsx-ast.js'; import { parseTsx } from '../lib/tsx-ast.js'; const MANIFESTS: Record = { v18 }; +export interface DetectedMigration { + /** installed @marigold/components version (or the declared range minimum) */ + installed: string; + /** where the version was read from */ + source: 'node_modules' | 'package.json'; + /** applicable migrations in run order, e.g. ['v18'] — empty: up to date */ + versions: string[]; +} + +/** + * Detect the consumer's Marigold version by walking up from the target path + * (theme directories usually sit far below the repo root that owns + * node_modules), preferring the installed package over the declared range. + * Returns null when no @marigold/components is found at all. + */ +export const detectMigration = ( + targetPath: string +): DetectedMigration | null => { + const found = ((): Omit | null => { + let dir = path.resolve(targetPath); + for (;;) { + const installed = installedVersion(dir, '@marigold/components'); + if (installed) return { installed, source: 'node_modules' }; + const declared = declaredVersion( + readPackageJson(path.join(dir, 'package.json')), + '@marigold/components' + ); + const min = declared ? minVersionFromRange(declared) : null; + if (min) return { installed: min, source: 'package.json' }; + const parent = path.dirname(dir); + if (parent === dir) return null; + dir = parent; + } + })(); + if (!found) return null; + + // Propose migrations targeting the installed major too (>=, not >): the + // documented procedure is "upgrade the packages, then migrate", so someone + // on 18.x is exactly who needs the v18 migration. Re-running is a no-op. + const major = Number.parseInt(found.installed, 10); + const versions = Object.keys(MANIFESTS) + .map(v => ({ v, target: Number.parseInt(v.replace(/^v/, ''), 10) })) + .filter(({ target }) => Number.isFinite(target) && target >= major) + .sort((a, b) => a.target - b.target) + .map(({ v }) => v); + return { ...found, versions }; +}; + export interface MigrateOptions { version: string; targetPath: string; diff --git a/packages/cli/src/lib/codemod/README.md b/packages/cli/src/lib/codemod/README.md index 3b9c852f8d..00594ce199 100644 --- a/packages/cli/src/lib/codemod/README.md +++ b/packages/cli/src/lib/codemod/README.md @@ -8,9 +8,14 @@ touching for a new release; everything release-specific lives in a manifest. ## Usage ```sh -npx marigold migrate v18 [path] [--dry-run] +npx marigold migrate [version] [path] [--dry-run] ``` +- `version` is optional: when omitted, the installed + `@marigold/components` is detected (walking up from the target path, + falling back to the declared package.json range) and the applicable + migration is proposed for interactive confirmation — Enter runs it. + Non-interactive sessions must pass the version explicitly. - `path` defaults to the current directory; point it at the consumer repo (or its theme directory) you want to migrate. - **Always run `--dry-run` first** and read the report. From fdf177d61b826baaa1ef90351521439bb5070ce3 Mon Sep 17 00:00:00 2001 From: aromko Date: Fri, 24 Jul 2026 14:40:36 +0200 Subject: [PATCH 07/11] feat(DST-1543): detect design-token breakage in marigold migrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Token breakage compiles and typechecks fine and only shows up in the browser, so the manifest gains a `tokens` section driving report-only checks (never edits — token values are consumer property): - renamed/removed tokens still referenced (`bg-brand` -> use `bg-primary`; text scan over every ts/tsx/css file, utilities and raw var() reads) - new tokens components hardcode internally (v18: SelectList's selection indicator needs `selected-bold`/`disabled-surface`) but the consumer CSS does not define — the classes bypass the theme layer entirely - repurposed tokens that kept their name but changed meaning (`disabled` flipped background->text, `secondary` surface->text, status tokens solid->muted): definition sites get a remap recipe until the `settledBy` token appears; consumers not defining them get old-role usage warnings Warnings are suppressed per token when the consumer's own CSS defines it (their vocabulary, still self-consistent), and theme-rui consumers count every added token as defined by construction. All manifest data is value-verified against theme-rui v17.9.1 vs v18 and codegen-able (DST-1650). vendor/ and *.min.css are skipped as build artifacts. --- .changeset/migrate-codemods.md | 2 +- packages/cli/src/commands/migrate.test.ts | 49 ++++ packages/cli/src/commands/migrate.ts | 55 +++- packages/cli/src/lib/codemod/README.md | 60 ++++- packages/cli/src/lib/codemod/manifests/v18.ts | 143 +++++++++++ .../cli/src/lib/codemod/primitives/jsx.ts | 2 +- .../cli/src/lib/codemod/primitives/tokens.ts | 235 ++++++++++++++++++ packages/cli/src/lib/codemod/tokens.test.ts | 224 +++++++++++++++++ packages/cli/src/lib/codemod/types.ts | 57 +++++ 9 files changed, 818 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/lib/codemod/primitives/tokens.ts create mode 100644 packages/cli/src/lib/codemod/tokens.test.ts diff --git a/.changeset/migrate-codemods.md b/.changeset/migrate-codemods.md index 9ea2c087d9..0f9514d888 100644 --- a/.changeset/migrate-codemods.md +++ b/.changeset/migrate-codemods.md @@ -2,4 +2,4 @@ '@marigold/cli': minor --- -feat(DST-1543): add `marigold migrate ` codemods for breaking Marigold releases. The v18 migration restructures theme files to the new slot shapes (never overriding consumer classes), swaps exact-baseline layout classes with a token diff report, scaffolds missing theme components, applies safe application-code renames (icon imports per the official mapping, `Tabs.TabPanel`/`SelectList.Item`, `Inset` spacing props, `TextField` min/max), and reports everything that needs a human decision with pinned source links. Run `npx marigold migrate v18 --dry-run` first. +feat(DST-1543): add `marigold migrate ` codemods for breaking Marigold releases. The v18 migration restructures theme files to the new slot shapes (never overriding consumer classes), swaps exact-baseline layout classes with a token diff report, scaffolds missing theme components, applies safe application-code renames (icon imports per the official mapping, `Tabs.TabPanel`/`SelectList.Item`, `Inset` spacing props, `TextField` min/max), and reports everything that needs a human decision with pinned source links. The report also covers design-token breakage that no typecheck can see: renamed/removed tokens still referenced, new tokens components require but the consumer CSS does not define, and repurposed tokens that kept their name but changed meaning (with a remap recipe at the definition site). Run `npx marigold migrate v18 --dry-run` first. diff --git a/packages/cli/src/commands/migrate.test.ts b/packages/cli/src/commands/migrate.test.ts index 0f414626c5..d8044cb182 100644 --- a/packages/cli/src/commands/migrate.test.ts +++ b/packages/cli/src/commands/migrate.test.ts @@ -246,4 +246,53 @@ export const Profile = () => ( expect(app).toContain(''); // warning only, never edited expect(output).toContain('Tooltip[open]'); }); + + test('reports token findings in CSS files and component internals', () => { + const root = setupFixture(); + writeFileSync( + path.join(root, 'theme', 'tokens.css'), + `:root { --color-brand: #f80; } +.help { color: var(--color-muted-foreground); } +` + ); + writeFileSync( + path.join(root, 'List.tsx'), + `import { SelectList } from '@marigold/components'; +export const List = () => ; +` + ); + + const { output } = runMigrate({ + version: 'v18', + targetPath: root, + dryRun: true, + }); + + expect(output).toContain('`muted-foreground` token was renamed'); + expect(output).not.toContain('`--color-brand`'); // defined by the consumer + expect(output).toContain('SelectList: its v18 implementation hardcodes'); + }); + + test('does not warn about added tokens when the consumer uses theme-rui', () => { + const root = setupFixture(); + writeFileSync( + path.join(root, 'setup.ts'), + `import '@marigold/theme-rui/styles.css'; +` + ); + writeFileSync( + path.join(root, 'List.tsx'), + `import { SelectList } from '@marigold/components'; +export const List = () => ; +` + ); + + const { output } = runMigrate({ + version: 'v18', + targetPath: root, + dryRun: true, + }); + + expect(output).not.toContain('hardcodes'); + }); }); diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index 1e8e8d5e29..49de6a75c4 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -26,6 +26,11 @@ import { } from '../lib/codemod/primitives/scaffold-component.js'; import { stubMissingSlots } from '../lib/codemod/primitives/stub-missing-slots.js'; import { swapExactClasses } from '../lib/codemod/primitives/swap-exact-classes.js'; +import { + definedTokensIn, + reportTokenDependencies, + reportTokenUsage, +} from '../lib/codemod/primitives/tokens.js'; import type { Codemod, MigrationManifest } from '../lib/codemod/types.js'; import { declaredVersion, @@ -99,6 +104,7 @@ export interface MigrateResult { const IGNORED_DIRS = new Set([ 'node_modules', + 'vendor', // Composer's node_modules 'dist', 'build', 'coverage', @@ -114,7 +120,11 @@ const collectSourceFiles = (dir: string): string[] => { if (entry.isDirectory()) { if (IGNORED_DIRS.has(entry.name) || entry.name.startsWith('.')) continue; out.push(...collectSourceFiles(path.join(dir, entry.name))); - } else if (/\.tsx?$/.test(entry.name) && !entry.name.endsWith('.d.ts')) { + } else if ( + /\.(tsx?|css)$/.test(entry.name) && + !entry.name.endsWith('.d.ts') && + !entry.name.endsWith('.min.css') // minified = build artifact + ) { out.push(path.join(dir, entry.name)); } } @@ -170,15 +180,30 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { // read each candidate once; every later pass works on this snapshot const root = path.resolve(options.targetPath); + const texts = new Map(); + for (const file of collectSourceFiles(root).sort()) { + texts.set(file, readFileSync(file, 'utf8')); + } // '@marigold/' covers system (themes), components (JSX) and icons (imports) const sources = new Map(); - for (const file of collectSourceFiles(root).sort()) { - const source = readFileSync(file, 'utf8'); - if (source.includes('@marigold/')) { - sources.set(file, source); + for (const [file, text] of texts) { + if (!file.endsWith('.css') && text.includes('@marigold/')) { + sources.set(file, text); } } + // The consumer's token vocabulary: `--color-*` definitions in their CSS. + // Token warnings are suppressed per defined token — a theme built on + // Marigold's token CSS defines every added token by construction. + const definedTokens = new Set(); + for (const [file, text] of texts) { + if (!file.endsWith('.css')) continue; + for (const token of definedTokensIn(text)) definedTokens.add(token); + } + if ([...texts.values()].some(t => t.includes('@marigold/theme-rui'))) { + for (const token of manifest.tokens.added) definedTokens.add(token); + } + // pass 1: inventory of theme components across the consumer tree const inventory = new Map(); for (const [file, source] of sources) { @@ -197,6 +222,7 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { // becomes a slot object), swap before stubbing (a stubbed cva({}) must not // be mistaken for a customized slot), reports last. The JSX transforms are // independent of the theme ones; they anchor on @marigold/components. + const tokenUsage = reportTokenUsage(manifest, definedTokens); const codemods = [ restructureToSlots(manifest), swapExactClasses(manifest), @@ -208,6 +234,8 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { reportDeadKeys(manifest), reportStructure(manifest), reportJsxUsage(manifest), + reportTokenDependencies(manifest, definedTokens), + tokenUsage, ]; const reports: FileReport[] = []; for (const [file, source] of sources) { @@ -224,6 +252,23 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { } } + // pass 2.5: token references live anywhere, not only in @marigold + // importers (own token CSS, generated CSS, plain components) — text-scan + // the files the pipeline did not see. + for (const [file, text] of texts) { + if (sources.has(file)) continue; + const outcome = tokenUsage.apply(text); + if (outcome.kind !== 'skipped' && outcome.warnings.length > 0) { + reports.push({ + file, + changes: [], + warnings: outcome.warnings, + skips: [], + output: text, + }); + } + } + // pass 3: scaffold components the target version added, next to the theme // files that require them, and register them in the local barrel file. const created: string[] = []; diff --git a/packages/cli/src/lib/codemod/README.md b/packages/cli/src/lib/codemod/README.md index 00594ce199..b74fe26e36 100644 --- a/packages/cli/src/lib/codemod/README.md +++ b/packages/cli/src/lib/codemod/README.md @@ -61,8 +61,8 @@ The pipeline order matters and is fixed in `commands/migrate.ts`: dropped slot keys are reported. 4. `rename-jsx-members` / `rename-jsx-props` / `remove-jsx-props`: safe application-code edits (see below). -5. `report-dead-keys` / `report-structure` / `report-jsx-usage`: report-only - passes. +5. `report-dead-keys` / `report-structure` / `report-jsx-usage` / + `report-token-dependencies` / `report-token-usage`: report-only passes. Scaffolding (new components like `BooleanField`) runs last, per manifest entry, next to the theme file of a component that requires it, and registers @@ -106,6 +106,45 @@ last one is deliberately NOT auto-removed: the `size` prop still exists in v18 and a standalone theme may define its own size variants; only the default theme dropped `large`. +## Design tokens + +Token breakage compiles and typechecks fine and only shows up in the +browser, so the manifest's `tokens` section drives report-only checks +(`primitives/tokens.ts`). Three kinds of breakage are covered: + +- **Old tokens still referenced** (`tokens.renamed`): a plain text scan over + every `.ts`/`.tsx`/`.css` file under the target, not just Marigold + importers, for color utilities (`bg-brand`) and raw custom properties + (`var(--color-brand)`) whose token the target version renamed or removed. + The warning names the replacement (`bg-primary`) when there is one. This + bites consumers whose CSS is built on Marigold's token vocabulary. +- **New tokens required but not defined** (`tokens.added` and + `tokens.componentDependencies`): some component implementations hardcode + new tokens (in v18: the `SelectList` selection indicator uses + `selected-bold`/`disabled-surface`); these classes bypass the theme layer, + so a standalone theme without the token renders them invisibly unstyled. + Files importing such a component get a warning, and the text scan also + flags any direct use of an undefined new token. +- **Repurposed tokens** (`tokens.repurposed`): tokens that kept their name + but changed meaning, which no rename scan can see. In v18: `disabled` + flipped from background to text color (the background moved to + `disabled-surface`), `secondary` from a near-white surface to the + secondary text color, and the four status tokens plus their + `-foreground`s from solid accents to muted surfaces. Consumers who + _define_ such a token get a warning at the definition site with the + remap recipe (e.g. move your `--color-disabled` value to + `--color-disabled-surface`, take the old `-foreground` value for + `--color-disabled`), silenced once the `settledBy` token appears in + their CSS. Consumers who do not define it get warnings on old-role + utilities (`bg-disabled`) and raw `var()` reads, since Marigold's value + changed underneath them. + +Suppression makes this quiet where it should be: a token counts as defined +when the consumer's own CSS declares `--color-`, and consumers that +import `@marigold/theme-rui` get every added token by construction. Only +dangling references warn. Minified CSS (`*.min.css`) and `vendor/` are +skipped as build artifacts. + ## What is important (invariants) 1. **Never override, only add.** Consumer class strings survive @@ -145,6 +184,14 @@ Per-version work should be data entry, not transform code: styles (`container` is the default). - `structureWarnings`: hand-written texts for DOM changes that break consumer CSS selectors. Not auto-fixable, but must not go unannounced. + - `tokens`: `renamed` from the release notes' rename tables, `added` from + the `--color-*` diff of the two theme-rui token files, and + `componentDependencies` from a grep of `packages/components/src` for + utilities referencing added tokens. All three are mechanical and + codegen-able (DST-1650). `repurposed` candidates are also mechanically + detectable (a token whose utility-prefix histogram or resolved value + shifts between the two theme-rui versions), but the recipes are + hand-written: explaining a role move needs human words. - Warning links: point at the **source in this repo**, as GitHub permalinks pinned to a commit SHA or release tag (never a branch name: line numbers rot). Link the line that answers "what do I replace this @@ -182,3 +229,12 @@ general abstraction from a single example is how wrong abstractions happen. - HTML-structure changes are report-only by design: consumers with their own CSS against Marigold's DOM (e.g. generated BEM selectors) must review the named components manually. +- Repurposed tokens whose _role_ stayed the same (v17 `bg-warning` was a + solid fill, v18 `bg-warning` is a muted surface — both backgrounds) are + only detectable at definition sites and raw `var()` reads. A utility + usage like `bg-warning` is valid in both versions, so no scan can tell + whether the visual change is intended; the release notes cover that + review. +- The token scan matches a curated list of color-utility prefixes; an exotic + utility (`border-x-brand`) slips through. Report-only, so the cost of a + miss is one undetected warning, never a wrong edit. diff --git a/packages/cli/src/lib/codemod/manifests/v18.ts b/packages/cli/src/lib/codemod/manifests/v18.ts index e2913e6d8f..b53955e727 100644 --- a/packages/cli/src/lib/codemod/manifests/v18.ts +++ b/packages/cli/src/lib/codemod/manifests/v18.ts @@ -541,4 +541,147 @@ export const v18: MigrationManifest = { }, ], }, + tokens: { + // Renames: the official tables in the v18 release notes (semantic + // renames + the status-token `-muted-` drop). `disabled-foreground` and + // `hover-foreground` follow theme-rui's own replacement (see the v17->v18 + // diff of themes/theme-rui). `null` = removed without a 1:1 replacement: + // `input` split into `control`/`control-border`, and the old `secondary` + // button pair was rebuilt on the `soft` tokens. + renamed: { + brand: 'primary', + 'brand-foreground': 'primary-foreground', + 'muted-foreground': 'secondary', + focus: 'focus-highlight', + 'destructive-muted': 'destructive', + 'destructive-muted-foreground': 'destructive-foreground', + 'destructive-muted-accent': 'destructive-accent', + 'info-muted': 'info', + 'info-muted-foreground': 'info-foreground', + 'info-muted-accent': 'info-accent', + 'success-muted': 'success', + 'success-muted-foreground': 'success-foreground', + 'success-muted-accent': 'success-accent', + 'warning-muted': 'warning', + 'warning-muted-foreground': 'warning-foreground', + 'warning-muted-accent': 'warning-accent', + 'disabled-foreground': 'disabled', + 'hover-foreground': 'foreground', + input: null, + 'secondary-foreground': null, + }, + // `--color-*` diff: v17.9.1 theme.css -> v18 tokens.css (both theme-rui) + added: [ + 'access-admin-accent', + 'access-master-accent', + 'charcoal-50', + 'charcoal-100', + 'charcoal-200', + 'charcoal-300', + 'charcoal-400', + 'charcoal-500', + 'charcoal-600', + 'charcoal-700', + 'charcoal-800', + 'charcoal-900', + 'charcoal-950', + 'control', + 'control-border', + 'destructive-accent', + 'destructive-bold', + 'destructive-bold-foreground', + 'disabled-border', + 'disabled-surface', + 'focus-highlight', + 'focus-highlight-bold', + 'info-accent', + 'overlay-backdrop', + 'primary', + 'primary-foreground', + 'selected-bold', + 'selected-bold-foreground', + 'soft', + 'soft-edge', + 'soft-edge-hover', + 'soft-hover', + 'success-accent', + 'warning-accent', + ], + // Same name, new meaning. Verified against the token values in v17.9.1 + // theme.css vs v18 tokens.css: `disabled` flipped bg->text (stone-200 -> + // charcoal-400), `secondary` flipped surface->text (stone-50 -> + // charcoal-600), and the four status tokens plus their `-foreground`s + // flipped solid->muted (e.g. warning yellow-400 -> yellow-100, + // foregrounds white -> *-950). `selected` and `hover` kept their role + // (values only modernized) and are deliberately NOT listed. + repurposed: { + disabled: { + recipe: + 'v17 used it as the disabled background, v18 uses it as the disabled text color; move your value to `--color-disabled-surface`, give `--color-disabled` your old `--color-disabled-foreground` value, then review your `bg-disabled` usages', + settledBy: 'disabled-surface', + oldRolePrefixes: ['bg'], + }, + secondary: { + recipe: + 'v17 used it as a near-white surface, v18 uses it as the secondary text color (the old `muted-foreground` role); give it your old `--color-muted-foreground` value — the surface role moved to the `soft` token', + settledBy: 'soft', + oldRolePrefixes: ['bg'], + }, + destructive: { + recipe: + 'v17 was the solid accent (red-600), v18 is the muted surface (your old `destructive-muted` value); the solid accent moved to `destructive-bold`', + settledBy: 'destructive-bold', + }, + 'destructive-foreground': { + recipe: + 'v17 was white text on the solid accent, v18 is dark text on the muted surface (your old `destructive-muted-foreground` value)', + settledBy: 'destructive-bold', + }, + success: { + recipe: + 'v17 was the solid accent (green-500), v18 is the muted surface (your old `success-muted` value); rebuild solid fills with a scale color or a `-bold` theme variant', + settledBy: 'success-accent', + }, + 'success-foreground': { + recipe: + 'v17 was white text on the solid accent, v18 is dark text on the muted surface (your old `success-muted-foreground` value)', + settledBy: 'success-accent', + }, + warning: { + recipe: + 'v17 was the solid accent (yellow-400), v18 is the muted surface (your old `warning-muted` value); rebuild solid fills with a scale color or a `-bold` theme variant', + settledBy: 'warning-accent', + }, + 'warning-foreground': { + recipe: + 'v17 was white text on the solid accent, v18 is dark text on the muted surface (your old `warning-muted-foreground` value)', + settledBy: 'warning-accent', + }, + info: { + recipe: + 'v17 was the solid accent (blue-500), v18 is the muted surface (your old `info-muted` value); rebuild solid fills with a scale color or a `-bold` theme variant', + settledBy: 'info-accent', + }, + 'info-foreground': { + recipe: + 'v17 was white text on the solid accent, v18 is dark text on the muted surface (your old `info-muted-foreground` value)', + settledBy: 'info-accent', + }, + }, + // grep of packages/components/src for `added`-token utilities: only the + // SelectList selection indicator hardcodes new tokens (codegen should + // re-run that scan per release) + componentDependencies: { + SelectList: { + tokens: [ + 'selected-bold', + 'selected-bold-foreground', + 'disabled-surface', + ], + url: 'https://github.com/marigold-ui/marigold/blob/946dc9f30/packages/components/src/SelectList/SelectionIndicator.tsx#L18-L26', + }, + }, + referenceUrl: + 'https://github.com/marigold-ui/marigold/blob/946dc9f30/docs/content/releases/blog/release-2026-06-30.mdx#L86-L135', + }, }; diff --git a/packages/cli/src/lib/codemod/primitives/jsx.ts b/packages/cli/src/lib/codemod/primitives/jsx.ts index 6062df9877..ffc47e9692 100644 --- a/packages/cli/src/lib/codemod/primitives/jsx.ts +++ b/packages/cli/src/lib/codemod/primitives/jsx.ts @@ -33,7 +33,7 @@ const attrName = (attr: AnyNode): string | null => null; /** local names for the manifest's component names, only when imported */ -const localsFor = (file: AnyNode, components: Iterable) => { +export const localsFor = (file: AnyNode, components: Iterable) => { const wanted = new Set(components); const locals = new Map(); // local -> canonical name for (const imp of collectImports(file)) { diff --git a/packages/cli/src/lib/codemod/primitives/tokens.ts b/packages/cli/src/lib/codemod/primitives/tokens.ts new file mode 100644 index 0000000000..ca01b54aea --- /dev/null +++ b/packages/cli/src/lib/codemod/primitives/tokens.ts @@ -0,0 +1,235 @@ +import { codeList, parseOr } from '../engine.js'; +import type { Codemod, MigrationManifest } from '../types.js'; +import { localsFor } from './jsx.js'; + +// Report-only design-token checks, driven by the manifest's `tokens` +// section. Three kinds of breakage, all invisible to the typechecker: +// - the consumer still references tokens the target version renamed or +// removed (bites themes built on Marigold's token CSS), +// - the target version's components hardcode NEW tokens the consumer's CSS +// does not define (bites standalone themes: the classes bypass the theme +// layer entirely and silently resolve to nothing), and +// - REPURPOSED tokens that kept their name but changed meaning (v18 +// `disabled` flipped background->text) — warned at definition sites for +// consumers who define them, at old-role usages for those who don't. +// The first two are suppressed per token when the consumer's own CSS +// defines it — then the utility resolves and there is nothing to warn +// about. Repurposing inverts that logic: a definition carrying the old +// meaning is exactly the problem. + +/** token names defined (`--color-x:`) in a chunk of CSS */ +export const definedTokensIn = (css: string): string[] => + [...css.matchAll(/--color-([\w-]+)\s*:/g)].map(m => m[1]); + +// Utility prefixes that take a color token (`bg-brand`). A curated list +// keeps text scanning honest: matching bare `-brand` suffixes would also +// hit variants and unrelated identifiers. +// ponytail: covers the color utilities Marigold themes actually use; extend +// the list when a consumer surfaces one we missed. +const COLOR_PREFIXES = + 'bg|text|border|ring|inset-ring|outline|fill|stroke|decoration|divide|accent|caret|shadow|from|via|to|placeholder'; + +const escapeRegExp = (name: string): string => + name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +/** longest name first, so `warning-muted-foreground` beats `muted-foreground` */ +const alternation = (names: string[]): string => + [...names] + .sort((a, b) => b.length - a.length) + .map(escapeRegExp) + .join('|'); + +/** + * Both ways a token is referenced in consumer text: as a Tailwind color + * utility (`bg-brand`, also inside variants and with modifiers) and as the + * raw custom property (`var(--color-brand)`, `bg-(--color-brand)`). + * The trailing lookahead keeps `focus` from matching `focus-highlight`. + */ +const referencePatterns = (names: string[]): RegExp[] => { + const alt = alternation(names); + return [ + new RegExp(`\\b(?:${COLOR_PREFIXES})-(${alt})(?![\\w-])`, 'g'), + new RegExp(`--color-(${alt})(?![\\w-])`, 'g'), + ]; +}; + +const lineOf = (source: string, index: number): number => + source.slice(0, index).split('\n').length; + +const REPORTED_LINES = 5; + +interface Finding { + token: string; + lines: number[]; + count: number; + truncated: boolean; +} + +/** occurrences grouped by the exact matched reference (`bg-brand`) */ +const collectFindings = ( + source: string, + patterns: RegExp[] +): Map => { + const findings = new Map(); + for (const pattern of patterns) { + for (const match of source.matchAll(pattern)) { + const finding = findings.get(match[0]) ?? { + token: match[1], + lines: [], + count: 0, + truncated: false, + }; + finding.count += 1; + // matches arrive in source order, so a repeat is always the last line + const line = lineOf(source, match.index); + if (finding.lines.at(-1) !== line) { + if (finding.lines.length < REPORTED_LINES) finding.lines.push(line); + else finding.truncated = true; + } + findings.set(match[0], finding); + } + } + return findings; +}; + +const lineList = ({ lines, count, truncated }: Finding): string => { + const label = + lines.length === 1 + ? `line ${lines[0]}` + : `lines ${lines.join(', ')}${truncated ? ', …' : ''}`; + return count > lines.length ? `${count}×, ${label}` : label; +}; + +/** + * Text scan (no parse — works on TS, TSX and CSS alike) for token + * references that break in the target version. Suppressed per token when + * the consumer's own CSS defines it: then it is the consumer's vocabulary, + * not a dangling reference. + */ +export const reportTokenUsage = ( + manifest: MigrationManifest, + defined: ReadonlySet +): Codemod => { + const { renamed, added, repurposed, referenceUrl } = manifest.tokens; + const see = referenceUrl ? ` See ${referenceUrl}` : ''; + const gone = Object.keys(renamed).filter(t => !defined.has(t)); + const missing = added.filter(t => !defined.has(t)); + const oldPatterns = gone.length > 0 ? referencePatterns(gone) : []; + const newPatterns = missing.length > 0 ? referencePatterns(missing) : []; + + // Repurposed tokens (same name, new meaning) split by consumer archetype: + // - the consumer defines the token: their value still carries the old + // meaning, and their own usages stay self-consistent until they remap — + // warn once per definition site with the recipe, until the settling + // token shows up in their CSS. + // - the consumer does not define it (the value comes from Marigold's + // CSS): the meaning changed under every existing reference — warn on + // old-role utilities and on raw var() reads. These warnings end + // naturally when the references move to the new vocabulary. + const repurposedEntries = Object.entries(repurposed); + const definedRepurposed = repurposedEntries + .filter( + ([token, entry]) => + defined.has(token) && !(entry.settledBy && defined.has(entry.settledBy)) + ) + .map(([token]) => token); + const undefinedRepurposed = repurposedEntries.filter( + ([token]) => !defined.has(token) + ); + const definitionPatterns = + definedRepurposed.length > 0 + ? [new RegExp(`--color-(${alternation(definedRepurposed)})\\s*:`, 'g')] + : []; + const changedPatterns: RegExp[] = undefinedRepurposed + .filter(([, entry]) => entry.oldRolePrefixes?.length) + .map( + ([token, entry]) => + new RegExp( + `\\b(?:${entry.oldRolePrefixes!.join('|')})-(${escapeRegExp(token)})(?![\\w-])`, + 'g' + ) + ); + if (undefinedRepurposed.length > 0) { + changedPatterns.push( + new RegExp( + `--color-(${alternation(undefinedRepurposed.map(([t]) => t))})(?![\\w-])`, + 'g' + ) + ); + } + + return { + name: 'report-token-usage', + apply: source => { + const warnings: string[] = []; + for (const [ref, finding] of collectFindings(source, oldPatterns)) { + const replacement = renamed[finding.token]; + warnings.push( + replacement + ? `\`${ref}\` (${lineList(finding)}): the \`${finding.token}\` token was renamed in ${manifest.version} — use \`${ref.slice(0, ref.length - finding.token.length)}${replacement}\`.${see}` + : `\`${ref}\` (${lineList(finding)}): the \`${finding.token}\` token was removed in ${manifest.version} without a 1:1 replacement.${see}` + ); + } + for (const [ref, finding] of collectFindings(source, newPatterns)) { + warnings.push( + `\`${ref}\` (${lineList(finding)}): the ${manifest.version} token \`${finding.token}\` is not defined in your CSS — define \`--color-${finding.token}\`.${see}` + ); + } + for (const finding of collectFindings( + source, + definitionPatterns + ).values()) { + warnings.push( + `\`--color-${finding.token}\` (${lineList(finding)}): defined here, but its meaning changed in ${manifest.version} — ${repurposed[finding.token].recipe}.${see}` + ); + } + for (const [ref, finding] of collectFindings(source, changedPatterns)) { + warnings.push( + `\`${ref}\` (${lineList(finding)}): the \`${finding.token}\` token changed meaning in ${manifest.version} — ${repurposed[finding.token].recipe}.${see}` + ); + } + return { kind: 'unchanged', warnings }; + }, + }; +}; + +/** + * Components whose implementation hardcodes new tokens (not themeable): + * when the consumer imports one and their CSS misses the token, the + * component renders partially unstyled and nothing else will catch it. + */ +export const reportTokenDependencies = ( + manifest: MigrationManifest, + defined: ReadonlySet +): Codemod => { + const pending = Object.entries(manifest.tokens.componentDependencies) + .map(([component, dep]) => ({ + component, + missing: dep.tokens.filter(t => !defined.has(t)), + url: dep.url ?? manifest.tokens.referenceUrl, + })) + .filter(entry => entry.missing.length > 0); + + return { + name: 'report-token-dependencies', + apply: source => { + if (pending.length === 0) return { kind: 'unchanged', warnings: [] }; + return parseOr(source, file => { + const warnings: string[] = []; + const imported = new Set( + localsFor( + file, + pending.map(e => e.component) + ).values() + ); + for (const entry of pending) { + if (!imported.has(entry.component)) continue; + warnings.push( + `${entry.component}: its ${manifest.version} implementation hardcodes ${codeList(entry.missing)} — not defined in your CSS, so the component renders partially unstyled; define the \`--color-*\` variable(s).${entry.url ? ` See ${entry.url}` : ''}` + ); + } + return { kind: 'unchanged', warnings }; + }); + }, + }; +}; diff --git a/packages/cli/src/lib/codemod/tokens.test.ts b/packages/cli/src/lib/codemod/tokens.test.ts new file mode 100644 index 0000000000..bef3283339 --- /dev/null +++ b/packages/cli/src/lib/codemod/tokens.test.ts @@ -0,0 +1,224 @@ +import { v18 } from './manifests/v18.js'; +import { + definedTokensIn, + reportTokenDependencies, + reportTokenUsage, +} from './primitives/tokens.js'; +import type { CodemodOutcome } from './types.js'; + +const warningsOf = (result: CodemodOutcome): string[] => + result.kind === 'skipped' ? [] : result.warnings; + +const NONE = new Set(); + +describe('definedTokensIn', () => { + test('collects --color-* definitions from CSS', () => { + const css = `:root { + --color-brand: #f80; + --color-disabled-surface: oklch(0.9 0 0); + --spacing-input: 2rem; +} +.x { color: var(--color-brand); } +`; + expect(definedTokensIn(css)).toEqual(['brand', 'disabled-surface']); + }); +}); + +describe('report-token-usage', () => { + test('flags renamed tokens in class utilities with the new name', () => { + const source = `const styles = 'hover:bg-brand text-brand-foreground/50';`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(source)); + + expect(warnings).toEqual([ + expect.stringContaining( + '`bg-brand` (line 1): the `brand` token was renamed in v18 — use `bg-primary`' + ), + expect.stringContaining('use `text-primary-foreground`'), + ]); + }); + + test('flags raw custom-property references', () => { + const source = `.legacy { outline-color: var(--color-focus); }`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(source)); + + expect(warnings).toEqual([ + expect.stringContaining( + '`--color-focus` (line 1): the `focus` token was renamed in v18 — use `--color-focus-highlight`' + ), + ]); + }); + + test('names removed-without-replacement tokens as such', () => { + const source = `const field = 'border-input';`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(source)); + + expect(warnings).toEqual([ + expect.stringContaining( + 'the `input` token was removed in v18 without a 1:1 replacement' + ), + ]); + }); + + test('prefers the longest token name (status tokens over muted-foreground)', () => { + const source = `const text = 'text-warning-muted-foreground';`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(source)); + + expect(warnings).toEqual([ + expect.stringContaining('use `text-warning-foreground`'), + ]); + }); + + test('does not mistake variants or new names for the old focus token', () => { + const source = `const ok = 'focus:ring-2 bg-focus-highlight group-focus:underline';`; + + const warnings = warningsOf( + reportTokenUsage(v18, new Set(v18.tokens.added)).apply(source) + ); + + expect(warnings).toEqual([]); + }); + + test('flags added tokens that the consumer CSS does not define', () => { + const source = `const track = 'bg-disabled-surface';`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(source)); + + expect(warnings).toEqual([ + expect.stringContaining( + 'the v18 token `disabled-surface` is not defined in your CSS — define `--color-disabled-surface`' + ), + ]); + }); + + test('stays silent for tokens the consumer defines themselves', () => { + const source = `const styles = 'bg-brand bg-disabled-surface';`; + + const warnings = warningsOf( + reportTokenUsage(v18, new Set(['brand', 'disabled-surface'])).apply( + source + ) + ); + + expect(warnings).toEqual([]); + }); + + test('warns at the definition site of a repurposed token with the remap recipe', () => { + const css = `:root { + --color-disabled: var(--color-gray-200); + --color-disabled-foreground: var(--color-gray-400); +} +`; + + const warnings = warningsOf( + reportTokenUsage(v18, new Set(['disabled', 'disabled-foreground'])).apply( + css + ) + ); + + expect(warnings).toEqual([ + expect.stringContaining( + '`--color-disabled` (line 2): defined here, but its meaning changed in v18 — v17 used it as the disabled background' + ), + ]); + }); + + test('settles the definition warning once the moved-to token is defined', () => { + const css = `:root { + --color-disabled: var(--color-gray-400); + --color-disabled-surface: var(--color-gray-200); +} +`; + + const warnings = warningsOf( + reportTokenUsage(v18, new Set(['disabled', 'disabled-surface'])).apply( + css + ) + ); + + expect(warnings).toEqual([]); + }); + + test('flags old-role usages of an undefined repurposed token', () => { + const source = `const styles = 'bg-disabled text-disabled';`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(source)); + + expect(warnings).toEqual([ + // `bg-` is the old role and warns; `text-` is the new role and stays + expect.stringContaining( + '`bg-disabled` (line 1): the `disabled` token changed meaning in v18' + ), + ]); + }); + + test('flags raw var() reads of an undefined repurposed token', () => { + const css = `.banner { background: var(--color-warning); }`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(css)); + + expect(warnings).toEqual([ + expect.stringContaining( + '`--color-warning` (line 1): the `warning` token changed meaning in v18 — v17 was the solid accent (yellow-400)' + ), + ]); + }); + + test('groups repeated references into one warning with line numbers', () => { + const source = `a { color: var(--color-brand); } +b { background: var(--color-brand); } +c { border-color: var(--color-brand); } +`; + + const warnings = warningsOf(reportTokenUsage(v18, NONE).apply(source)); + + expect(warnings).toEqual([ + expect.stringContaining('`--color-brand` (lines 1, 2, 3)'), + ]); + }); +}); + +describe('report-token-dependencies', () => { + test('warns when an importer uses a component that hardcodes missing tokens', () => { + const source = `import { SelectList } from '@marigold/components'; +export const App = () => ; +`; + + const warnings = warningsOf( + reportTokenDependencies(v18, NONE).apply(source) + ); + + expect(warnings).toEqual([ + expect.stringContaining( + 'SelectList: its v18 implementation hardcodes `selected-bold`, `selected-bold-foreground`, `disabled-surface`' + ), + ]); + }); + + test('anchors on the import: same name from another package stays silent', () => { + const source = `import { SelectList } from './my-components'; +export const App = () => ; +`; + + const warnings = warningsOf( + reportTokenDependencies(v18, NONE).apply(source) + ); + + expect(warnings).toEqual([]); + }); + + test('stays silent when every hardcoded token is defined', () => { + const source = `import { SelectList } from '@marigold/components'; +export const App = () => ; +`; + + const warnings = warningsOf( + reportTokenDependencies(v18, new Set(v18.tokens.added)).apply(source) + ); + + expect(warnings).toEqual([]); + }); +}); diff --git a/packages/cli/src/lib/codemod/types.ts b/packages/cli/src/lib/codemod/types.ts index 6f3ef3f69d..f73c676d2c 100644 --- a/packages/cli/src/lib/codemod/types.ts +++ b/packages/cli/src/lib/codemod/types.ts @@ -108,6 +108,62 @@ export interface JsxChanges { warnings: JsxUsageWarning[]; } +export interface TokenDependency { + /** target-version tokens the component's implementation hardcodes */ + tokens: readonly string[]; + /** permalink to the hardcoded classes in the component source */ + url?: string; +} + +/** + * A token that kept its name but changed meaning (role or value) in the + * target version — the case no rename scan can see. Detected two ways: + * consumers who define the token get a definition-site warning with the + * remap recipe; consumers who don't (the value comes from Marigold's CSS) + * get warnings on old-role utilities and raw `var()` reads. + */ +export interface RepurposedToken { + /** hand-written remap explanation, phrased to work in both warnings */ + recipe: string; + /** + * Token whose presence in the consumer CSS marks the remap as done — + * definition-site warnings stop once it is defined. + */ + settledBy?: string; + /** + * Utility prefixes that indicate the OLD role (e.g. `bg` for v17 + * `disabled`). Only meaningful when the role itself flipped; a pure + * value flip (v17 `warning`) leaves this unset and is only detectable + * at definition sites and raw `var()` reads. + */ + oldRolePrefixes?: readonly string[]; +} + +/** + * Design-token (`--color-*`) changes of the target version. Report-only: + * token classes live in consumer-owned CSS and class strings, so every + * finding is a warning, never an edit. + */ +export interface TokenChanges { + /** + * Old token name -> new name, or `null` when the token was removed + * without a 1:1 replacement. Source of truth: the release notes. + */ + renamed: Record; + /** token names the target version introduced */ + added: readonly string[]; + /** tokens that kept their name but changed meaning */ + repurposed: Record; + /** + * Components whose implementation hardcodes `added` tokens (not + * themeable via the consumer theme) — a standalone theme missing the + * token renders them partially unstyled. + */ + componentDependencies: Record; + /** permalink to the official token-change documentation */ + referenceUrl?: string; +} + export interface MigrationManifest { schemaVersion: 1; version: string; @@ -124,6 +180,7 @@ export interface MigrationManifest { swaps: SwapEntry[]; structureWarnings: StructureWarning[]; jsx: JsxChanges; + tokens: TokenChanges; /** * Pinned base URL of the default theme's component style sources * (`/.styles.ts`), used in reports as the reference for From 3cae693004c00767dfd329ead64d17c2c9c6d6be Mon Sep 17 00:00:00 2001 From: aromko Date: Fri, 24 Jul 2026 14:41:28 +0200 Subject: [PATCH 08/11] feat(DST-1543): pre-analyze migrations and let the user select changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interactive `marigold migrate` runs now analyze the target first (a dry run in memory) and offer the changes that actually fire — name, description, change and file counts — as a multiselect. Everything is preselected, so Enter still applies the full migration; deselected entries are skipped. Report-only passes are not selectable and always run: the warnings are the safety net, and a partial migration needs them more, not less. `runMigrate` returns the structured `summary` and takes `only` (edit codemod names plus scaffold-components), so scripts and CI get the same subset non-interactively via `--only a,b` — unknown names fail listing the valid ones. A full run without a selection is byte-identical to before. --- .changeset/migrate-codemods.md | 2 +- packages/cli/src/bin/marigold.ts | 41 ++++++++++- packages/cli/src/commands/migrate.test.ts | 72 +++++++++++++++++++ packages/cli/src/commands/migrate.ts | 87 +++++++++++++++++++++-- packages/cli/src/lib/codemod/README.md | 10 ++- packages/cli/src/lib/commands-spec.ts | 5 +- 6 files changed, 208 insertions(+), 9 deletions(-) diff --git a/.changeset/migrate-codemods.md b/.changeset/migrate-codemods.md index 0f9514d888..9eefed96ec 100644 --- a/.changeset/migrate-codemods.md +++ b/.changeset/migrate-codemods.md @@ -2,4 +2,4 @@ '@marigold/cli': minor --- -feat(DST-1543): add `marigold migrate ` codemods for breaking Marigold releases. The v18 migration restructures theme files to the new slot shapes (never overriding consumer classes), swaps exact-baseline layout classes with a token diff report, scaffolds missing theme components, applies safe application-code renames (icon imports per the official mapping, `Tabs.TabPanel`/`SelectList.Item`, `Inset` spacing props, `TextField` min/max), and reports everything that needs a human decision with pinned source links. The report also covers design-token breakage that no typecheck can see: renamed/removed tokens still referenced, new tokens components require but the consumer CSS does not define, and repurposed tokens that kept their name but changed meaning (with a remap recipe at the definition site). Run `npx marigold migrate v18 --dry-run` first. +feat(DST-1543): add `marigold migrate ` codemods for breaking Marigold releases. The v18 migration restructures theme files to the new slot shapes (never overriding consumer classes), swaps exact-baseline layout classes with a token diff report, scaffolds missing theme components, applies safe application-code renames (icon imports per the official mapping, `Tabs.TabPanel`/`SelectList.Item`, `Inset` spacing props, `TextField` min/max), and reports everything that needs a human decision with pinned source links. The report also covers design-token breakage that no typecheck can see: renamed/removed tokens still referenced, new tokens components require but the consumer CSS does not define, and repurposed tokens that kept their name but changed meaning (with a remap recipe at the definition site). Interactive runs pre-analyze the target and offer the fired changes as a multiselect (Enter applies everything; `--only ` selects non-interactively). Run `npx marigold migrate v18 --dry-run` first. diff --git a/packages/cli/src/bin/marigold.ts b/packages/cli/src/bin/marigold.ts index 512708aa6b..85d0290fad 100644 --- a/packages/cli/src/bin/marigold.ts +++ b/packages/cli/src/bin/marigold.ts @@ -109,6 +109,9 @@ ${pc.bold('Migrate options:')} and the proposed migration confirmed interactively [path] Directory to migrate (default: current directory) --dry-run Report what would change without writing files + --only Apply only these changes (comma-separated codemod + names from the pre-analysis); skips the interactive + selection. Warnings always run. ${pc.bold('Environment:')} MARIGOLD_DOCS_URL Override docs site base URL @@ -216,6 +219,7 @@ const parseMigrateCommand = (argv: string[]) => allowPositionals: true, options: { 'dry-run': { type: 'boolean', default: false }, + only: { type: 'string' }, }, }); @@ -452,7 +456,9 @@ export const main = async ( }; if (positionals.length > (explicitVersion ? 2 : 1)) { - fail('Usage: marigold migrate [version] [path] [--dry-run]'); + fail( + 'Usage: marigold migrate [version] [path] [--dry-run] [--only ]' + ); } // Lazy-load: migrate pulls in @babel/parser and magic-string, which we @@ -500,11 +506,42 @@ export const main = async ( versions = detected.versions; } + const only = values.only + ?.split(',') + .map(s => s.trim()) + .filter(Boolean); + for (const version of versions) { - const result = await runMigrate({ + // pre-analysis: dry-run in memory, offer the fired changes for + // selection. Skipped for explicit --only / --dry-run / non-TTY runs. + let selected = only; + if (!selected && !values['dry-run'] && process.stdout.isTTY) { + const analysis = runMigrate({ version, targetPath, dryRun: true }); + if (analysis.summary.length > 0) { + const { multiselect, isCancel } = await import('@clack/prompts'); + const chosen = await multiselect({ + message: `The ${version} migration fires these changes — deselect what you want to skip (warnings always run):`, + options: analysis.summary.map(s => ({ + value: s.name, + label: s.name, + hint: `${s.description} — ${s.changes} change(s) in ${s.files} file(s)`, + })), + initialValues: analysis.summary.map(s => s.name), + required: false, + }); + if (isCancel(chosen)) { + writeOutput('Aborted — nothing changed.'); + return 130; + } + selected = chosen as string[]; + } + } + + const result = runMigrate({ version, targetPath, dryRun: values['dry-run'], + only: selected, }); writeOutput(result.output); } diff --git a/packages/cli/src/commands/migrate.test.ts b/packages/cli/src/commands/migrate.test.ts index d8044cb182..5510531851 100644 --- a/packages/cli/src/commands/migrate.test.ts +++ b/packages/cli/src/commands/migrate.test.ts @@ -247,6 +247,78 @@ export const Profile = () => ( expect(output).toContain('Tooltip[open]'); }); + test('returns a pre-analysis summary of the changes that fired', () => { + const root = setupFixture(); + + const { summary } = runMigrate({ + version: 'v18', + targetPath: root, + dryRun: true, + }); + + expect(summary).toEqual([ + { + name: 'restructure-to-slots', + description: expect.stringContaining('slot objects'), + files: 1, + changes: expect.any(Number), + }, + { + name: 'swap-exact-classes', + description: expect.stringContaining('baseline'), + files: 1, + changes: 1, + }, + // stub-missing-slots does not fire: the fixture Switch already has + // every v18 slot and the Card restructure stubs its own new slots + { + name: 'scaffold-components', + description: expect.stringContaining('create theme styles'), + files: 2, // BooleanField.styles.ts + the barrel index export + changes: 2, + }, + ]); + }); + + test('only applies the selected changes; everything else stays untouched', () => { + const root = setupFixture(); + const components = path.join(root, 'theme', 'components'); + const cardBefore = readFileSync( + path.join(components, 'Card.styles.ts'), + 'utf8' + ); + + runMigrate({ + version: 'v18', + targetPath: root, + dryRun: false, + only: ['swap-exact-classes'], + }); + + expect(readFileSync(path.join(components, 'Card.styles.ts'), 'utf8')).toBe( + cardBefore + ); + expect( + readFileSync(path.join(components, 'Switch.styles.ts'), 'utf8') + ).toContain(`'grid gap-x-2 items-center'`); + expect(existsSync(path.join(components, 'BooleanField.styles.ts'))).toBe( + false + ); + }); + + test('rejects unknown names in only', () => { + const root = setupFixture(); + + expect(() => + runMigrate({ + version: 'v18', + targetPath: root, + dryRun: true, + only: ['swap-exact-clases'], + }) + ).toThrow(/Unknown change\(s\): swap-exact-clases/); + }); + test('reports token findings in CSS files and component internals', () => { const root = setupFixture(); writeFileSync( diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index 49de6a75c4..8ae365df9d 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -96,12 +96,43 @@ export interface MigrateOptions { version: string; targetPath: string; dryRun: boolean; + /** + * Names of the changes to apply (codemod names plus 'scaffold-components'). + * Report-only passes always run. Omitted: everything runs. + */ + only?: readonly string[]; +} + +/** one selectable change of a migration, with its impact on the target */ +export interface CodemodSummary { + name: string; + description: string; + files: number; + changes: number; } export interface MigrateResult { output: string; + /** changes that fired on this target, in pipeline order — the + * pre-analysis a caller can offer for selection via `only` */ + summary: CodemodSummary[]; } +const SCAFFOLD = 'scaffold-components'; + +// user-facing one-liners for the selectable changes (report-only passes are +// not selectable: they are the safety net and always run) +const DESCRIPTIONS: Record = { + 'restructure-to-slots': 'move single-style theme components to slot objects', + 'swap-exact-classes': 'swap unchanged baseline styles to the new baseline', + 'stub-missing-slots': 'stub new theme slots with empty cva()', + 'rename-jsx-members': 'rename compound components (e.g. Tabs.TabPanel)', + 'rename-jsx-props': 'rename component props (e.g. Inset space to p)', + 'remove-jsx-props': 'remove props the target version dropped', + 'rename-imports': 'rename moved exports (e.g. the icon renames)', + [SCAFFOLD]: 'create theme styles for components the target version requires', +}; + const IGNORED_DIRS = new Set([ 'node_modules', 'vendor', // Composer's node_modules @@ -137,6 +168,8 @@ interface FileReport { warnings: string[]; skips: string[]; output: string; + /** change count per codemod name, for the pre-analysis summary */ + perCodemod: Record; } const edited = (report: FileReport): boolean => report.changes.length > 0; @@ -152,6 +185,7 @@ const applyPipeline = ( warnings: [], skips: [], output: source, + perCodemod: {}, }; for (const codemod of codemods) { // each transform re-parses its input when chained; babel parse is fast @@ -165,6 +199,8 @@ const applyPipeline = ( if (outcome.kind === 'edited') { report.output = outcome.output; report.changes.push(...outcome.changes); + report.perCodemod[codemod.name] = + (report.perCodemod[codemod.name] ?? 0) + outcome.changes.length; } } return report; @@ -223,7 +259,7 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { // be mistaken for a customized slot), reports last. The JSX transforms are // independent of the theme ones; they anchor on @marigold/components. const tokenUsage = reportTokenUsage(manifest, definedTokens); - const codemods = [ + const editCodemods = [ restructureToSlots(manifest), swapExactClasses(manifest), stubMissingSlots(manifest), @@ -231,6 +267,22 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { renameJsxProps(manifest), removeJsxProps(manifest), renameImports(manifest), + ]; + if (options.only) { + const known = new Set([...editCodemods.map(c => c.name), SCAFFOLD]); + const unknown = options.only.filter(name => !known.has(name)); + if (unknown.length > 0) { + throw new Error( + `Unknown change(s): ${unknown.join(', ')} (available: ${[...known].join(', ')})` + ); + } + } + const active = options.only + ? editCodemods.filter(c => options.only!.includes(c.name)) + : editCodemods; + const scaffoldEnabled = !options.only || options.only.includes(SCAFFOLD); + const codemods = [ + ...active, reportDeadKeys(manifest), reportStructure(manifest), reportJsxUsage(manifest), @@ -265,6 +317,7 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { warnings: outcome.warnings, skips: [], output: text, + perCodemod: {}, }); } } @@ -273,7 +326,7 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { // files that require them, and register them in the local barrel file. const created: string[] = []; const scaffoldWarnings: string[] = []; - for (const entry of manifest.scaffolds) { + for (const entry of scaffoldEnabled ? manifest.scaffolds : []) { if (inventory.has(entry.name)) continue; const host = entry.requiredBy.map(c => inventory.get(c)).find(Boolean); if (!host) continue; @@ -317,6 +370,7 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { warnings: [], skips: [], output: outcome.output, + perCodemod: { [SCAFFOLD]: outcome.changes.length }, }); } } else { @@ -326,6 +380,31 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { } } + // pre-analysis summary: which changes fired, aggregated across files, in + // pipeline order (scaffolds last) — offered to the caller for selection + const totals = new Map(); + for (const report of reports) { + for (const [name, changes] of Object.entries(report.perCodemod)) { + const total = totals.get(name) ?? { files: 0, changes: 0 }; + total.files += 1; + total.changes += changes; + totals.set(name, total); + } + } + if (created.length > 0) { + const total = totals.get(SCAFFOLD) ?? { files: 0, changes: 0 }; + total.files += created.length; + total.changes += created.length; + totals.set(SCAFFOLD, total); + } + const summary: CodemodSummary[] = [...editCodemods.map(c => c.name), SCAFFOLD] + .filter(name => totals.has(name)) + .map(name => ({ + name, + description: DESCRIPTIONS[name] ?? '', + ...totals.get(name)!, + })); + // render report. picocolors is TTY-aware: piped / test / agent output stays // byte-for-byte plain, only interactive terminals gain color. // `code` spans render cyan (same convention as doctor), URLs cyan+underline. @@ -357,7 +436,7 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { lines.push( `No files importing @marigold/system or @marigold/components found under ${root}.` ); - return { output: lines.join('\n') }; + return { output: lines.join('\n'), summary: [] }; } for (const report of reports) { lines.push(pc.bold(path.relative(root, report.file))); @@ -394,5 +473,5 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { ) ); } - return { output: lines.join('\n') }; + return { output: lines.join('\n'), summary }; }; diff --git a/packages/cli/src/lib/codemod/README.md b/packages/cli/src/lib/codemod/README.md index b74fe26e36..656987bda6 100644 --- a/packages/cli/src/lib/codemod/README.md +++ b/packages/cli/src/lib/codemod/README.md @@ -8,7 +8,7 @@ touching for a new release; everything release-specific lives in a manifest. ## Usage ```sh -npx marigold migrate [version] [path] [--dry-run] +npx marigold migrate [version] [path] [--dry-run] [--only ] ``` - `version` is optional: when omitted, the installed @@ -18,6 +18,14 @@ npx marigold migrate [version] [path] [--dry-run] Non-interactive sessions must pass the version explicitly. - `path` defaults to the current directory; point it at the consumer repo (or its theme directory) you want to migrate. +- **Pre-analysis and selection**: before writing anything, an interactive + run analyzes the target (a dry run in memory) and lists the changes that + actually fire — name, description, change and file counts — as a + multiselect. Everything is preselected, so Enter applies the full + migration; deselect entries to skip them. Report-only passes (warnings) + are not selectable and always run. `--only +swap-exact-classes,scaffold-components` applies the same subset + non-interactively (and skips the prompt). - **Always run `--dry-run` first** and read the report. - After a real run, run the consumer's typechecker: the slot Records in `@marigold/system`'s `Theme` type are exhaustive, so the typecheck is the diff --git a/packages/cli/src/lib/commands-spec.ts b/packages/cli/src/lib/commands-spec.ts index db27c49ece..432c69453f 100644 --- a/packages/cli/src/lib/commands-spec.ts +++ b/packages/cli/src/lib/commands-spec.ts @@ -107,7 +107,10 @@ export const SUBCOMMANDS: readonly SubcommandSpec[] = [ }, { name: 'migrate', - flags: [{ name: '--dry-run', type: 'boolean' }], + flags: [ + { name: '--dry-run', type: 'boolean' }, + { name: '--only', type: 'string' }, + ], }, { name: 'telemetry', From 49b76b79aae664735fd09b9bb866b0c406b6e287 Mon Sep 17 00:00:00 2001 From: aromko Date: Fri, 24 Jul 2026 16:15:34 +0200 Subject: [PATCH 09/11] fix(DST-1543): never break re-exports or JSX members in rename-imports Review findings on #5675, all verified against the branch: - a bare `export { Pickup }` forced through a direct rename left the specifier pointing at nothing; re-exported names now force the alias fallback, and `export { Pickup } from '@marigold/icons'` is rewritten to `Store as Pickup`, keeping the public name - `` properties were renamed like usages; JSXMemberExpression properties are name positions now - the scan includes .js/.jsx (and skips .min.js), so JS consumers get the codemods and token warnings too - new report-only pass warns on namespace imports of affected packages, which every anchor silently misses - a file-level parse error is reported once, not per codemod - dropped the stray awaits on the synchronous runMigrate in tests --- packages/cli/src/commands/migrate.test.ts | 55 +++++++-- packages/cli/src/commands/migrate.ts | 11 +- packages/cli/src/lib/codemod/README.md | 22 ++-- packages/cli/src/lib/codemod/jsx.test.ts | 75 +++++++++++++ .../cli/src/lib/codemod/primitives/jsx.ts | 106 +++++++++++++++++- 5 files changed, 243 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/commands/migrate.test.ts b/packages/cli/src/commands/migrate.test.ts index 5510531851..f7fbb02ebb 100644 --- a/packages/cli/src/commands/migrate.test.ts +++ b/packages/cli/src/commands/migrate.test.ts @@ -120,12 +120,12 @@ describe('runMigrate', () => { ).toThrow(/Unknown migration 'v99'/); }); - test('dry run reports changes without writing anything', async () => { + test('dry run reports changes without writing anything', () => { const root = setupFixture(); const cardPath = path.join(root, 'theme', 'components', 'Card.styles.ts'); const before = readFileSync(cardPath, 'utf8'); - const { output } = await runMigrate({ + const { output } = runMigrate({ version: 'v18', targetPath: root, dryRun: true, @@ -143,11 +143,11 @@ describe('runMigrate', () => { ).toBe(false); }); - test('applies edits, scaffolds required components, updates the barrel', async () => { + test('applies edits, scaffolds required components, updates the barrel', () => { const root = setupFixture(); const components = path.join(root, 'theme', 'components'); - const { output } = await runMigrate({ + const { output } = runMigrate({ version: 'v18', targetPath: root, dryRun: false, @@ -176,15 +176,15 @@ describe('runMigrate', () => { expect(output).toContain('Run your typechecker'); }); - test('is idempotent: a second run changes nothing', async () => { + test('is idempotent: a second run changes nothing', () => { const root = setupFixture(); - await runMigrate({ version: 'v18', targetPath: root, dryRun: false }); + runMigrate({ version: 'v18', targetPath: root, dryRun: false }); const components = path.join(root, 'theme', 'components'); const snapshot = ['Card.styles.ts', 'Switch.styles.ts', 'index.ts'].map(f => readFileSync(path.join(components, f), 'utf8') ); - const { output } = await runMigrate({ + const { output } = runMigrate({ version: 'v18', targetPath: root, dryRun: false, @@ -198,11 +198,11 @@ describe('runMigrate', () => { expect(output).toContain('Edited 0 file(s)'); }); - test('reports when no Marigold imports are found', async () => { + test('reports when no Marigold imports are found', () => { const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-migrate-')); writeFileSync(path.join(root, 'app.ts'), `export const x = 1;\n`); - const { output } = await runMigrate({ + const { output } = runMigrate({ version: 'v18', targetPath: root, dryRun: true, @@ -211,7 +211,7 @@ describe('runMigrate', () => { expect(output).toContain('No files importing'); }); - test('applies safe application-code codemods alongside theme codemods', async () => { + test('applies safe application-code codemods alongside theme codemods', () => { const root = setupFixture(); const appFile = path.join(root, 'app', 'Profile.tsx'); mkdirSync(path.dirname(appFile), { recursive: true }); @@ -232,7 +232,7 @@ export const Profile = () => ( ` ); - const { output } = await runMigrate({ + const { output } = runMigrate({ version: 'v18', targetPath: root, dryRun: false, @@ -247,6 +247,39 @@ export const Profile = () => ( expect(output).toContain('Tooltip[open]'); }); + test('scans .jsx and .js application files too', () => { + const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-migrate-')); + const appFile = path.join(root, 'App.jsx'); + writeFileSync( + appFile, + `import { Pickup } from '@marigold/icons'; +export const App = () => ; +` + ); + + runMigrate({ version: 'v18', targetPath: root, dryRun: false }); + + expect(readFileSync(appFile, 'utf8')).toContain(''); + }); + + test('reports a file-level parse error once, not per codemod', () => { + const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-migrate-')); + writeFileSync( + path.join(root, 'broken.tsx'), + `import { Inset } from '@marigold/components'; +const = broken(; +` + ); + + const { output } = runMigrate({ + version: 'v18', + targetPath: root, + dryRun: true, + }); + + expect(output.match(/parse error/g)).toHaveLength(1); + }); + test('returns a pre-analysis summary of the changes that fired', () => { const root = setupFixture(); diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index 8ae365df9d..035d15607c 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -14,6 +14,7 @@ import { renameJsxMembers, renameJsxProps, reportJsxUsage, + reportNamespaceImports, } from '../lib/codemod/primitives/jsx.js'; import { reportDeadKeys, @@ -152,9 +153,9 @@ const collectSourceFiles = (dir: string): string[] => { if (IGNORED_DIRS.has(entry.name) || entry.name.startsWith('.')) continue; out.push(...collectSourceFiles(path.join(dir, entry.name))); } else if ( - /\.(tsx?|css)$/.test(entry.name) && + /\.(tsx?|jsx?|css)$/.test(entry.name) && !entry.name.endsWith('.d.ts') && - !entry.name.endsWith('.min.css') // minified = build artifact + !/\.min\.(css|js)$/.test(entry.name) // minified = build artifact ) { out.push(path.join(dir, entry.name)); } @@ -192,7 +193,10 @@ const applyPipeline = ( // and shared-AST plumbing is not worth it const outcome = codemod.apply(report.output); if (outcome.kind === 'skipped') { - report.skips.push(`${codemod.name}: ${outcome.reason}`); + // one parse error hits every parsing codemod — report it once + if (!report.skips.some(skip => skip.endsWith(outcome.reason))) { + report.skips.push(`${codemod.name}: ${outcome.reason}`); + } continue; } report.warnings.push(...outcome.warnings); @@ -286,6 +290,7 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { reportDeadKeys(manifest), reportStructure(manifest), reportJsxUsage(manifest), + reportNamespaceImports(manifest), reportTokenDependencies(manifest, definedTokens), tokenUsage, ]; diff --git a/packages/cli/src/lib/codemod/README.md b/packages/cli/src/lib/codemod/README.md index 656987bda6..4cbb89708b 100644 --- a/packages/cli/src/lib/codemod/README.md +++ b/packages/cli/src/lib/codemod/README.md @@ -70,7 +70,8 @@ The pipeline order matters and is fixed in `commands/migrate.ts`: 4. `rename-jsx-members` / `rename-jsx-props` / `remove-jsx-props`: safe application-code edits (see below). 5. `report-dead-keys` / `report-structure` / `report-jsx-usage` / - `report-token-dependencies` / `report-token-usage`: report-only passes. + `report-namespace-imports` / `report-token-dependencies` / + `report-token-usage`: report-only passes. Scaffolding (new components like `BooleanField`) runs last, per manifest entry, next to the theme file of a component that requires it, and registers @@ -92,7 +93,9 @@ The anchor here is the **import**: a JSX element only counts as a Marigold component when its (possibly aliased) name is imported from `@marigold/components`. Only explicit JSX attributes are touched; props hidden behind spreads are unreachable by design and left to the consumer's -typecheck. +typecheck. Namespace imports (`import * as X`) of migration-affected +packages cannot be followed by any codemod and raise a warning instead +(`report-namespace-imports`). **Renamed exports** (the v18 icon migration, driven by the manifest's `jsx.importRenames` and the official mapping table in @@ -100,10 +103,13 @@ typecheck. every usage (`` becomes ``) — when the file provably allows it. Member accesses, object keys, and attribute names with the same spelling are never touched. When a direct rename is not provably safe (the -old name is shadowed or used as a shorthand property, or the new name -already exists in the file), the specifier falls back to the -release-notes-blessed alias form (`Store as Pickup`), which keeps every -call site valid, and the report names the reason. Re-running is a no-op +old name is shadowed or used as a shorthand property, re-exported from the +file, or the new name already exists in the file), the specifier falls back +to the release-notes-blessed alias form (`Store as Pickup`), which keeps +every call site — including `export { Pickup }` — valid, and the report +names the reason. Direct re-exports (`export { Pickup } from +'@marigold/icons'`) are rewritten to `export { Store as Pickup }`, so the +public name downstream consumers import survives. Re-running is a no-op either way. Everything that needs a structural JSX move or a design decision is a @@ -121,8 +127,8 @@ browser, so the manifest's `tokens` section drives report-only checks (`primitives/tokens.ts`). Three kinds of breakage are covered: - **Old tokens still referenced** (`tokens.renamed`): a plain text scan over - every `.ts`/`.tsx`/`.css` file under the target, not just Marigold - importers, for color utilities (`bg-brand`) and raw custom properties + every `.ts`/`.tsx`/`.js`/`.jsx`/`.css` file under the target, not just + Marigold importers, for color utilities (`bg-brand`) and raw custom properties (`var(--color-brand)`) whose token the target version renamed or removed. The warning names the replacement (`bg-primary`) when there is one. This bites consumers whose CSS is built on Marigold's token vocabulary. diff --git a/packages/cli/src/lib/codemod/jsx.test.ts b/packages/cli/src/lib/codemod/jsx.test.ts index e76684eb0b..5dabd0fd60 100644 --- a/packages/cli/src/lib/codemod/jsx.test.ts +++ b/packages/cli/src/lib/codemod/jsx.test.ts @@ -5,6 +5,7 @@ import { renameJsxMembers, renameJsxProps, reportJsxUsage, + reportNamespaceImports, } from './primitives/jsx.js'; import { assertEdited } from './test-helpers.js'; import type { CodemodOutcome } from './types.js'; @@ -151,6 +152,55 @@ export const x = config.Pickup; expect(result.output).toContain(''); }); + test('does not touch JSX member properties with the old name', () => { + const source = `import { Pickup } from '@marigold/icons'; +const Icons = { Pickup: () => null }; +export const App = () => ( +
+ + +
+); +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain(`import { Store } from '@marigold/icons';`); + expect(result.output).toContain(''); + expect(result.output).toContain(''); + }); + + test('falls back to an alias when the old name is re-exported', () => { + const source = `import { Pickup } from '@marigold/icons'; +export { Pickup }; +export const App = () => ; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + `import { Store as Pickup } from '@marigold/icons';` + ); + expect(result.output).toContain('export { Pickup };'); + expect(result.output).toContain(''); + expect(result.changes[0]).toContain('re-exported from this file'); + }); + + test('rewrites re-exports from the package, keeping the public name', () => { + const source = `export { Pickup } from '@marigold/icons'; +export { Email as MailIcon } from '@marigold/icons'; +`; + const result = renameImports(v18).apply(source); + + assertEdited(result); + expect(result.output).toContain( + `export { Store as Pickup } from '@marigold/icons';` + ); + expect(result.output).toContain( + `export { Mail as MailIcon } from '@marigold/icons';` + ); + }); + test('falls back to an alias when the new name already exists in the file', () => { const source = `import { Pickup } from '@marigold/icons'; import { Store } from './my-store'; @@ -221,9 +271,34 @@ export const App = () => ; `; const aliased = `import { Store as Pickup } from '@marigold/icons'; export const App = () => ; +`; + const reexported = `export { Store as Pickup } from '@marigold/icons'; `; expect(renameImports(v18).apply(direct).kind).toBe('unchanged'); expect(renameImports(v18).apply(aliased).kind).toBe('unchanged'); + expect(renameImports(v18).apply(reexported).kind).toBe('unchanged'); + }); +}); + +describe('report-namespace-imports', () => { + test('warns on namespace imports of migration-affected packages', () => { + const source = `import * as Icons from '@marigold/icons'; +export const App = () => ; +`; + const warnings = warningsOf(reportNamespaceImports(v18).apply(source)); + + expect(warnings).toEqual([ + expect.stringContaining( + "`import * as Icons from '@marigold/icons'`: the codemods cannot follow namespace imports" + ), + ]); + }); + + test('stays silent for unaffected packages', () => { + const source = `import * as path from 'node:path'; +export const join = path.join; +`; + expect(warningsOf(reportNamespaceImports(v18).apply(source))).toEqual([]); }); }); diff --git a/packages/cli/src/lib/codemod/primitives/jsx.ts b/packages/cli/src/lib/codemod/primitives/jsx.ts index ffc47e9692..97ea09e84a 100644 --- a/packages/cli/src/lib/codemod/primitives/jsx.ts +++ b/packages/cli/src/lib/codemod/primitives/jsx.ts @@ -5,7 +5,7 @@ import { jsxOpeningName, walk, } from '../../tsx-ast.js'; -import { MARIGOLD_COMPONENTS } from '../anchor.js'; +import { MARIGOLD_COMPONENTS, MARIGOLD_SYSTEM } from '../anchor.js'; import { parseOr } from '../engine.js'; import type { Codemod, CodemodOutcome, MigrationManifest } from '../types.js'; @@ -120,6 +120,10 @@ const isNamePosition = (n: AnyNode, parent: AnyNode | null): boolean => { case 'MemberExpression': case 'OptionalMemberExpression': return parent.property === n && !computed; + case 'JSXMemberExpression': + // `` — the property names a key on `Icons`, not the + // import binding; only the object side references a binding + return parent.property === n; case 'ObjectProperty': case 'ObjectMethod': case 'ClassProperty': @@ -210,7 +214,26 @@ export const renameImports = (manifest: MigrationManifest): Codemod => { const usages = new Map(); // renameable references const shadowed = new Set(); const namesInFile = new Set(); + const reexported = new Set(); // `export { Pickup }` locals + const exportsFrom: AnyNode[] = []; // `export { ... } from ''` walk(file, (n, parent) => { + if (n.type === 'ExportNamedDeclaration') { + if (n.source) { + exportsFrom.push(n); + } else { + // a bare re-export references the local binding by name — a + // direct rename would leave it pointing at nothing + for (const spec of (n.specifiers as AnyNode[] | undefined) ?? + []) { + const local = (spec.local as { name?: string } | undefined) + ?.name; + if (spec.type === 'ExportSpecifier' && local) { + reexported.add(local); + } + } + } + return; + } if (n.type !== 'Identifier' && n.type !== 'JSXIdentifier') return; const name = (n as { name?: string }).name; if (!name) return; @@ -263,9 +286,11 @@ export const renameImports = (manifest: MigrationManifest): Codemod => { const aliasReason = namesInFile.has(entry.to) ? `\`${entry.to}\` is already used in this file` - : shadowed.has(entry.from) - ? `\`${entry.from}\` is re-declared or used as a shorthand property here` - : null; + : reexported.has(entry.from) + ? `\`${entry.from}\` is re-exported from this file` + : shadowed.has(entry.from) + ? `\`${entry.from}\` is re-declared or used as a shorthand property here` + : null; if (aliasReason) { s.overwrite( @@ -292,12 +317,85 @@ export const renameImports = (manifest: MigrationManifest): Codemod => { ); } } + + // `export { Pickup } from '@marigold/icons'`: the specifier reads + // the package's export directly, so the local walk above never sees + // it. Rewrite to `Store as Pickup` — the public name downstream + // consumers import must survive. + for (const decl of exportsFrom) { + const src = (decl.source as { value?: string } | undefined)?.value; + const renames = src ? byPackage.get(src) : undefined; + if (!renames) continue; + for (const spec of (decl.specifiers as AnyNode[] | undefined) ?? []) { + if (spec.type !== 'ExportSpecifier') continue; + const local = spec.local as AnyNode; // name in the source package + const localName = (local as { name?: string }).name; + const entry = localName ? renames.get(localName) : undefined; + if (!entry) continue; + const note = entry.note ? ` (${entry.note})` : ''; + const exportedName = ( + spec.exported as { name?: string } | undefined + )?.name; + if (exportedName === localName) { + s.overwrite( + spec.start as number, + spec.end as number, + `${entry.to} as ${entry.from}` + ); + changes.push( + `${src}: \`${entry.from}\` is now \`${entry.to}\` — re-exported as \`${entry.to} as ${entry.from}\` to keep the public name${note}` + ); + } else { + // already aliased (`export { Pickup as Foo }`): only the + // source-side name changes + s.overwrite(local.start as number, local.end as number, entry.to); + changes.push( + `${src}: \`${entry.from}\` is now \`${entry.to}\` (public name \`${exportedName}\` kept)${note}` + ); + } + } + } if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; return { kind: 'edited', output: s.toString(), changes, warnings: [] }; }), }; }; +/** + * Report-only: namespace imports (`import * as X`) of packages this + * migration changes. No codemod can follow `X.*` usages — theme anchoring, + * prop renames and import renames all silently miss them — so the file + * needs a manual review against the release notes. + */ +export const reportNamespaceImports = ( + manifest: MigrationManifest +): Codemod => { + const affected = new Set([ + MARIGOLD_COMPONENTS, + MARIGOLD_SYSTEM, + ...manifest.jsx.importRenames.map(e => e.package), + ]); + return { + name: 'report-namespace-imports', + apply: source => + parseOr(source, file => { + const warnings: string[] = []; + for (const imp of collectImports(file)) { + const src = (imp.source as { value?: string } | undefined)?.value; + if (!src || !affected.has(src)) continue; + for (const spec of (imp.specifiers as AnyNode[] | undefined) ?? []) { + if (spec.type !== 'ImportNamespaceSpecifier') continue; + const local = (spec.local as { name?: string } | undefined)?.name; + warnings.push( + `\`import * as ${local} from '${src}'\`: the codemods cannot follow namespace imports — review the \`${local}.*\` usages against the ${manifest.version} changes manually` + ); + } + } + return { kind: 'unchanged', warnings }; + }), + }; +}; + /** Renames compound members, e.g. Tabs.TabPanel to Tabs.Panel. */ export const renameJsxMembers = (manifest: MigrationManifest): Codemod => ({ name: 'rename-jsx-members', From afec448092b55db0869ac247f8e8258750a72afb Mon Sep 17 00:00:00 2001 From: aromko Date: Mon, 27 Jul 2026 12:46:54 +0200 Subject: [PATCH 10/11] fix(DST-1543): address review feedback on the migrate codemods Manifest drift (the blocking findings): - add `ErrorState` to the v18 slots (#5666) and `Sidebar`'s 9 rail slots (#5654). A themed ErrorState no longer gets a false "not a themeable component" warning, and the rail slots are stubbed like every other new slot. - add `manifests/theme-drift.test.ts`: parses the live `Theme` type in @marigold/system and asserts slot parity with the manifest. Verified to catch both drift kinds (missing component, missing slot). This is the re-check trigger until DST-1650's codegen lands; the README names it as a step when authoring a new manifest. Correctness and consistency: - stubs honor an aliased cva import: `themeCodemod` resolves the local name once per file and threads it into `stubSlotLine`, so a consumer importing `cva as c` gets `c({})` instead of a reference to an undefined `cva`. - the interactivity gate matches init's: stdout AND stdin must be a TTY, so a piped stdin can no longer hang on a prompt. - `marigold migrate 18.1 ./src` now says "did you mean v18?" instead of dying on a filesystem error. Checked before the positional-count validation, which would otherwise reject it as "too many paths". - reword the `Print` -> `Printer` manifest note: the mapping table gained the row in this PR, so it is no longer "missing" from it. Structure: - move the ~110-line interactive flow out of bin/marigold.ts into `runMigrateCommand` in commands/migrate.ts, matching how init keeps its prompt flow in the command and the router thin. The flow had no coverage; it now has tests for detection, the non-TTY refusal, the declined confirm (130) and the multiselect subset. - `description` is a required field on `Codemod`, replacing the runner-side name -> description map whose `?? ''` degraded silently on a typo. - fold report.ts's hand-rolled `analyze` into `themeCodemod`, which already returns `unchanged` carrying warnings when a visit pushes no changes. - add a `jsxCodemod` frame behind the four @marigold/components-anchored primitives, and equivalent package guards to `renameImports`, `reportNamespaceImports` and `reportTokenDependencies`, so theme-only files are skipped before parsing instead of after. - hoist the duplicated `escapeRegex` into lib/regex.ts (was byte-identical in edit-css.ts and codemod/primitives/tokens.ts). - drop the dead `default` arm in `isBindingPosition` and two redundant intermediate casts; rename the `ponytail:` comment markers to `Note:`. - blank lines between the AAA blocks in the codemod tests. Docs: - document `marigold migrate` on /getting-started/cli, which only listed the other commands. --- .changeset/dst-1543-cli-migrate-docs.md | 7 + docs/content/getting-started/cli/index.mdx | 37 ++++- packages/cli/src/bin/marigold.test.ts | 17 ++ packages/cli/src/bin/marigold.ts | 118 ++++--------- packages/cli/src/commands/migrate.test.ts | 153 ++++++++++++++++- packages/cli/src/commands/migrate.ts | 127 +++++++++++--- packages/cli/src/lib/codemod/README.md | 15 +- packages/cli/src/lib/codemod/codemod.test.ts | 2 + packages/cli/src/lib/codemod/engine.ts | 32 +++- packages/cli/src/lib/codemod/jsx.test.ts | 5 + .../lib/codemod/manifests/theme-drift.test.ts | 112 +++++++++++++ packages/cli/src/lib/codemod/manifests/v18.ts | 13 +- .../cli/src/lib/codemod/primitives/jsx.ts | 156 +++++++++++------- .../cli/src/lib/codemod/primitives/report.ts | 47 ++---- .../primitives/restructure-to-slots.ts | 5 +- .../codemod/primitives/scaffold-component.ts | 1 + .../codemod/primitives/stub-missing-slots.ts | 7 +- .../codemod/primitives/swap-exact-classes.ts | 3 +- .../cli/src/lib/codemod/primitives/tokens.ts | 21 ++- packages/cli/src/lib/codemod/tokens.test.ts | 1 + packages/cli/src/lib/codemod/types.ts | 6 + packages/cli/src/lib/edit-css.ts | 3 +- packages/cli/src/lib/regex.ts | 3 + 23 files changed, 667 insertions(+), 224 deletions(-) create mode 100644 .changeset/dst-1543-cli-migrate-docs.md create mode 100644 packages/cli/src/lib/codemod/manifests/theme-drift.test.ts create mode 100644 packages/cli/src/lib/regex.ts diff --git a/.changeset/dst-1543-cli-migrate-docs.md b/.changeset/dst-1543-cli-migrate-docs.md new file mode 100644 index 0000000000..90a0cb0530 --- /dev/null +++ b/.changeset/dst-1543-cli-migrate-docs.md @@ -0,0 +1,7 @@ +--- +'@marigold/docs': patch +--- + +docs(DST-1543): document `marigold migrate` on the CLI page + +Adds a `marigold migrate` section to `/getting-started/cli`, alongside the other commands: what the codemods cover (theme slot restructures and baseline swaps, application-code renames, report-only design-token checks), the optional version and path positionals, the `--dry-run` and `--only` flags, and the safety model (warnings instead of guesses, idempotent runs, the typecheck as the completeness check). diff --git a/docs/content/getting-started/cli/index.mdx b/docs/content/getting-started/cli/index.mdx index 115fbdff9f..d623465bfb 100644 --- a/docs/content/getting-started/cli/index.mdx +++ b/docs/content/getting-started/cli/index.mdx @@ -1,7 +1,7 @@ --- title: Marigold CLI description: Component docs, discovery, and project setup, straight from the terminal. -badge: new +badge: updated --- The **`@marigold/cli`** package brings the Marigold documentation into your terminal: fetch component docs, discover components and pages, scaffold Marigold into an existing project, and more. It's useful on its own and is built to ground AI coding agents on the canonical component API. @@ -241,6 +241,41 @@ It checks, against the current working directory: health from `errors.length === 0` and act on each `suggestion`. +### marigold migrate + +Apply codemods for a breaking Marigold release, to your theme files and your application code. Run it after upgrading `@marigold/components`, from your project root. + +```bash +marigold migrate v18 # migrate the current directory +marigold migrate v18 ./src --dry-run # report what would change, write nothing +marigold migrate # detect the installed version and confirm +marigold migrate v18 --only rename-imports # apply a subset, non-interactively +``` + +| Flag | Description | Default | +| ---------------- | ----------------------------------------------------------------------- | ------- | +| `--dry-run` | Report what would change without writing files | | +| `--only ` | Apply only these changes (comma-separated), skipping the selection step | | + +Both positionals are optional. Without a version, the installed `@marigold/components` is detected and the applicable migration is proposed for confirmation. Without a path, the current directory is used. + +It anchors on imports rather than on file names or directory layout, so it works whatever your project structure is: + +1. **Theme files** (anchored on `ThemeComponent<'X'>` from `@marigold/system`): restructures single-style components into the slot shape the new version requires, moving your classes verbatim, swaps a baseline style only when it still matches the old baseline byte for byte (proof you never customized it), stubs new slots as `cva({})`, and scaffolds theme files for components the new version requires. +2. **Application code** (anchored on `@marigold/components` and `@marigold/icons`): renamed exports such as the icon migration, renamed compound components, renamed and removed props. +3. **Design tokens**: report-only, since token values are yours. Covers renamed or removed tokens you still reference, new tokens that component internals hardcode but your CSS does not define, and tokens that kept their name but changed meaning. + +Anything that cannot be decided from the source, a spread that hides slots, a customized style, a DOM change your own CSS may target, becomes a warning pointing at the release notes, never a guess. + + + `--dry-run` prints the full report and writes nothing. Interactive runs + pre-analyze the project and list the changes that actually fire, so you can + deselect any of them before applying; report-only checks always run. Applying + twice is a no-op, so a partial run can be completed later. After an applied + run, your typechecker is the completeness check: the slot `Record`s in + `@marigold/system` are exhaustive. + + ### marigold completion Print a tab-completion script for `bash`, `zsh`, or `fish`. Source it once per shell, or write it to your shell's completion directory for persistence. diff --git a/packages/cli/src/bin/marigold.test.ts b/packages/cli/src/bin/marigold.test.ts index 16646d3064..af886cf6af 100644 --- a/packages/cli/src/bin/marigold.test.ts +++ b/packages/cli/src/bin/marigold.test.ts @@ -250,3 +250,20 @@ describe('main() — doctor command', () => { }); }); }); + +describe('main() — migrate command', () => { + // The version positional is optional, so a mistyped version is otherwise + // indistinguishable from a path. The hint has to be checked before the + // positional-count validation, which would reject this as "too many paths". + test('names the migration a version-ish positional probably meant', async () => { + const code = await main(['migrate', '18.1', './src']); + + expect(code).toBe(1); + expect(stderrSpy.mock.calls.flat().join('')).toContain('Did you mean v18?'); + expect(emitMock.mock.calls[0][0]).toMatchObject({ + command: 'migrate', + exitCode: 1, + args: expect.objectContaining({ version: 'auto' }), + }); + }); +}); diff --git a/packages/cli/src/bin/marigold.ts b/packages/cli/src/bin/marigold.ts index 684221ea8c..d2ffe287dd 100644 --- a/packages/cli/src/bin/marigold.ts +++ b/packages/cli/src/bin/marigold.ts @@ -443,11 +443,15 @@ export const main = async ( } else if (command === 'migrate') { const { positionals, values } = parseMigrateCommand(rest); // the version positional is optional: `migrate ./src` treats the first - // positional as a path, `migrate v18 ./src` as version + path - const looksLikeVersion = (p: string | undefined): p is string => - p !== undefined && /^v?\d+$/.test(p); + // positional as a path, `migrate v18 ./src` as version + path. Both + // `18` and `v18` name a migration. const [first, second] = positionals; - const explicitVersion = looksLikeVersion(first) ? first : undefined; + const explicitVersion = + first !== undefined && /^v?\d+$/.test(first) + ? first.startsWith('v') + ? first + : `v${first}` + : undefined; const targetPath = (explicitVersion ? second : first) ?? process.cwd(); telemetryArgs = { @@ -455,6 +459,19 @@ export const main = async ( ...(values['dry-run'] ? { dryRun: 'true' } : {}), }; + // A version-ish first positional that is not an exact major is a typo, + // not a directory. Checked before the count below, which would + // otherwise swallow `migrate 18.1 ./src` as "too many paths". + if ( + !explicitVersion && + first !== undefined && + /^v?\d+\.[\d.]*$/.test(first) + ) { + fail( + `Unknown migration '${first}' — migrations are named by major version. ` + + `Did you mean v${Number.parseInt(first.replace(/^v/, ''), 10)}?` + ); + } if (positionals.length > (explicitVersion ? 2 : 1)) { fail( 'Usage: marigold migrate [version] [path] [--dry-run] [--only ]' @@ -463,88 +480,17 @@ export const main = async ( // Lazy-load: migrate pulls in @babel/parser and magic-string, which we // keep off the docs/list hot path. - const { detectMigration, runMigrate } = - await import('../commands/migrate.js'); - - let versions: string[]; - if (explicitVersion) { - // accept both `18` and `v18` - versions = [ - explicitVersion.startsWith('v') - ? explicitVersion - : `v${explicitVersion}`, - ]; - } else { - const detected = detectMigration(targetPath); - if (!detected) { - fail( - `Could not find @marigold/components under ${targetPath} — pass the migration explicitly: marigold migrate v18 [path]` - ); - } - if (detected.versions.length === 0) { - writeOutput( - `Detected @marigold/components ${detected.installed} (${detected.source}) — already up to date, no migration to run.` - ); - return 0; - } - if (!process.stdout.isTTY) { - fail( - `Detected @marigold/components ${detected.installed} — would run ${detected.versions.join(', then ')}. ` + - `Non-interactive session: confirm by passing the version explicitly, e.g. marigold migrate ${detected.versions[0]} [path]` - ); - } - const { confirm, isCancel } = await import('@clack/prompts'); - const proceed = await confirm({ - message: - `Detected @marigold/components ${detected.installed} (${detected.source}). ` + - `Run the ${detected.versions.join(', then the ')} migration${values['dry-run'] ? ' (dry run)' : ''}?`, - }); - if (isCancel(proceed) || proceed !== true) { - writeOutput('Aborted — nothing changed.'); - return 130; - } - versions = detected.versions; - } - - const only = values.only - ?.split(',') - .map(s => s.trim()) - .filter(Boolean); - - for (const version of versions) { - // pre-analysis: dry-run in memory, offer the fired changes for - // selection. Skipped for explicit --only / --dry-run / non-TTY runs. - let selected = only; - if (!selected && !values['dry-run'] && process.stdout.isTTY) { - const analysis = runMigrate({ version, targetPath, dryRun: true }); - if (analysis.summary.length > 0) { - const { multiselect, isCancel } = await import('@clack/prompts'); - const chosen = await multiselect({ - message: `The ${version} migration fires these changes — deselect what you want to skip (warnings always run):`, - options: analysis.summary.map(s => ({ - value: s.name, - label: s.name, - hint: `${s.description} — ${s.changes} change(s) in ${s.files} file(s)`, - })), - initialValues: analysis.summary.map(s => s.name), - required: false, - }); - if (isCancel(chosen)) { - writeOutput('Aborted — nothing changed.'); - return 130; - } - selected = chosen as string[]; - } - } - - const result = runMigrate({ - version, - targetPath, - dryRun: values['dry-run'], - only: selected, - }); - writeOutput(result.output); - } + const { runMigrateCommand } = await import('../commands/migrate.js'); + exitCode = await runMigrateCommand({ + version: explicitVersion, + targetPath, + dryRun: values['dry-run'], + only: values.only + ?.split(',') + .map(s => s.trim()) + .filter(Boolean), + write: writeOutput, + }); } else if (command === 'telemetry') { const [sub] = rest; telemetryArgs = sub ? { sub } : {}; diff --git a/packages/cli/src/commands/migrate.test.ts b/packages/cli/src/commands/migrate.test.ts index f7fbb02ebb..e5159fb594 100644 --- a/packages/cli/src/commands/migrate.test.ts +++ b/packages/cli/src/commands/migrate.test.ts @@ -2,7 +2,14 @@ import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; import { existsSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { detectMigration, runMigrate } from './migrate.js'; +import { detectMigration, runMigrate, runMigrateCommand } from './migrate.js'; + +const prompts = vi.hoisted(() => ({ + confirm: vi.fn(), + multiselect: vi.fn(), + isCancel: vi.fn(() => false), +})); +vi.mock('@clack/prompts', () => prompts); // End-to-end run against a miniature portal-shaped theme tree: standalone // theme, 4-space indent, one style file per component, barrel index. @@ -73,6 +80,7 @@ describe('detectMigration', () => { test('walks up to node_modules and proposes the applicable migration', () => { const detected = detectMigration(setupRepo('17.9.1')); + expect(detected).toEqual({ installed: '17.9.1', source: 'node_modules', @@ -82,6 +90,7 @@ describe('detectMigration', () => { test('falls back to the declared range when nothing is installed', () => { const detected = detectMigration(setupRepo('^17.0.0', false)); + expect(detected).toEqual({ installed: '17.0.0', source: 'package.json', @@ -91,6 +100,7 @@ describe('detectMigration', () => { test('proposes the same-major migration (upgrade first, then migrate)', () => { const detected = detectMigration(setupRepo('18.0.0-beta.4')); + expect(detected).toEqual({ installed: '18.0.0-beta.4', source: 'node_modules', @@ -100,6 +110,7 @@ describe('detectMigration', () => { test('reports up to date when the installed major is past every migration', () => { const detected = detectMigration(setupRepo('19.0.0')); + expect(detected).toEqual({ installed: '19.0.0', source: 'node_modules', @@ -109,6 +120,7 @@ describe('detectMigration', () => { test('returns null when no @marigold/components exists anywhere', () => { const root = mkdtempSync(path.join(os.tmpdir(), 'marigold-detect-')); + expect(detectMigration(root)).toBeNull(); }); }); @@ -401,3 +413,142 @@ export const List = () => ; expect(output).not.toContain('hardcodes'); }); }); + +describe('runMigrateCommand', () => { + // a fixture whose Marigold version is discoverable, so the command can be + // driven through detection + confirmation instead of an explicit version + const setupDetectable = (version: string): string => { + const root = setupFixture(); + writeFileSync( + path.join(root, 'package.json'), + JSON.stringify({ dependencies: { '@marigold/components': version } }) + ); + return root; + }; + + beforeEach(() => { + prompts.isCancel.mockReturnValue(false); + }); + + test('runs the explicitly named migration without prompting', async () => { + const root = setupFixture(); + const written: string[] = []; + + const code = await runMigrateCommand({ + version: 'v18', + targetPath: root, + dryRun: true, + write: output => written.push(output), + interactive: false, + }); + + expect(code).toBe(0); + expect(written.join('\n')).toContain('marigold migrate v18 (dry run)'); + expect(prompts.confirm).not.toHaveBeenCalled(); + }); + + test('refuses to auto-detect without a terminal to confirm in', async () => { + const root = setupDetectable('17.9.1'); + + const run = runMigrateCommand({ + targetPath: root, + dryRun: true, + write: () => {}, + interactive: false, + }); + + await expect(run).rejects.toThrow(/Non-interactive session/); + }); + + test('reports a target that has no migration left to run', async () => { + const root = setupDetectable('19.0.0'); + const written: string[] = []; + + const code = await runMigrateCommand({ + targetPath: root, + dryRun: true, + write: output => written.push(output), + interactive: true, + }); + + expect(code).toBe(0); + expect(written.join('\n')).toContain('already up to date'); + }); + + test('fails when the target has no Marigold at all', async () => { + const root = setupFixture(); + + const run = runMigrateCommand({ + targetPath: root, + dryRun: true, + write: () => {}, + interactive: true, + }); + + await expect(run).rejects.toThrow(/Could not find @marigold\/components/); + }); + + test('aborts with 130 when the detected migration is declined', async () => { + const root = setupDetectable('17.9.1'); + const written: string[] = []; + prompts.confirm.mockResolvedValue(false); + + const code = await runMigrateCommand({ + targetPath: root, + dryRun: false, + write: output => written.push(output), + interactive: true, + }); + + expect(code).toBe(130); + expect(written).toEqual(['Aborted — nothing changed.']); + }); + + test('applies only the changes left selected in the multiselect', async () => { + const root = setupDetectable('17.9.1'); + const components = path.join(root, 'theme', 'components'); + const cardBefore = readFileSync( + path.join(components, 'Card.styles.ts'), + 'utf8' + ); + prompts.confirm.mockResolvedValue(true); + prompts.multiselect.mockResolvedValue(['swap-exact-classes']); + + await runMigrateCommand({ + targetPath: root, + dryRun: false, + write: () => {}, + interactive: true, + }); + + expect(readFileSync(path.join(components, 'Card.styles.ts'), 'utf8')).toBe( + cardBefore + ); + expect( + readFileSync(path.join(components, 'Switch.styles.ts'), 'utf8') + ).toContain(`'grid gap-x-2 items-center'`); + }); + + test('offers every change that fired, described, for selection', async () => { + const root = setupDetectable('17.9.1'); + prompts.confirm.mockResolvedValue(true); + prompts.multiselect.mockResolvedValue([]); + + await runMigrateCommand({ + targetPath: root, + dryRun: false, + write: () => {}, + interactive: true, + }); + + expect(prompts.multiselect).toHaveBeenCalledWith( + expect.objectContaining({ + options: [ + expect.objectContaining({ value: 'restructure-to-slots' }), + expect.objectContaining({ value: 'swap-exact-classes' }), + expect.objectContaining({ value: 'scaffold-components' }), + ], + }) + ); + }); +}); diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index 035d15607c..906f18192d 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -119,20 +119,12 @@ export interface MigrateResult { summary: CodemodSummary[]; } +// Scaffolding is a pipeline phase rather than a Codemod (it creates files +// instead of editing one), so unlike the codemods it carries its selection +// name and description here. const SCAFFOLD = 'scaffold-components'; - -// user-facing one-liners for the selectable changes (report-only passes are -// not selectable: they are the safety net and always run) -const DESCRIPTIONS: Record = { - 'restructure-to-slots': 'move single-style theme components to slot objects', - 'swap-exact-classes': 'swap unchanged baseline styles to the new baseline', - 'stub-missing-slots': 'stub new theme slots with empty cva()', - 'rename-jsx-members': 'rename compound components (e.g. Tabs.TabPanel)', - 'rename-jsx-props': 'rename component props (e.g. Inset space to p)', - 'remove-jsx-props': 'remove props the target version dropped', - 'rename-imports': 'rename moved exports (e.g. the icon renames)', - [SCAFFOLD]: 'create theme styles for components the target version requires', -}; +const SCAFFOLD_DESCRIPTION = + 'create theme styles for components the target version requires'; const IGNORED_DIRS = new Set([ 'node_modules', @@ -402,13 +394,12 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { total.changes += created.length; totals.set(SCAFFOLD, total); } - const summary: CodemodSummary[] = [...editCodemods.map(c => c.name), SCAFFOLD] - .filter(name => totals.has(name)) - .map(name => ({ - name, - description: DESCRIPTIONS[name] ?? '', - ...totals.get(name)!, - })); + const summary: CodemodSummary[] = [ + ...editCodemods.map(c => ({ name: c.name, description: c.description })), + { name: SCAFFOLD, description: SCAFFOLD_DESCRIPTION }, + ] + .filter(({ name }) => totals.has(name)) + .map(entry => ({ ...entry, ...totals.get(entry.name)! })); // render report. picocolors is TTY-aware: piped / test / agent output stays // byte-for-byte plain, only interactive terminals gain color. @@ -480,3 +471,99 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { } return { output: lines.join('\n'), summary }; }; + +/** a prompt has to be shown AND answered, so both ends must be a terminal */ +const canPrompt = (): boolean => + Boolean(process.stdout.isTTY && process.stdin.isTTY); + +export interface MigrateCommandOptions { + /** migration to run (`v18`); omitted: detect it and confirm interactively */ + version?: string; + targetPath: string; + dryRun: boolean; + /** `--only` names; skips the interactive change selection */ + only?: readonly string[]; + /** where reports and status messages go */ + write: (output: string) => void; + /** override the TTY check (tests) */ + interactive?: boolean; +} + +/** + * The `marigold migrate` flow: resolve which migration(s) to run (explicit, + * or detected from the target and confirmed), offer the changes that fire + * for deselection, then apply. Prompts happen only on a real terminal; every + * non-interactive path either runs unattended or throws with instructions. + * Returns the exit code (130 when the user aborted a prompt). + */ +export const runMigrateCommand = async ( + options: MigrateCommandOptions +): Promise => { + const { targetPath, dryRun, only, write } = options; + const interactive = options.interactive ?? canPrompt(); + + let versions: string[]; + if (options.version) { + versions = [options.version]; + } else { + const detected = detectMigration(targetPath); + if (!detected) { + throw new Error( + `Could not find @marigold/components under ${targetPath} — pass the migration explicitly: marigold migrate v18 [path]` + ); + } + if (detected.versions.length === 0) { + write( + `Detected @marigold/components ${detected.installed} (${detected.source}) — already up to date, no migration to run.` + ); + return 0; + } + if (!interactive) { + throw new Error( + `Detected @marigold/components ${detected.installed} — would run ${detected.versions.join(', then ')}. ` + + `Non-interactive session: confirm by passing the version explicitly, e.g. marigold migrate ${detected.versions[0]} [path]` + ); + } + const { confirm, isCancel } = await import('@clack/prompts'); + const proceed = await confirm({ + message: + `Detected @marigold/components ${detected.installed} (${detected.source}). ` + + `Run the ${detected.versions.join(', then the ')} migration${dryRun ? ' (dry run)' : ''}?`, + }); + if (isCancel(proceed) || proceed !== true) { + write('Aborted — nothing changed.'); + return 130; + } + versions = detected.versions; + } + + for (const version of versions) { + // pre-analysis: dry-run in memory, offer the fired changes for + // selection. Skipped for explicit --only / --dry-run / non-TTY runs. + let selected = only; + if (!selected && !dryRun && interactive) { + const analysis = runMigrate({ version, targetPath, dryRun: true }); + if (analysis.summary.length > 0) { + const { multiselect, isCancel } = await import('@clack/prompts'); + const chosen = await multiselect({ + message: `The ${version} migration fires these changes — deselect what you want to skip (warnings always run):`, + options: analysis.summary.map(s => ({ + value: s.name, + label: s.name, + hint: `${s.description} — ${s.changes} change(s) in ${s.files} file(s)`, + })), + initialValues: analysis.summary.map(s => s.name), + required: false, + }); + if (isCancel(chosen)) { + write('Aborted — nothing changed.'); + return 130; + } + selected = chosen as string[]; + } + } + + write(runMigrate({ version, targetPath, dryRun, only: selected }).output); + } + return 0; +}; diff --git a/packages/cli/src/lib/codemod/README.md b/packages/cli/src/lib/codemod/README.md index 4cbb89708b..886b91adbb 100644 --- a/packages/cli/src/lib/codemod/README.md +++ b/packages/cli/src/lib/codemod/README.md @@ -212,7 +212,13 @@ Per-version work should be data entry, not transform code: with", e.g. the prop type that excludes the removed value. Codegen should re-resolve line numbers against the release tag. 2. Register it in `MANIFESTS` in `commands/migrate.ts`. -3. Acceptance test: `--dry-run` against a real consumer repo and read every +3. Point `manifests/theme-drift.test.ts` at the new manifest. It compares + `slots` against the live `Theme` type in `@marigold/system` and is the + re-check trigger until DST-1650's codegen lands: a hand-written manifest + silently falls behind every component merged after it was authored (v18's + `Sidebar` rail slots and `ErrorState` did exactly that), which costs + consumers their stubs and produces false "not themeable" warnings. +4. Acceptance test: `--dry-run` against a real consumer repo and read every line of the report. A change that no primitive can express gets a **one-off transform** for that @@ -226,6 +232,13 @@ general abstraction from a single example is how wrong abstractions happen. must return `edited | unchanged | skipped` plus warnings, re-parse its own input (transforms are chained by re-parsing; babel parse is fast), and uphold the invariants above. +- A `Codemod` carries its own `description` (the one-liner the interactive + selection shows), so name and description cannot drift apart. +- Build on a shared frame rather than repeating it: `themeCodemod` in + `engine.ts` for anything anchored on `ThemeComponent` (a visit that only + pushes warnings makes it a report-only pass), `jsxCodemod` in + `primitives/jsx.ts` for anything anchored on `@marigold/components`. Both + short-circuit before parsing on files that cannot match. - Add fixture tests in `codemod.test.ts`. Fixtures are consumer-shaped (4-space indent, single quotes, portal-style files), and byte-preservation is asserted on the fixture's class strings. diff --git a/packages/cli/src/lib/codemod/codemod.test.ts b/packages/cli/src/lib/codemod/codemod.test.ts index 20504b4167..0ea42a67eb 100644 --- a/packages/cli/src/lib/codemod/codemod.test.ts +++ b/packages/cli/src/lib/codemod/codemod.test.ts @@ -65,6 +65,7 @@ describe('anchor', () => { const source = `import { cva, ThemeComponent } from 'other-system'; export const Switch: ThemeComponent<'Switch'> = { container: cva({}) }; `; + expect(findThemeComponents(parse(source))).toEqual([]); }); @@ -72,6 +73,7 @@ export const Switch: ThemeComponent<'Switch'> = { container: cva({}) }; const source = `import { ThemeComponent as TC, cva } from '@marigold/system'; export const Badge: TC<'Badge'> = cva({}); `; + expect(findThemeComponents(parse(source)).map(d => d.component)).toEqual([ 'Badge', ]); diff --git a/packages/cli/src/lib/codemod/engine.ts b/packages/cli/src/lib/codemod/engine.ts index f9a88158bb..8f4c3a8654 100644 --- a/packages/cli/src/lib/codemod/engine.ts +++ b/packages/cli/src/lib/codemod/engine.ts @@ -37,6 +37,8 @@ export interface ThemeVisit { source: string; s: MagicString; unit: string; + /** local name of `cva` in this file — aliased imports keep their alias */ + cva: string; changes: string[]; warnings: string[]; } @@ -46,14 +48,17 @@ export interface ThemeVisit { * anchor requires a literal `ThemeComponent` import), parse-or-skip, one * MagicString, a visit per anchored component, and the uniform * unchanged/edited outcome. `ensureCva` adds the cva import when edits - * introduced stubs. + * introduced stubs. A visit that only pushes warnings makes this a + * report-only pass: the outcome stays `unchanged` and carries them. */ export const themeCodemod = ( name: string, + description: string, visit: (ctx: ThemeVisit) => void, options: { ensureCva?: boolean } = {} ): Codemod => ({ name, + description, apply: source => { if (!source.includes('ThemeComponent')) { return { kind: 'unchanged', warnings: [] }; @@ -61,10 +66,23 @@ export const themeCodemod = ( return parseOr(source, file => { const s = new MagicString(source); const unit = detectIndentUnit(source); + // stubs must call cva under the name this file imports it as; the + // import is only inserted below, when edits actually happened + const cva = marigoldLocalName(file, 'cva') ?? 'cva'; const changes: string[] = []; const warnings: string[] = []; for (const { component, init } of findThemeComponents(file)) { - visit({ component, init, file, source, s, unit, changes, warnings }); + visit({ + component, + init, + file, + source, + s, + unit, + cva, + changes, + warnings, + }); } if (changes.length === 0) return { kind: 'unchanged', warnings }; if (options.ensureCva) ensureCvaImport(s, file); @@ -83,8 +101,7 @@ export const classStringsIn = (node: AnyNode): string[] => { walk(node, n => { if ( n.type === 'ObjectProperty' && - (n.key as AnyNode | undefined as { name?: string } | undefined)?.name === - 'defaultVariants' + (n.key as { name?: string } | undefined)?.name === 'defaultVariants' ) { return false; } @@ -158,8 +175,11 @@ export const codeList = (names: string[]): string => names.map(name => `\`${name}\``).join(', '); /** the empty-stub property line shared by restructure and stubbing */ -export const stubSlotLine = (slot: string, indent: string): string => - `${indent}${asPropertyKey(slot)}: cva({}),`; +export const stubSlotLine = ( + slot: string, + indent: string, + cva: string +): string => `${indent}${asPropertyKey(slot)}: ${cva}({}),`; /** deep link to the default theme's styles for a component, if configured */ export const stylesReference = ( diff --git a/packages/cli/src/lib/codemod/jsx.test.ts b/packages/cli/src/lib/codemod/jsx.test.ts index 5dabd0fd60..1f04726eb6 100644 --- a/packages/cli/src/lib/codemod/jsx.test.ts +++ b/packages/cli/src/lib/codemod/jsx.test.ts @@ -34,6 +34,7 @@ export const App = () => ( const source = `import { Inset } from './my-components'; export const App = () => ; `; + expect(renameJsxProps(v18).apply(source).kind).toBe('unchanged'); }); @@ -103,6 +104,7 @@ export const App = () => ( const source = `import { Tabs } from 'other-lib'; export const App = () => content; `; + expect(renameJsxMembers(v18).apply(source).kind).toBe('unchanged'); }); }); @@ -262,6 +264,7 @@ export const App = () => ; import { Email } from 'other-icons'; export const App = () => ; `; + expect(renameImports(v18).apply(source).kind).toBe('unchanged'); }); @@ -274,6 +277,7 @@ export const App = () => ; `; const reexported = `export { Store as Pickup } from '@marigold/icons'; `; + expect(renameImports(v18).apply(direct).kind).toBe('unchanged'); expect(renameImports(v18).apply(aliased).kind).toBe('unchanged'); expect(renameImports(v18).apply(reexported).kind).toBe('unchanged'); @@ -298,6 +302,7 @@ export const App = () => ; const source = `import * as path from 'node:path'; export const join = path.join; `; + expect(warningsOf(reportNamespaceImports(v18).apply(source))).toEqual([]); }); }); diff --git a/packages/cli/src/lib/codemod/manifests/theme-drift.test.ts b/packages/cli/src/lib/codemod/manifests/theme-drift.test.ts new file mode 100644 index 0000000000..34dbe74b19 --- /dev/null +++ b/packages/cli/src/lib/codemod/manifests/theme-drift.test.ts @@ -0,0 +1,112 @@ +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { type AnyNode, parseTsx, walk } from '../../tsx-ast.js'; +import { v18 } from './v18.js'; + +// The manifest's `slots` mirror the `Theme` type in @marigold/system, and the +// two have drifted apart once already: components merged into `beta-release` +// after the manifest was hand-written (Sidebar's rail slots, ErrorState) left +// consumers without stubs and produced a false "not themeable" warning. +// Until DST-1650 generates the manifests, this test is the re-check trigger. +// +// It pins the manifest of the CURRENT major against the live type. When v19 +// lands, point it at the v19 manifest: v18's slots are then frozen history, +// not a mirror of the type. +const THEME_TYPES = path.join( + path.dirname(fileURLToPath(import.meta.url)), + '../../../../../system/src/types/theme.ts' +); + +// Babel 8 stores type arguments as `typeArguments`, Babel 7 as +// `typeParameters` (same accommodation as anchor.ts). +const typeArgs = (ref: AnyNode): AnyNode[] => + ( + (ref.typeArguments ?? ref.typeParameters) as + | { params?: AnyNode[] } + | undefined + )?.params ?? []; + +/** string-literal members of `'a' | 'b'` (or of a lone `'a'`) */ +const literalNames = (node: AnyNode | undefined): string[] => { + if (!node) return []; + const types = + node.type === 'TSUnionType' ? (node.types as AnyNode[]) : [node]; + return types + .filter(t => t.type === 'TSLiteralType') + .map(t => (t.literal as { value?: string } | undefined)?.value) + .filter((value): value is string => typeof value === 'string'); +}; + +/** + * Slot sets of `Theme['components']`, keyed by component. `null` marks a + * single style function — the same shape the manifest uses. + */ +const themeSlots = (source: string): Record => { + const out: Record = {}; + walk(parseTsx(source) as unknown as AnyNode, n => { + if (n.type !== 'TSPropertySignature') return; + if ((n.key as { name?: string } | undefined)?.name !== 'components') return; + const literal = (n.typeAnnotation as AnyNode | undefined) + ?.typeAnnotation as AnyNode | undefined; + for (const member of (literal?.members as AnyNode[] | undefined) ?? []) { + const name = (member.key as { name?: string } | undefined)?.name; + if (!name) continue; + const annotation = (member.typeAnnotation as AnyNode | undefined) + ?.typeAnnotation as AnyNode | undefined; + const isRecord = + annotation?.type === 'TSTypeReference' && + (annotation.typeName as { name?: string } | undefined)?.name === + 'Record'; + out[name] = isRecord ? literalNames(typeArgs(annotation)[0]) : null; + } + return false; + }); + return out; +}; + +describe('v18 manifest vs the @marigold/system Theme type', () => { + const theme = themeSlots(readFileSync(THEME_TYPES, 'utf8')); + + test('reads the component map out of the Theme type', () => { + const components = Object.keys(theme); + + // guards the assertions below against passing vacuously on a parse miss + expect(components.length).toBeGreaterThan(50); + }); + + test('knows every themeable component', () => { + const missing = Object.keys(theme).filter(name => !(name in v18.slots)); + + expect(missing).toEqual([]); + }); + + test('knows no component the Theme type does not have', () => { + const extra = Object.keys(v18.slots).filter(name => !(name in theme)); + + expect(extra).toEqual([]); + }); + + test('mirrors every slot set', () => { + const drifted: string[] = []; + for (const [name, slots] of Object.entries(theme)) { + const known = v18.slots[name]; + if (known === undefined) continue; // reported by the test above + if (slots === null || known === null) { + if ((slots === null) !== (known === null)) { + drifted.push( + `${name}: ${slots === null ? 'single style function' : 'slotted'} in the Theme type, the other in the manifest` + ); + } + continue; + } + const missing = slots.filter(slot => !known.includes(slot)); + const extra = known.filter(slot => !slots.includes(slot)); + if (missing.length > 0 || extra.length > 0) { + drifted.push(`${name}: missing [${missing}], extra [${extra}]`); + } + } + + expect(drifted).toEqual([]); + }); +}); diff --git a/packages/cli/src/lib/codemod/manifests/v18.ts b/packages/cli/src/lib/codemod/manifests/v18.ts index b53955e727..5b76794438 100644 --- a/packages/cli/src/lib/codemod/manifests/v18.ts +++ b/packages/cli/src/lib/codemod/manifests/v18.ts @@ -233,6 +233,7 @@ export const v18: MigrationManifest = { 'itemRemove', ], EmptyState: ['container', 'title', 'description', 'action'], + ErrorState: ['container', 'title', 'description', 'action'], ToggleButton: ['group', 'button'], SegmentedControl: ['group', 'list', 'field', 'option', 'indicator'], Sidebar: [ @@ -250,6 +251,16 @@ export const v18: MigrationManifest = { 'navLink', 'backButton', 'content', + // two-level rail (Sidebar.Rail): persistent rail + section panel + 'railRoot', + 'railLayout', + 'railColumn', + 'railToggle', + 'rail', + 'railItem', + 'railFooter', + 'panel', + 'panelTitle', ], TopNavigation: ['container', 'start', 'middle', 'end'], }, @@ -465,7 +476,7 @@ export const v18: MigrationManifest = { package: '@marigold/icons', from: 'Print', to: 'Printer', - note: 'missing from the official mapping table — Printer is the Lucide equivalent', + note: 'added to the official mapping table in this PR — Printer is the Lucide equivalent', }, ], removals: [ diff --git a/packages/cli/src/lib/codemod/primitives/jsx.ts b/packages/cli/src/lib/codemod/primitives/jsx.ts index 97ea09e84a..f586e06dac 100644 --- a/packages/cli/src/lib/codemod/primitives/jsx.ts +++ b/packages/cli/src/lib/codemod/primitives/jsx.ts @@ -29,8 +29,7 @@ const jsxAttributes = (opening: AnyNode | undefined): AnyNode[] => ); const attrName = (attr: AnyNode): string | null => - (attr.name as AnyNode | undefined as { name?: string } | undefined)?.name ?? - null; + (attr.name as { name?: string } | undefined)?.name ?? null; /** local names for the manifest's component names, only when imported */ export const localsFor = (file: AnyNode, components: Iterable) => { @@ -51,17 +50,46 @@ export const localsFor = (file: AnyNode, components: Iterable) => { return locals; }; -/** Renames props on Marigold components, e.g. Inset `space` to `p`. */ -export const renameJsxProps = (manifest: MigrationManifest): Codemod => ({ - name: 'rename-jsx-props', - apply: source => - parseOr(source, file => { - const locals = localsFor( - file, - manifest.jsx.renames.map(e => e.component) - ); +export interface JsxVisit { + file: AnyNode; + source: string; + /** local name -> canonical component name, for the components asked for */ + locals: Map; +} + +/** + * The shared frame of the primitives anchored on @marigold/components: skip + * without parsing when the file cannot import the package at all, parse-or- + * skip, resolve the manifest's components to their local names, and skip + * again when this file imports none of them. + */ +const jsxCodemod = ( + meta: { name: string; description: string }, + components: string[], + run: (ctx: JsxVisit) => CodemodOutcome +): Codemod => ({ + ...meta, + apply: source => { + if (!source.includes(MARIGOLD_COMPONENTS)) { + return { kind: 'unchanged', warnings: [] }; + } + return parseOr(source, file => { + const locals = localsFor(file, components); if (locals.size === 0) return { kind: 'unchanged', warnings: [] }; + return run({ file, source, locals }); + }); + }, +}); +/** Renames props on Marigold components, e.g. Inset `space` to `p`. */ +export const renameJsxProps = (manifest: MigrationManifest): Codemod => + jsxCodemod( + { + name: 'rename-jsx-props', + description: 'rename component props (e.g. Inset space to p)', + }, + manifest.jsx.renames.map(e => e.component), + ({ file, source, locals }) => { const s = new MagicString(source); const changes: string[] = []; for (const el of jsxElements(file)) { @@ -87,8 +115,8 @@ export const renameJsxProps = (manifest: MigrationManifest): Codemod => ({ } if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; return { kind: 'edited', output: s.toString(), changes, warnings: [] }; - }), -}); + } + ); const wrapValueInArray = ( s: MagicString, @@ -169,14 +197,10 @@ const isBindingPosition = (n: AnyNode, parent: AnyNode | null): boolean => { case 'RestElement': case 'AssignmentPattern': return true; + // A plain function param (`function f(Pickup) {}`) is deliberately not + // listed: it produces a correct uniform alpha-rename, and a collision + // with the target name is caught by `namesInFile` independently. default: - if ( - (parent.type === 'FunctionDeclaration' || - parent.type === 'FunctionExpression') && - ((parent.params as AnyNode[]) ?? []).includes(n) - ) { - return true; - } return false; } }; @@ -203,10 +227,18 @@ export const renameImports = (manifest: MigrationManifest): Codemod => { byPackage.set(entry.package, forPackage); } + const packages = [...byPackage.keys()]; + return { name: 'rename-imports', - apply: source => - parseOr(source, file => { + description: 'rename moved exports (e.g. the icon renames)', + apply: source => { + // these anchor on their own packages (@marigold/icons), not on + // @marigold/components, so they cannot use the jsxCodemod frame + if (!packages.some(pkg => source.includes(pkg))) { + return { kind: 'unchanged', warnings: [] }; + } + return parseOr(source, file => { const s = new MagicString(source); const changes: string[] = []; @@ -357,7 +389,8 @@ export const renameImports = (manifest: MigrationManifest): Codemod => { } if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; return { kind: 'edited', output: s.toString(), changes, warnings: [] }; - }), + }); + }, }; }; @@ -377,8 +410,12 @@ export const reportNamespaceImports = ( ]); return { name: 'report-namespace-imports', - apply: source => - parseOr(source, file => { + description: 'report namespace imports the codemods cannot follow', + apply: source => { + if (![...affected].some(pkg => source.includes(pkg))) { + return { kind: 'unchanged', warnings: [] }; + } + return parseOr(source, file => { const warnings: string[] = []; for (const imp of collectImports(file)) { const src = (imp.source as { value?: string } | undefined)?.value; @@ -392,21 +429,20 @@ export const reportNamespaceImports = ( } } return { kind: 'unchanged', warnings }; - }), + }); + }, }; }; /** Renames compound members, e.g. Tabs.TabPanel to Tabs.Panel. */ -export const renameJsxMembers = (manifest: MigrationManifest): Codemod => ({ - name: 'rename-jsx-members', - apply: source => - parseOr(source, file => { - const locals = localsFor( - file, - manifest.jsx.memberRenames.map(e => e.object) - ); - if (locals.size === 0) return { kind: 'unchanged', warnings: [] }; - +export const renameJsxMembers = (manifest: MigrationManifest): Codemod => + jsxCodemod( + { + name: 'rename-jsx-members', + description: 'rename compound components (e.g. Tabs.TabPanel)', + }, + manifest.jsx.memberRenames.map(e => e.object), + ({ file, locals, source }) => { const s = new MagicString(source); const changes: string[] = []; const renameTag = (tag: AnyNode | undefined, to: string): void => { @@ -442,20 +478,18 @@ export const renameJsxMembers = (manifest: MigrationManifest): Codemod => ({ } if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; return { kind: 'edited', output: s.toString(), changes, warnings: [] }; - }), -}); + } + ); /** Removes props the target version dropped, e.g. TextField min/max. */ -export const removeJsxProps = (manifest: MigrationManifest): Codemod => ({ - name: 'remove-jsx-props', - apply: source => - parseOr(source, file => { - const locals = localsFor( - file, - manifest.jsx.removals.map(e => e.component) - ); - if (locals.size === 0) return { kind: 'unchanged', warnings: [] }; - +export const removeJsxProps = (manifest: MigrationManifest): Codemod => + jsxCodemod( + { + name: 'remove-jsx-props', + description: 'remove props the target version dropped', + }, + manifest.jsx.removals.map(e => e.component), + ({ file, locals, source }) => { const s = new MagicString(source); const changes: string[] = []; for (const el of jsxElements(file)) { @@ -477,25 +511,23 @@ export const removeJsxProps = (manifest: MigrationManifest): Codemod => ({ } if (changes.length === 0) return { kind: 'unchanged', warnings: [] }; return { kind: 'edited', output: s.toString(), changes, warnings: [] }; - }), -}); + } + ); /** * Report-only: usages that need a human decision (removed components, * props that moved structurally, design decisions). Value-conditional * entries also warn when the value cannot be verified statically. */ -export const reportJsxUsage = (manifest: MigrationManifest): Codemod => ({ - name: 'report-jsx-usage', - apply: (source): CodemodOutcome => - parseOr(source, file => { +export const reportJsxUsage = (manifest: MigrationManifest): Codemod => + jsxCodemod( + { + name: 'report-jsx-usage', + description: 'report usages that need a human decision', + }, + manifest.jsx.warnings.map(e => e.component), + ({ file, locals }) => { const warnings: string[] = []; - const locals = localsFor( - file, - manifest.jsx.warnings.map(e => e.component) - ); - if (locals.size === 0) return { kind: 'unchanged', warnings }; - const imported = new Set(locals.values()); for (const entry of manifest.jsx.warnings) { if (!entry.prop && imported.has(entry.component)) { @@ -525,5 +557,5 @@ export const reportJsxUsage = (manifest: MigrationManifest): Codemod => ({ } } return { kind: 'unchanged', warnings }; - }), -}); + } + ); diff --git a/packages/cli/src/lib/codemod/primitives/report.ts b/packages/cli/src/lib/codemod/primitives/report.ts index 15d5a731ec..30b74f33a9 100644 --- a/packages/cli/src/lib/codemod/primitives/report.ts +++ b/packages/cli/src/lib/codemod/primitives/report.ts @@ -1,31 +1,15 @@ -import { findThemeComponents } from '../anchor.js'; -import { parseOr } from '../engine.js'; -import type { Codemod, CodemodOutcome, MigrationManifest } from '../types.js'; - -const analyze = ( - source: string, - collect: (component: string, warnings: string[]) => void -): CodemodOutcome => { - if (!source.includes('ThemeComponent')) { - return { kind: 'unchanged', warnings: [] }; - } - return parseOr(source, file => { - const warnings: string[] = []; - for (const { component } of findThemeComponents(file)) { - collect(component, warnings); - } - return { kind: 'unchanged', warnings }; - }); -}; +import { themeCodemod } from '../engine.js'; +import type { Codemod, MigrationManifest } from '../types.js'; /** * Theme keys for components the target version no longer knows: they * silently no-op at runtime and are dead weight in the consumer theme. */ -export const reportDeadKeys = (manifest: MigrationManifest): Codemod => ({ - name: 'report-dead-keys', - apply: source => - analyze(source, (component, warnings) => { +export const reportDeadKeys = (manifest: MigrationManifest): Codemod => + themeCodemod( + 'report-dead-keys', + 'report theme keys the target version no longer knows', + ({ component, warnings }) => { if (component in manifest.slots) return; const removed = manifest.removedComponents.includes(component); warnings.push( @@ -33,20 +17,21 @@ export const reportDeadKeys = (manifest: MigrationManifest): Codemod => ({ ? `${component}: component was removed in ${manifest.version} — these styles are dead` : `${component}: not a themeable component in ${manifest.version} — these styles are silently unused` ); - }), -}); + } + ); /** * HTML-structure changes are not auto-fixable from here (the consumer may * target the old DOM with their own CSS, e.g. generated BEM selectors), so * they surface as structured warnings on the components actually themed. */ -export const reportStructure = (manifest: MigrationManifest): Codemod => ({ - name: 'report-structure', - apply: source => - analyze(source, (component, warnings) => { +export const reportStructure = (manifest: MigrationManifest): Codemod => + themeCodemod( + 'report-structure', + 'report HTML-structure changes that may break your own CSS', + ({ component, warnings }) => { for (const entry of manifest.structureWarnings) { if (entry.component === component) warnings.push(entry.text); } - }), -}); + } + ); diff --git a/packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts b/packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts index 9031c31e55..b5b8a07790 100644 --- a/packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts +++ b/packages/cli/src/lib/codemod/primitives/restructure-to-slots.ts @@ -16,7 +16,8 @@ import type { Codemod, MigrationManifest } from '../types.js'; export const restructureToSlots = (manifest: MigrationManifest): Codemod => themeCodemod( 'restructure-to-slots', - ({ component, init, source, s, unit, changes, warnings }) => { + 'move single-style theme components to slot objects', + ({ component, init, source, s, unit, cva, changes, warnings }) => { const slots = manifest.slots[component]; if (!Array.isArray(slots)) return; if (init.type === 'ObjectExpression') return; // already slotted @@ -37,7 +38,7 @@ export const restructureToSlots = (manifest: MigrationManifest): Codemod => const inner = base + unit; const stubs = slots .filter(slot => slot !== primary) - .map(slot => stubSlotLine(slot, inner)) + .map(slot => stubSlotLine(slot, inner, cva)) .join('\n'); const original = source.slice(start, end); s.overwrite( diff --git a/packages/cli/src/lib/codemod/primitives/scaffold-component.ts b/packages/cli/src/lib/codemod/primitives/scaffold-component.ts index 43a3c5ff34..370e5e46d8 100644 --- a/packages/cli/src/lib/codemod/primitives/scaffold-component.ts +++ b/packages/cli/src/lib/codemod/primitives/scaffold-component.ts @@ -49,6 +49,7 @@ export const addIndexExport = ( context?: string ): Codemod => ({ name: 'add-index-export', + description: 'register a scaffolded component in the theme barrel', apply: (source): CodemodOutcome => { const exportLine = `export * from './${moduleBase}';`; if (source.includes(`'./${moduleBase}'`)) { diff --git a/packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts b/packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts index b297369d22..c850fca245 100644 --- a/packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts +++ b/packages/cli/src/lib/codemod/primitives/stub-missing-slots.ts @@ -17,7 +17,8 @@ import type { Codemod, MigrationManifest } from '../types.js'; export const stubMissingSlots = (manifest: MigrationManifest): Codemod => themeCodemod( 'stub-missing-slots', - ({ component, init, source, s, unit, changes, warnings }) => { + 'stub new theme slots with empty cva()', + ({ component, init, source, s, unit, cva, changes, warnings }) => { const slots = manifest.slots[component]; if (!Array.isArray(slots) || init.type !== 'ObjectExpression') return; @@ -55,7 +56,9 @@ export const stubMissingSlots = (manifest: MigrationManifest): Codemod => const base = lineIndentAt(source, init.start as number); const inner = base + unit; - const stubs = missing.map(slot => stubSlotLine(slot, inner)).join('\n'); + const stubs = missing + .map(slot => stubSlotLine(slot, inner, cva)) + .join('\n'); if (props.length === 0) { s.overwrite( diff --git a/packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts b/packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts index 64b71c927f..bda4bb15ef 100644 --- a/packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts +++ b/packages/cli/src/lib/codemod/primitives/swap-exact-classes.ts @@ -50,6 +50,7 @@ export const swapExactClasses = (manifest: MigrationManifest): Codemod => { return themeCodemod( 'swap-exact-classes', + 'swap unchanged baseline styles to the new baseline', ({ component, init, file, source, s, unit, changes, warnings }) => { const entries = resolved.get(component); if (!entries || init.type !== 'ObjectExpression') return; @@ -97,7 +98,7 @@ export const swapExactClasses = (manifest: MigrationManifest): Codemod => { // Render the change as a token diff (the report colorizes -/+ lines) // so renamed tokens and new layout utilities are visible at a glance. - // ponytail: added utilities are flagged for manual verification; the + // Note: added utilities are flagged for manual verification; the // upgrade path is resolving them against the consumer's actual CSS. if (entry.added.length > 0 || entry.removed.length > 0) { warnings.push( diff --git a/packages/cli/src/lib/codemod/primitives/tokens.ts b/packages/cli/src/lib/codemod/primitives/tokens.ts index ca01b54aea..0224329870 100644 --- a/packages/cli/src/lib/codemod/primitives/tokens.ts +++ b/packages/cli/src/lib/codemod/primitives/tokens.ts @@ -1,3 +1,5 @@ +import { escapeRegex } from '../../regex.js'; +import { MARIGOLD_COMPONENTS } from '../anchor.js'; import { codeList, parseOr } from '../engine.js'; import type { Codemod, MigrationManifest } from '../types.js'; import { localsFor } from './jsx.js'; @@ -24,19 +26,16 @@ export const definedTokensIn = (css: string): string[] => // Utility prefixes that take a color token (`bg-brand`). A curated list // keeps text scanning honest: matching bare `-brand` suffixes would also // hit variants and unrelated identifiers. -// ponytail: covers the color utilities Marigold themes actually use; extend -// the list when a consumer surfaces one we missed. +// Note: covers the color utilities Marigold themes actually use; extend the +// list when a consumer surfaces one we missed. const COLOR_PREFIXES = 'bg|text|border|ring|inset-ring|outline|fill|stroke|decoration|divide|accent|caret|shadow|from|via|to|placeholder'; -const escapeRegExp = (name: string): string => - name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - /** longest name first, so `warning-muted-foreground` beats `muted-foreground` */ const alternation = (names: string[]): string => [...names] .sort((a, b) => b.length - a.length) - .map(escapeRegExp) + .map(escapeRegex) .join('|'); /** @@ -145,7 +144,7 @@ export const reportTokenUsage = ( .map( ([token, entry]) => new RegExp( - `\\b(?:${entry.oldRolePrefixes!.join('|')})-(${escapeRegExp(token)})(?![\\w-])`, + `\\b(?:${entry.oldRolePrefixes!.join('|')})-(${escapeRegex(token)})(?![\\w-])`, 'g' ) ); @@ -160,6 +159,7 @@ export const reportTokenUsage = ( return { name: 'report-token-usage', + description: 'report design-token references that break in this version', apply: source => { const warnings: string[] = []; for (const [ref, finding] of collectFindings(source, oldPatterns)) { @@ -212,8 +212,13 @@ export const reportTokenDependencies = ( return { name: 'report-token-dependencies', + description: 'report tokens the new component internals hardcode', apply: source => { - if (pending.length === 0) return { kind: 'unchanged', warnings: [] }; + // same anchor as the jsx primitives: no import of the package, no + // component to warn about, so there is nothing to parse for + if (pending.length === 0 || !source.includes(MARIGOLD_COMPONENTS)) { + return { kind: 'unchanged', warnings: [] }; + } return parseOr(source, file => { const warnings: string[] = []; const imported = new Set( diff --git a/packages/cli/src/lib/codemod/tokens.test.ts b/packages/cli/src/lib/codemod/tokens.test.ts index bef3283339..d39f8a05bd 100644 --- a/packages/cli/src/lib/codemod/tokens.test.ts +++ b/packages/cli/src/lib/codemod/tokens.test.ts @@ -20,6 +20,7 @@ describe('definedTokensIn', () => { } .x { color: var(--color-brand); } `; + expect(definedTokensIn(css)).toEqual(['brand', 'disabled-surface']); }); }); diff --git a/packages/cli/src/lib/codemod/types.ts b/packages/cli/src/lib/codemod/types.ts index f73c676d2c..6595821591 100644 --- a/packages/cli/src/lib/codemod/types.ts +++ b/packages/cli/src/lib/codemod/types.ts @@ -1,5 +1,11 @@ export interface Codemod { name: string; + /** + * User-facing one-liner, shown next to `name` in the interactive change + * selection. Required so it cannot drift away from the codemod it + * describes (a runner-side lookup by name silently degrades on a typo). + */ + description: string; apply: (source: string) => CodemodOutcome; } diff --git a/packages/cli/src/lib/edit-css.ts b/packages/cli/src/lib/edit-css.ts index 39dfc4758c..87d1bae4da 100644 --- a/packages/cli/src/lib/edit-css.ts +++ b/packages/cli/src/lib/edit-css.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import type { Framework } from './detect-project.js'; import { CSS_ENTRY_CANDIDATES, exists } from './fs-utils.js'; +import { escapeRegex } from './regex.js'; export type CssEditOutcome = | { kind: 'edited'; path: string; created: boolean; added: string[] } @@ -34,8 +35,6 @@ const computeSourcePath = (cssAbs: string, cwd: string): string => { return rel.startsWith('.') ? rel : `./${rel}`; }; -const escapeRegex = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const hasImport = (contents: string, target: string): boolean => new RegExp(`@import\\s+["']${escapeRegex(target)}["'];?`, 'm').test(contents); diff --git a/packages/cli/src/lib/regex.ts b/packages/cli/src/lib/regex.ts new file mode 100644 index 0000000000..15f13ec9d0 --- /dev/null +++ b/packages/cli/src/lib/regex.ts @@ -0,0 +1,3 @@ +/** escape a string for literal use inside a RegExp */ +export const escapeRegex = (s: string): string => + s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); From 4446028f77a56317619d1c2b11fec0183b08ed0c Mon Sep 17 00:00:00 2001 From: aromko Date: Mon, 27 Jul 2026 12:50:32 +0200 Subject: [PATCH 11/11] docs(DST-1543): state why the token scan runs in two complementary passes --- packages/cli/src/commands/migrate.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/migrate.ts b/packages/cli/src/commands/migrate.ts index 906f18192d..6d6dddadbd 100644 --- a/packages/cli/src/commands/migrate.ts +++ b/packages/cli/src/commands/migrate.ts @@ -303,7 +303,11 @@ export const runMigrate = (options: MigrateOptions): MigrateResult => { // pass 2.5: token references live anywhere, not only in @marigold // importers (own token CSS, generated CSS, plain components) — text-scan - // the files the pipeline did not see. + // the files the pipeline did not see. This is the exact complement of + // `sources`, so tightening that filter shifts files between the two passes + // and can never narrow token coverage. The scan stays inside the pipeline + // for the files it does cover, because there it runs on the post-edit text + // and so sees classes `swap-exact-classes` just introduced. for (const [file, text] of texts) { if (sources.has(file)) continue; const outcome = tokenUsage.apply(text);