Skip to content

fix(tools-node): limit package dependency search scope#4154

Merged
tido64 merged 3 commits into
microsoft:mainfrom
vivekjm:fix-801-package-search-stop-dir
Jun 12, 2026
Merged

fix(tools-node): limit package dependency search scope#4154
tido64 merged 3 commits into
microsoft:mainfrom
vivekjm:fix-801-package-search-stop-dir

Conversation

@vivekjm

@vivekjm vivekjm commented May 16, 2026

Copy link
Copy Markdown
Contributor

Description

Resolves #801.

This adds a stopDir option to findPackageDependencyDir so callers can keep dependency lookup bounded to a repository root instead of walking all the way up the filesystem. The stop directory itself remains part of the search, so root-level node_modules is still found.

A regression test covers both cases: a dependency above stopDir is ignored, while a dependency at stopDir/node_modules is still resolved.

Test plan

  • PATH="$HOME/.nvm/versions/node/v22.22.0/bin:$PATH" yarn workspace @rnx-kit/tools-node test
  • PATH="$HOME/.nvm/versions/node/v22.22.0/bin:$PATH" yarn workspace @rnx-kit/tools-node build
  • PATH="$HOME/.nvm/versions/node/v22.22.0/bin:$PATH" yarn workspace @rnx-kit/tools-node lint
  • PATH="$HOME/.nvm/versions/node/v22.22.0/bin:$PATH" yarn change:check

Note: the default local Node v20.15.0 cannot run the repo test command because it lacks fs.globSync; the commands above were run with local Node v22.22.0.

Comment thread packages/tools-node/src/package.ts Outdated
@vivekjm vivekjm force-pushed the fix-801-package-search-stop-dir branch from db43dbf to 98283fc Compare May 22, 2026 12:19
Comment thread packages/tools-node/src/package.ts Outdated
@vivekjm

vivekjm commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Updated in 5918878. I reverted the incidental startDir defaulting change and left the stopAt pass-through/regression coverage intact.

Test note: I ran corepack yarn install --immutable, then attempted corepack yarn workspace @rnx-kit/tools-node test; the local run is blocked because this machine has Node v20.15.0 and the repo test command calls fs.globSync, which is unavailable there.

@tido64

tido64 commented Jun 9, 2026

Copy link
Copy Markdown
Member

Thanks! It looks like CI is failing on formatting. Can you run yarn format and push the changes?

Test note: I ran corepack yarn install --immutable, then attempted corepack yarn workspace @rnx-kit/tools-node test; the local run is blocked because this machine has Node v20.15.0 and the repo test command calls fs.globSync, which is unavailable there.

Is there a particular reason for why you can't use Node v22 or greater?

@vivekjm

vivekjm commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. I ran yarn format and pushed the formatting-only follow-up in 4d71149b.

I also reran the package test with Node 22.22.0 on PATH:

  • PATH="$HOME/.nvm/versions/node/v22.22.0/bin:$PATH" corepack yarn workspace @rnx-kit/tools-node test

That passed: 36 tests, 3 suites. The earlier local failure was because my default shell was still on Node 20.15.0, where the repo's test runner hits fs.globSync before it gets to the package tests.

@tido64 tido64 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@tido64 tido64 merged commit 95cadcf into microsoft:main Jun 12, 2026
11 of 12 checks passed
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.

Limit the scope of file and directory searching when resolving modules

2 participants