Skip to content

fix(codemods): make the composite codemod runnable from a URL - #2942

Closed
mfal wants to merge 14 commits into
mainfrom
claude/flowalphall-codemod-error-89c0db
Closed

fix(codemods): make the composite codemod runnable from a URL#2942
mfal wants to merge 14 commits into
mainfrom
claude/flowalphall-codemod-error-89c0db

Conversation

@mfal

@mfal mfal commented Aug 26, 2026

Copy link
Copy Markdown
Member

What & why

flowAlphaAll (now flow1) has been broken for every consumer since it shipped in #2884:

Error: Cannot find module './flowAlphaActionPropToOnAction'
Require stack:
- /var/folders/.../T/jscodeshift-65239-mBfrbFIj29B6-.ts

Root cause. jscodeshift downloads a -t <url> transform into the OS temp
dir and requires it from there (Runner.js:182-193), so relative imports
resolve 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/transforms is what consumers fetch and must stay self-contained.
src/composites holds transforms that compose others, and
pnpm nx build codemods inlines a composite into a standalone, committed
bundle.

  • The composite is renamed to flow1 — named after where it takes you, the
    way flow020 already is. "All" is gone on purpose: of the 23 breaking-change
    entries in MIGRATION.md, 5 have a codemod, so the script is the mechanical
    part 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.
  • The bundler parses with jscodeshift itself and keeps types and JSDoc, so the
    generated file passes tsc, eslint and prettier like hand-written code
    (no ignore entries needed). It is strict on purpose: it knows the shape every
    transform has — one type-only jscodeshift import, one top-level
    declaration, one export default <identifier> — and throws on anything else
    instead of emitting a bundle that silently drops code.
  • nx wiring: build declares outputs and excludes its own output from
    inputs; test:unit depends on build and folds its hash in via
    dependentTasksOutputFiles, so affected:test regenerates and CI's
    git diff --exit-code catches 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.ts is the regression test: it copies each transform alone
into a temp directory and runs it, which is exactly how a URL-fetched transform
executes. Verified against the bug — 8 passed, flowAlphaAll failed.

One subtlety worth knowing when reading runTransform.ts: jscodeshift exits
0 even when its worker dies while loading the transform.
A test asserting on
the exit code would have stayed green. The helper asserts on the run summary
instead (0 errors and exactly one file processed).

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 flow1 chains 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 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.

Also in here

AccentBox codemod for alpha.786 (flowAlphaAccentBoxColorToBackgroundColor,
also part of flowAlphaAll). The entry existed without a transform.

This is not a rename — color changed meaning. It used to accept
"blue" | "green" | "gradient" | "neutral" and now accepts
"default" | "dark" | "light" | "dark-static" | "light-static", so a blanket
rename 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 other
literal 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 and
    foreground in new code.
  • An element that already carries backgroundColor — moving color there
    would overwrite the explicit value.

flow020.ts type error. Pre-existing, surfaced by the new test:compile
gate. Fixed with a String() cast; behaviour is unchanged.

Reviewer notes

  • packages/codemods/src/transforms/flow1.ts is generated — review
    src/composites/flow1.ts instead. It is ~650 lines of the diff.
  • New package docs in packages/codemods/AGENTS.md,
    plus a generated-artifact row and a common-failures row in the root
    AGENTS.md.
  • The runtime fallback in AccentBox.tsx maps neutral/gradient/green onto
    the background but not blue. That is intended, so
    <AccentBox color="blue"> renders neutral today, and the codemod moving it to
    backgroundColor="blue" brings the blue background back. That is the one case
    where running the codemod changes what you see; the migration entry says so.

Base branch & title

fix(codemods): → base main. No feature and no breaking change: the fix
restores a documented command that never worked, and the new codemod is
additive tooling in a private package.

Checklist

  • PR title is a Conventional Commit and matches the base branch above
  • pnpm lint is clean and pnpm affected:test passes (browser tests if
    behavior changed)
  • Generated code is committed (git diff is empty after the relevant
    build:* targets)
  • User-facing strings added to both de-DE and en-US locale files —
    n/a, no UI text
  • Docs updated if a public API changed; intentional visual changes get
    updated snapshots / the update-screenshots label — no visual change

`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>
@mfal
mfal requested a review from a team August 26, 2026 12:27
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for ./packages/components/

Status Category Percentage Covered / Total
🔵 Lines 76% 627 / 825
🔵 Statements 75.91% 643 / 847
🔵 Functions 77.95% 145 / 186
🔵 Branches 66.66% 298 / 447
File CoverageNo changed files found.
Generated in workflow #6325 for commit ac6a596 by the Vitest Coverage Report Action

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>
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

Preview environments are ready:

Type URL
docs pr-2942.docs.review.flow-components.de
storybook pr-2942.storybook.review.flow-components.de

Images:

  • docs: ghcr.io/mittwald/flow/docs:pr-2942
  • storybook: ghcr.io/mittwald/flow/storybook:pr-2942

mfal and others added 2 commits August 26, 2026 14:40
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>
@mfal mfal changed the title fix(codemods): make flowAlphaAll runnable from a URL fix(codemods): make the composite codemod runnable from a URL Aug 26, 2026
mfal and others added 8 commits August 26, 2026 15:04
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>
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>
@mfal
mfal enabled auto-merge (squash) August 26, 2026 14:01
mfal and others added 2 commits August 26, 2026 16:25
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>
@mfal
mfal marked this pull request as draft August 27, 2026 06:59
auto-merge was automatically disabled August 27, 2026 06:59

Pull request was converted to draft

@mfal mfal closed this Aug 27, 2026
mfal added a commit that referenced this pull request Aug 28, 2026
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>
mfal added a commit that referenced this pull request Aug 28, 2026
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>
@mfal

mfal commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Closed in favour of #2978, which supersedes this.

What was adopted there: the four transforms this PR added, the test harness (runTransform, the fixture and idempotency suites), remoteScope.ts and its derived remote-package check, and the migration prose this PR added to MIGRATION.md — in particular the AccentBox entry explaining that color was re-meant rather than renamed. All of it reviewed and green here, so re-deriving it would have been waste.

What was not: the composite bundler and src/composites. #2978 publishes @mittwald/flow-codemods as a CLI, which runs the codemods individually and reports per codemod, so bundling them into one self-contained transform has no remaining caller. The raw-GitHub-URL delivery path that made the bundler necessary is retired there, and the transform files are renamed to dashed catalogue ids.

That rename is why this PR was closed rather than merged: merging it to main afterwards would conflict with every renamed file.

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.

mfal added a commit that referenced this pull request Aug 31, 2026
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>
mfal added a commit that referenced this pull request Aug 31, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant