Skip to content

fix(codemods): install with the project's real package manager - #3041

Merged
mfal merged 3 commits into
mainfrom
claude/codemods-package-manager
Sep 1, 2026
Merged

mfal merged 3 commits into
mainfrom
claude/codemods-package-manager

Conversation

@mfal

@mfal mfal commented Aug 31, 2026 •

Copy link
Copy Markdown
Member

upgrade detected a package manager lockfiles-in-cwd-only, and the execution had
three further holes. All of it is live in the published latest — the upgrade
CLI shipped in 1.1.0 — so this targets main as a fix, not next.

Implements the approved design in
2026-08-31-upgrade-cli-package-manager-design.md.

The install

The reported failure: a Yarn 4 workspace package. It has no lockfile of its
own, so detection fell through to npm and ran npm install on a workspace:*
manifest — EUNSUPPORTEDPROTOCOL, after the bump was already written.

package-manager-detector@1.8.0 (MIT, zero dependencies) does detection and
command resolution; execution stays ours, because that is where the test seams,
the frozen-lockfile quirks and the Windows handling live. Its behaviour was
verified against the installed package rather than from memory: strategies apply
in order per directory walking up, version is the string "berry" for Yarn 2+,
and its "install" is the plain install for every agent (the frozen variants are
a separate command there).

  • Detection walks up, and covers packageManager,
    devEngines.packageManager and the node_modules install metadata as well as
    lockfiles. npm stays the fallback.
  • A packageManager pin is honoured. Binary satisfies it → run directly;
    mismatch or missing → corepack with COREPACK_ENABLE_DOWNLOAD_PROMPT=0; no
    corepack → refuse before installing, naming the pin, the found version and
    the install page. Checked rather than always-corepack because corepack ignores
    PATH and downloads into its own cache, which would hit offline CI. The check
    is semver.satisfies, not a string compare: pnpm@8 yields the pin "8"
    against a reported 8.15.0.
  • shell on win32. Node has refused to spawn .cmd without a shell since
    2024, so every non-npm manager died with ENOENT there.
  • The log names agent, pin and the command actually run, so a wrong detection
    is visible instead of silent. The failure message also names the detected
    manager and says the bump itself is already correct.

The commands it prints

resolveInvoke gives every printed command the right prefix — npx (npm and
Yarn Classic, which has no dlx), pnpm dlx, yarn dlx, bun x — and
displaySourcePath gives it the right path. list used to hardcode src, so on
a project whose sources live elsewhere, pasting the printed line ran the codemod
against a directory that does not exist, and the failing run then blamed the path.

Verified end to end from a nested package of a pnpm workspace: list prints
pnpm dlx @mittwald/flow-codemods@latest <id> . — manager from the workspace
root, path from the package.

Accepted trade: the bare list now reads lockfiles and package.json up the
tree, so "reads no manifest" is gone from the README. It still hits no network. A
command a reader can paste beats that claim. MIGRATION.md stays on npx — it is
generated and cannot know the reader's manager — but now says so and names the
equivalents.

Scope

The two catalogue findings from the same upgrade run that used to sit here have
moved to their own PR against main: #3047. What is left is the
CLI work, which is why the base and the type changed.

Verification

  • 307 unit tests pass, test:compile clean, pnpm lint clean (0 errors,
    prettier green), pnpm nx build codemods regenerates nothing further
  • install.test.ts covers the monorepo walk-up, the Yarn-4-as-berry distinction,
    devEngines, the pin tree with a stubbed probe (match → direct, mismatch →
    corepack, no corepack → throws naming both versions), and that "berry" is
    never treated as a pin
  • Test-merged with fix(codemods): codemods for two entries the catalogue called manual #3047: no conflict, and the merged tree
    regenerates MIGRATION.md byte-identically

Follow-up

Two sentences in the versioning-page prompt (#3038) become redundant
once this ships: the note that list hardcodes src, and the warning that
package-manager detection is directory-local. Both stay harmless — they are
defensive advice, not claims that become wrong — and they are correct for every
currently published version. Worth trimming in a follow-up.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

Preview environments are ready:

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

Images:

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

@mfal
mfal force-pushed the claude/codemods-package-manager branch from e1492b2 to 44fe599 Compare August 31, 2026 16:03
@mfal
mfal marked this pull request as ready for review August 31, 2026 16:27
@mfal
mfal requested a review from a team August 31, 2026 16:27
mfal and others added 2 commits September 1, 2026 11:26
`list` printed a runnable command per codemod entry with the path argument fixed
to `src`. On a project whose sources are anywhere else, pasting that line runs
the codemod against a directory that does not exist — and the run then reports
"no files under <path> were processed. Is the path right?", sending the reader
after a path the tool handed them.

`displaySourcePath` sits beside `resolveSourcePath` and makes the same choice in
the form a reader would type: an explicit `--path` verbatim, else `src` when it
exists, else `.` — never an omitted argument, which would default back to `src`
and pick the wrong tree. `renderList` takes it as `path`, and both call sites
fill it: `list` from `--path` and the real cwd, `upgrade` with the path that run
actually used, so a command copied out of the by-hand block works.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 8d1a0b9)
(cherry picked from commit f3c77b6)
`upgrade` detected a package manager lockfiles-in-cwd-only, and the execution
had three further holes. All of it is live in the published `latest` — the
upgrade CLI shipped in 1.1.0 — so this is a fix on `main`, not a feature.
Implements the approved design in
`2026-08-31-upgrade-cli-package-manager-design.md`.

The reported failure: a Yarn 4 workspace package. It has no lockfile of its own,
so detection fell through to npm and ran `npm install` on a `workspace:*`
manifest — `EUNSUPPORTEDPROTOCOL`, after the bump was already written.

`package-manager-detector@1.8.0` (MIT, zero dependencies) does detection and
command resolution; execution stays ours, because that is where the test seams,
the frozen-lockfile quirks and the Windows handling live. Verified its behaviour
against the installed package rather than from memory: strategies apply in order
per directory walking up, `version` is the string `"berry"` for Yarn 2+, and its
`"install"` is the plain install for every agent.

- Detection walks up, and now covers `packageManager`,
  `devEngines.packageManager` and the `node_modules` install metadata as well as
  lockfiles. npm stays the fallback.
- A `packageManager` pin is honoured: binary satisfies it → run directly;
  mismatch or missing → corepack with `COREPACK_ENABLE_DOWNLOAD_PROMPT=0`; no
  corepack → refuse *before* installing, naming the pin, the found version and
  the install page. Checked rather than always-corepack because corepack ignores
  PATH and downloads into its own cache, which would hit offline CI. The check is
  `semver.satisfies`, not a string compare: `pnpm@8` yields the pin `"8"` against
  a reported `8.15.0`.
- `shell` on win32. Node has refused to spawn `.cmd` without a shell since 2024,
  so every non-npm manager died with ENOENT there.
- The install log names agent, pin and the command actually run, so a wrong
  detection is visible. `install` returns that description; the failure message
  also names the detected manager and says the bump itself is already correct.
- `resolveInvoke` gives the printed commands the right prefix — `npx` (npm and
  Yarn Classic, which has no `dlx`), `pnpm dlx`, `yarn dlx`, `bun x`. Verified
  end to end: from a nested package of a pnpm workspace, `list` prints
  `pnpm dlx … <id> .`, picking up both the manager from the root and the path
  from the package.

Consequence, accepted deliberately: the bare `list` now reads lockfiles and
`package.json` up the tree, so "reads no manifest" is gone from the README. It
still hits no network. A command a reader can paste beats that claim.

`MIGRATION.md` stays on `npx` — it is generated and cannot know the reader's
manager — but now says so and names the equivalents.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mfal mfal changed the title feat(codemods): install with the project's real package manager fix(codemods): install with the project's real package manager Sep 1, 2026
@mfal
mfal changed the base branch from next to main September 1, 2026 09:30
@mfal
mfal force-pushed the claude/codemods-package-manager branch from eb49e1b to 112bea7 Compare September 1, 2026 09:38
@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report for ./packages/components/

Status Category Percentage Covered / Total
🔵 Lines 78.69% 746 / 948
🔵 Statements 78.57% 763 / 971
🔵 Functions 80.09% 165 / 206
🔵 Branches 70.33% 377 / 536
File CoverageNo changed files found.
Generated in workflow #6493 for commit ab8f7e1 by the Vitest Coverage Report Action

The cherry-pick from the `next`-based branch carried `next`'s release bump with
it — `1.2.0-next.0` against a `lerna.json` that says `1.1.3`. The `package.json`
merge driver only runs on merges, not on `cherry-pick`, so nothing caught it
locally either: `pnpm lint` does not include the version guard, which is its own
CI step.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mfal
mfal enabled auto-merge (squash) September 1, 2026 11:29
@mfal
mfal requested a review from Lisa18289 September 1, 2026 11:30
@mfal
mfal merged commit 63875ae into main Sep 1, 2026
32 of 35 checks passed
@mfal
mfal deleted the claude/codemods-package-manager branch September 1, 2026 11:59
mfal added a commit that referenced this pull request Sep 2, 2026
The entry said `action: manual` while being the same shape as
`password-tools-subpath-renamed`, which ships a codemod — one exact module
specifier, swapped. Two entries with the same shape and different `action`
values is hard to explain to a reader budgeting manual review.

Modelled on that neighbour: every JS/TS form that names a module, and an exact
match rather than a prefix, since `./styles` had nothing underneath it (so
`all-layered.css` beside it is untouched). What it cannot reach is an `@import`
in a `.css`/`.scss`, and `apply` says so rather than implying full coverage.

`catalog.test.ts` named `renamed-css-export` as its example of a manual entry;
adding the codemod broke a test that was only checking that the parser reads a
non-codemod `action`. Made it id-independent — the correspondence between
`action` and the transform file is enforced separately, over every entry.

Reported from a real upgrade run. Extracted from #3041, which
targets `next` for its package-manager work; this one corrects a catalogue
entry that is already published, so it belongs on `main`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mfal added a commit that referenced this pull request Sep 2, 2026
The entry said `action: manual` while its change is mechanically decidable for
the cases the source spells out. `accent-box-color-to-background-color` is the
template: ship a codemod for the decidable subset and say in `apply` what it
will not touch.

Two changes with different decidability, and the transform treats them
differently:

- `maxWidth` always goes. The prop was removed from the type, so an explicit
  attribute is wrong at any value — `maxWidth={computed}` included. Decidable
  without reading the value.
- `width`/`minWidth` go only where the source literally says `null`, which is
  what "no explicit width" is spelled as now. `width={maybeNull}` could be
  anything at runtime and is left alone, exactly as
  `accent-box-color-to-background-color` leaves `color={expression}`.

Scoped like its neighbours: only elements resolving to `TableColumn` through a
Flow import, named, aliased or namespace, subpath entries included. A spread
that might carry `maxWidth` is invisible and stays — `apply` names both gaps
rather than implying full coverage.

Reported from a real upgrade run. Extracted from #3041 for the
same reason as the entry before it: the catalogue entry is already published.

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.

2 participants