Skip to content

JavaScript: setupSharedDependencies must search inside recipe directory itself#7515

Merged
Jenson3210 merged 1 commit intomainfrom
Jenson3210/shared-deps-dir-target
Apr 29, 2026
Merged

JavaScript: setupSharedDependencies must search inside recipe directory itself#7515
Jenson3210 merged 1 commit intomainfrom
Jenson3210/shared-deps-dir-target

Conversation

@Jenson3210
Copy link
Copy Markdown
Contributor

@Jenson3210 Jenson3210 commented Apr 29, 2026

Why

Recipe authoring loop: edit, register the local path with mod config recipes npm install /path/to/my-recipes, run against a fixture before publishing.

Today this silently no-ops on package.json for any ScanningRecipe (UpgradeDependencyVersion, AddDependency, etc.). setupSharedDependencies does path.dirname(targetModulePath), which for a directory path jumps to the parent — the upward node_modules walk never enters the recipe's own node_modules. The host→target require.cache mapping is skipped, the recipe loads its own @openrewrite/rewrite instance, recipe instanceof ScanningRecipe returns false in visit.ts, and the scan: phase is skipped. Editor still fires, but acc.projectsToUpdate is always empty.

npm install <pkg@version> is unaffected because require.resolve returns a file path, and dirname/walk-up lands on the install dir's hoisted node_modules.

Fix

When targetModulePath is a directory, start the walk from the directory itself.

Test plan

  • vitest run — 1821/1821 pass
  • Manual: cemex UpgradeTo18 now bumps package.json end-to-end. rpc.log changes from Could not find @openrewrite/rewrite in target's node_modules to Mapped 132 modules for @openrewrite/rewrite.
  • CI green

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Apr 29, 2026
@Jenson3210 Jenson3210 closed this Apr 29, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Apr 29, 2026
@Jenson3210 Jenson3210 deleted the Jenson3210/shared-deps-dir-target branch April 29, 2026 07:14
@Jenson3210 Jenson3210 restored the Jenson3210/shared-deps-dir-target branch April 29, 2026 07:15
@Jenson3210 Jenson3210 reopened this Apr 29, 2026
@github-project-automation github-project-automation Bot moved this from Done to In Progress in OpenRewrite Apr 29, 2026
@Jenson3210 Jenson3210 marked this pull request as draft April 29, 2026 07:18
…ectory itself

When a recipe is installed by local path (e.g.
`mod config recipes npm install /path/to/my-recipes`), `targetModulePath`
is the package directory itself. `path.dirname(targetModulePath)` jumps
to the parent and the upward `node_modules` walk never enters the
package's own `node_modules`, so the target package root for shared
dependencies (`@openrewrite/rewrite`, `vscode-jsonrpc`, `mutative`) is
never found and the host→target require.cache mapping is skipped.

The downstream symptom is the dual-package problem: the recipe ends up
with its own `@openrewrite/rewrite` instance, distinct from the host's,
so `recipe instanceof ScanningRecipe` in `rpc/request/visit.ts` returns
`false`. The runtime then skips the `scan:` phase entirely. Editors are
still invoked (~3000 times per cemex run in our reproducer), but every
accumulator is empty, so `UpgradeDependencyVersion` silently returns
`doc` from `package.json` and no version bumps land — even though the
install in the temp dir would have succeeded.

When `targetModulePath` resolves to a directory, start the
`node_modules` walk from the directory itself; only fall back to
`path.dirname` when it resolves to a file.

Repro: any local-path-installed recipe set whose `package.json`
declares `@openrewrite/rewrite` as a dependency. Before this change,
rpc.log shows `Could not find @openrewrite/rewrite in target's
node_modules`. After, it shows `Mapped <N> modules for
@openrewrite/rewrite`.
@Jenson3210 Jenson3210 force-pushed the Jenson3210/shared-deps-dir-target branch from 42137ba to 47b6516 Compare April 29, 2026 07:24
@Jenson3210 Jenson3210 marked this pull request as ready for review April 29, 2026 07:26
@github-project-automation github-project-automation Bot moved this from In Progress to Ready to Review in OpenRewrite Apr 29, 2026
@Jenson3210 Jenson3210 merged commit aef30f5 into main Apr 29, 2026
1 check passed
@Jenson3210 Jenson3210 deleted the Jenson3210/shared-deps-dir-target branch April 29, 2026 10:03
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants