feat(fluent-cli): implement migrate v8-to-v9 command and skill#35836
Conversation
dmytrokirpa
force-pushed
the
experimental/fluent-cli-migariton-skill
branch
from
a16b033 to
ca012cb
Compare
|
Pull request demo site: URL |
dmytrokirpa
changed the title
Hotell
left a comment
Hotell
left a comment
There was a problem hiding this comment.
some questions/comments/actionables. nothing blocking
| Find all files using v8: | ||
| **Determine source root first:** | ||
|
|
||
| 1. Check `tsconfig.json` for `include` or `rootDir` to identify the source root (e.g., `src/`, `app/`, `lib/`) |
There was a problem hiding this comment.
we should probably start with finding package.json as source of truth for "project root". the tsconfig.json on it's own might be tricky if project uses solution configs etc.
so we can do this simple/naive check and then always instruct skill to verify with user if this is "correct" directory
for monorepo situation it might be even trickier ( it would be very easy if everyone used nx though )
| **Scope:** <files changed> files, <components migrated> component types migrated | ||
| **Assumptions logged:** <count> (see inline `// MIGRATION ASSUMPTION:` comments) | ||
| **Unresolved deltas:** <list or "none"> | ||
| **Shims still in place:** <list ShimComponent → target file — or "none"> |
There was a problem hiding this comment.
<list ...>, <count> etc -> is this some standardized "agent" output DSL or its just made up ? will it be deterministic agent to agent ?
|
|
||
| **Phase order:** check for annotations (+ tsc baseline) → if missing, ask user to run CLI → process annotations (`auto` → `scaffold` → `manual` → `no-equivalent`) → validate → report (Output Template above) | ||
|
|
||
| **Baseline TypeScript check before starting:** run the repo's TypeScript check command (`tsc --noEmit` or the equivalent in `package.json` scripts) before any changes and record the error count. In the Validation step, report TypeScript as ✅ if post-migration error count ≤ baseline — do not block on pre-existing errors. |
There was a problem hiding this comment.
this might be very slow especially in monorepos not set properly, we need to come up with some more sofisticated approach probably. ( we could leverage the output of report or stats command in some way which could do the repo/monorepo exploration in future )
| Process the `@fluent-migrate:` annotations as a work queue. Get all annotations: | ||
|
|
||
| ```sh | ||
| grep -rn "@fluent-migrate:" <SOURCE_ROOT> --include="*.ts" --include="*.tsx" |
There was a problem hiding this comment.
I think we can create metadata.json when running the annotation parser/writer so this can be avoided and be deterministic. the metadata would contain all the files paths where annotations exist. wdyt ?
|
|
||
| ### Step 4 — Validate | ||
|
|
||
| Run the host repo's own commands. Do not assume `npx tsc` or `npm test` — check `package.json` scripts first. |
There was a problem hiding this comment.
this is better than agent execution contract chapter and should be aligned https://github.com/microsoft/fluentui/pull/35836/changes#r2888486865
| command: 'migrate <migration>', | ||
| describe: 'Run migration analysis and annotation', | ||
| builder: yargs => | ||
| yargs.command(v8ToV9Command).demandCommand(1, 'Specify a migration type: v8-to-v9').version(false).help(), |
There was a problem hiding this comment.
'Specify a migration type: <migration-id>' this should be more genericwhen we add another migration right ?
we also want migrate --list to report all available migrations
| @@ -0,0 +1,92 @@ | |||
| import type { CommandHandler } from '../../../utils/types'; | |||
| import { analyzeFiles } from './utils/annotator'; | |||
There was a problem hiding this comment.
nit: lets make stronger public api contract. ./utils/annotator only imports. or is there any reason why we import from nested annotator files
|
|
||
| for (const sourceFile of project.getSourceFiles()) { | ||
| // Skip files with no @fluentui/react imports (already migrated or unrelated) | ||
| const hasFluentImport = sourceFile |
There was a problem hiding this comment.
we can probably use getFluentImportNames here and pass those names to the rules to avoid repetitive work per rule
|
|
||
| expect(results).toHaveLength(1); | ||
| const importAnnotations = results[0].annotations.filter(a => a.codemod === 'import-paths'); | ||
| expect(importAnnotations.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
lets go with inline snapshots and more narrowed assertion in general to provide check for whole output (human/machine) readable
| expect(importAnnotations.length).toBeGreaterThan(0); | |
| expect(results[0].filePath).toContain('import-paths.tsx'); | |
| expect(importAnnotations).toMatchInlineSnapshot(); |
| if (tagName === 'IconButton') { | ||
| results.push({ | ||
| action: 'auto', | ||
| codemod: 'button-variants', |
There was a problem hiding this comment.
am I missing something or it's not part of this PR -> how will these codemod ids map to the skill ?
| @@ -0,0 +1,58 @@ | |||
| import type { SourceFile, Node } from 'ts-morph'; | |||
| import { SyntaxKind } from 'ts-morph'; | |||
There was a problem hiding this comment.
this is "scary" , as we discussed for now ts morph is "ok" but we should revisit once we ship
ts-morph
tsquery
dmytrokirpa
merged commit e568178
into
microsoft:experimental/fluent-cli
2 participants
This pull request significantly enhances the migration guide for Fluent UI React v8 to v9 by introducing a structured agent workflow, an output reporting template, and annotation-driven migration steps. The changes clarify agent responsibilities, enforce a validation checklist, and ensure migration progress is tracked and reported consistently.
Agent workflow and reporting:
Annotation-driven migration process:
auto,scaffold,manual, andno-equivalentannotations, and how to verify all annotations are resolved before validating.Validation and readiness checks:
Visual and accessibility troubleshooting: