fix(codemods): make the composite codemod runnable from a URL - #2942
fix(codemods): make the composite codemod runnable from a URL#2942mfal wants to merge 14 commits into
Conversation
`flowAlphaAll` died with `Cannot find module './flowAlphaActionPropToOnAction'` for every consumer since it shipped in #2884. jscodeshift downloads a `-t <url>` transform into the OS temp dir and requires it from there, so relative imports resolve against that temp dir, where no sibling file exists. `flowAlphaAll` was the only transform importing its siblings, so it was the only broken one. Split the package: `src/transforms` is what consumers fetch and stays self-contained, `src/composites` holds transforms that compose others, and `pnpm nx build codemods` inlines a composite into a standalone, committed bundle. The documented URL is unchanged, so the call in already-published MIGRATION.md copies starts working. The bundler parses with jscodeshift itself and keeps types and JSDoc, so the generated file passes tsc, eslint and prettier like hand-written code. It is strict on purpose: it knows the shape every transform has and throws on anything else instead of emitting a bundle that silently drops code. Tests run the real jscodeshift CLI on a temp copy of the transform, which is the consumer's path. `standalone.test.ts` would have caught this. Note that jscodeshift exits 0 even when its worker dies while loading, so the helper asserts on the run summary rather than the exit code. Also adds the AccentBox codemod for alpha.786, which had a migration entry but no transform. `color` changed meaning rather than being renamed, so the transform decides per value: a value from the new foreground union stays on `color`, every other literal moves to `backgroundColor`. Dynamic values and elements that already carry `backgroundColor` are left for a human — neither is decidable from the source. Fixes a latent type error in `flow020.ts` that the new `test:compile` gate surfaced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage Report for ./packages/components/
File CoverageNo changed files found. |
The runtime fallback maps "neutral", "gradient" and "green" onto the background but not "blue", so `<AccentBox color="blue">` renders neutral today. The codemod moves the value to `backgroundColor`, which brings the blue background back — the one case where running it changes what you see. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🚀 Preview DeploymentPreview environments are ready:
Images:
|
An idempotency test over every transform turned up two defects in
flowAlphaAlignToCombine, both producing code the consumer has to repair by
hand.
`import { Align as Row }` was never rewritten, although the transform documents
that an alias is kept and only the imported name changes. The early return
counted non-aliased renames only, so an alias-only file looked untouched and
the mutation was discarded. `Align` no longer exists, so the consumer was left
with a broken import.
A file importing both `Align` and `Combine` ended up with
`import { Combine, Combine }`, which does not parse — the first run produced a
file no second run could read. Colliding specifiers now collapse onto one, and
a value import wins over a type-only one so nothing loses its runtime binding.
All eight transforms are idempotent, asserted over a fixture per transform that
carries the cases most likely to break it: values the transform must leave
alone, elements it already migrated, and imports it has already rewritten. A
transform without a fixture fails the suite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Name the composite after where it takes you, not where you come from — which is also how `flow020` is named. The old name described the source series and told a consumer nothing about the destination. Dropping "all" is part of the point: the codemod is not the whole migration. Of the 23 breaking-change entries in MIGRATION.md, 5 have a codemod; the rest need a hand. Both the guide and the transform's own doc now say so instead of implying the script finishes the job. The old URL was never functional — it failed with MODULE_NOT_FOUND for every consumer since it shipped — so no working command changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MutedActionError, the password-tools rule classes and the Button props interfaces were all removed without an alias, so a codebase still using them does not compile. Of the 23 breaking-change entries in MIGRATION.md these are the ones a script can do in full. - `MutedActionError` -> `AbortActionError`, including both static helpers and an `error.name === "MutedActionError"` comparison. A bare occurrence of the string is left alone; only a comparison identifies it as the error's name. - `AsyncRule` / `SyncRule` -> `Rule`, scoped to the mittwald-password-tools-js entry. - `RemoteButtonElementProps` / `ResetButtonProps` / `SubmitButtonProps` -> `ButtonProps`. This one is more than a rename: the old names came from the react-hook-form entry, which does not export `ButtonProps`, so the specifier moves to the package root instead of swapping one import error for another. Collapsing several names onto one collides with a name the file already binds, across import declarations as well as within one. Such a specifier now goes away instead of producing a duplicate declaration that does not parse, and a declaration left without specifiers is removed rather than decaying into a side-effect import. flowAlphaAlignToCombine had the same gap across declarations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…demod-error-89c0db
The `merge=package-json` driver keeps "our" version field, which is right for the forward-merge cascade (`main -> next`) but wrong when `main` is merged into a branch off it: the 1.0.1 release bump was reverted to 1.0.0 while the package's own CHANGELOG already recorded 1.0.1. Every other package is on 1.0.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each entry gets the ready-made call, plus what the codemod cannot do: the AbortActionError one only recognises an `error.name` comparison in a file that imports the class, and the ButtonProps one has to move the import to the package root because the react-hook-form entry does not export `ButtonProps`. A test now checks both directions between the guides and the transforms: every URL a guide names resolves to a transform that exists, and every transform is reachable from a guide. A dead URL is the same failure for a consumer as a transform that cannot run — the command dies instead of migrating anything. `flowRemote` is exempt and named as such: it ports an app to the remote package rather than migrating a version, so it has no entry in the migration guide, and no consumer-facing home anywhere else either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… package `@mittwald/flow-remote-react-components` exports no component prop types — not one. Moving a remote import onto `ButtonProps` there produced an import of a name that does not exist, which is the failure the codemod is supposed to remove. `RemoteButtonElementProps` is dropped from the transform for the same reason, plus one of its own: `@mittwald/flow-remote-elements` still exports that name today, so rewriting it would break working code. The migration entry says so, since a remote codebase has to pick its own source for the type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The remote package mirrors the component API, so most codemods apply there — which is exactly why the exceptions get missed. It exports the @flr-generate components and nothing else: no prop types, no error classes, three entries against the main package's nine. A test now reads that export surface and the exports maps, and holds every transform to one rule: it may claim the remote package exactly when something it targets exists there. It fails on a transform that claims it without a reachable target, on one that scopes itself to an entry no consumer can import, and on a transform missing from the list. That turned up three scopes that could never match, now removed: - `Action` is not remote-capable — the remote package has `ActionGroup`. - The remote package exports no error classes, so MutedActionError had nothing to find. - There is no `mittwald-password-tools-js` entry on the remote package at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit read the remote package's surface as "the generated components plus react-hook-form" and drew two wrong conclusions from it. The remote package also re-exports the main package's whole `flr-universal` entry through `FlowRemoteUniversal.ts`, which carries `Action`, `Modal`, the overlay hooks and their prop types — 317 names in total, not 142. So `flowAlphaActionPropToOnAction` gets its remote scope back: `Action` is not a generated remote component, but it is reachable, and dropping the scope silently stopped the codemod from migrating remote code. The scope resolver now follows a workspace package specifier, not only `@/` and `./` — that one gap is what hid the universal surface, since the remote package pulls it in by package name. `ButtonProps` stays out of the remote scope, but for the right reason: the remote package does export prop types, just not that one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`codemods:test:unit` went red on CI with "Test timed out in 5000ms" while passing locally. Nothing is wrong with the transform: every test spawns the real jscodeshift CLI, which is a Node cold start plus babel per run and twice per case in the idempotency suite. That is deliberate — it reproduces how a consumer runs a transform, and a faster in-process call would not — but it makes the default 5s far too tight on a runner, where one case took 5.2s. Not switching the CLI to `--run-in-band` to speed it up: the worker process is exactly what makes a transform with a relative import fail the way it fails for a consumer, which is the behaviour under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…demod-error-89c0db
Second time on this branch, same cause: `merge=package-json` keeps "our" version field, which is right for the forward-merge cascade but wrong when `main` is merged into a branch off it. The package's own CHANGELOG already recorded 1.0.2 and every other package is on 1.0.2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pull request was converted to draft
Two changes after review of the first draft. PR #2942 is no longer a prerequisite. Task 0 pulls its useful parts onto this branch — the four added transforms, the rewritten flowAlphaAlignToCombine (+155/-38 for alias and namespace resolution), the test harness, remoteScope, and the migration prose it added to MIGRATION.md — and deliberately leaves the composite bundler behind. It supersedes #2942, which should be closed rather than merged: merging it to main afterwards would conflict with every rename. Task 15 adds the deprecated APIs that have no MIGRATION.md entry today. The first draft silently delivered only half the agreed "breaking plus deprecations" scope: all 22 ported entries come from the existing guide and only two are deprecations, both already documented. Without Task 15, `upgrade` on the 1.x line still finds almost nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pulls reviewed, CI-green work from PR #2942 onto this branch: the runTransform test harness, the remoteScope authority, nine transforms (four new, three improved), and package configs. Drops the broken flowAlphaAll composite and everything that exists only to serve it (the bundler scripts, flow1.ts, and the standalone/bundledComposites/ documented test suites) — a later task replaces the composite mechanism. remoteScope.test.ts, idempotency.test.ts and transforms.test.ts had their flow1-specific fixtures, targets and comments removed accordingly so the hand-maintained lists match what is actually on disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closed in favour of #2978, which supersedes this. What was adopted there: the four transforms this PR added, the test harness ( What was not: the composite bundler and That rename is why this PR was closed rather than merged: merging it to The bug this PR fixed is fixed there too — by construction rather than by bundling. The CLI hands jscodeshift a path inside the installed package, so a transform is never downloaded into a temp directory and can never fail to resolve a sibling import. |
Two changes after review of the first draft. PR #2942 is no longer a prerequisite. Task 0 pulls its useful parts onto this branch — the four added transforms, the rewritten flowAlphaAlignToCombine (+155/-38 for alias and namespace resolution), the test harness, remoteScope, and the migration prose it added to MIGRATION.md — and deliberately leaves the composite bundler behind. It supersedes #2942, which should be closed rather than merged: merging it to main afterwards would conflict with every rename. Task 15 adds the deprecated APIs that have no MIGRATION.md entry today. The first draft silently delivered only half the agreed "breaking plus deprecations" scope: all 22 ported entries come from the existing guide and only two are deprecations, both already documented. Without Task 15, `upgrade` on the 1.x line still finds almost nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pulls reviewed, CI-green work from PR #2942 onto this branch: the runTransform test harness, the remoteScope authority, nine transforms (four new, three improved), and package configs. Drops the broken flowAlphaAll composite and everything that exists only to serve it (the bundler scripts, flow1.ts, and the standalone/bundledComposites/ documented test suites) — a later task replaces the composite mechanism. remoteScope.test.ts, idempotency.test.ts and transforms.test.ts had their flow1-specific fixtures, targets and comments removed accordingly so the hand-maintained lists match what is actually on disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What & why
flowAlphaAll(nowflow1) has been broken for every consumer since it shipped in #2884:Root cause. jscodeshift downloads a
-t <url>transform into the OS tempdir and requires it from there (
Runner.js:182-193), so relative importsresolve against that temp dir, where no sibling file exists. It was
the only transform importing its siblings, so it was the only broken one — the
individual codemods all work.
The fix
src/transformsis what consumers fetch and must stay self-contained.src/compositesholds transforms that compose others, andpnpm nx build codemodsinlines a composite into a standalone, committedbundle.
flow1— named after where it takes you, theway
flow020already is. "All" is gone on purpose: of the 23 breaking-changeentries in
MIGRATION.md, 5 have a codemod, so the script is the mechanicalpart of the migration, not the whole of it. The guide and the transform's own
doc now say so. The old URL never worked, so no functioning command changes.
generated file passes
tsc,eslintandprettierlike hand-written code(no ignore entries needed). It is strict on purpose: it knows the shape every
transform has — one type-only
jscodeshiftimport, one top-leveldeclaration, one
export default <identifier>— and throws on anything elseinstead of emitting a bundle that silently drops code.
builddeclaresoutputsand excludes its own output frominputs;test:unitdepends onbuildand folds its hash in viadependentTasksOutputFiles, soaffected:testregenerates and CI'sgit diff --exit-codecatches a stale bundle.Tests
The package had none. It now has 20, all running the real jscodeshift CLI on
a temp copy of the transform — the consumer's path, not just the exported
function.
standalone.test.tsis the regression test: it copies each transform aloneinto a temp directory and runs it, which is exactly how a URL-fetched transform
executes. Verified against the bug — 8 passed,
flowAlphaAllfailed.Two more defects, found by the idempotency test
All eight transforms are now asserted to be idempotent — a second run must be a
no-op, which matters because
flow1chains five of them over the same source.Writing that test turned up two pre-existing defects in
flowAlphaAlignToCombine, both shipping broken code to the consumer:import { Align as Row }was never rewritten, although the transformdocuments that an alias is kept and only the imported name changes. The early
return counted non-aliased renames only, so an alias-only file looked
untouched and the mutation was discarded.
Alignno longer exists, so theconsumer was left with a broken import.
AlignandCombineended up withimport { Combine, Combine }, which does not parse — the first run produced afile no second run could read. Colliding specifiers now collapse onto one, and
a value import wins over a type-only one so nothing loses its runtime binding.
Also in here
AccentBox codemod for alpha.786 (
flowAlphaAccentBoxColorToBackgroundColor,also part of
flowAlphaAll). The entry existed without a transform.This is not a rename —
colorchanged meaning. It used to accept"blue" | "green" | "gradient" | "neutral"and now accepts"default" | "dark" | "light" | "dark-static" | "light-static", so a blanketrename would break every element already on the new API. The transform decides
per value: a value from the new foreground union stays on
color, every otherliteral moves to
backgroundColor.Two cases are deliberately left for a human, because neither is decidable from
the source — both are documented in the migration entry:
color={expression}— the same expression means background in old code andforeground in new code.
backgroundColor— movingcolortherewould overwrite the explicit value.
flow020.tstype error. Pre-existing, surfaced by the newtest:compilegate. Fixed with a
String()cast; behaviour is unchanged.Reviewer notes
packages/codemods/src/transforms/flow1.tsis generated — reviewsrc/composites/flow1.tsinstead. It is ~650 lines of the diff.packages/codemods/AGENTS.md,plus a generated-artifact row and a common-failures row in the root
AGENTS.md.AccentBox.tsxmapsneutral/gradient/greenontothe background but not
blue. That is intended, so<AccentBox color="blue">renders neutral today, and the codemod moving it tobackgroundColor="blue"brings the blue background back. That is the one casewhere running the codemod changes what you see; the migration entry says so.
Base branch & title
fix(codemods):→ basemain. No feature and no breaking change: the fixrestores a documented command that never worked, and the new codemod is
additive tooling in a private package.
Checklist
pnpm lintis clean andpnpm affected:testpasses (browser tests ifbehavior changed)
git diffis empty after the relevantbuild:*targets)de-DEanden-USlocale files —n/a, no UI text
updated snapshots / the
update-screenshotslabel — no visual change