Skip to content

fix: resolve hoisted tsc binaries#944

Open
mrousavy wants to merge 1 commit into
callstack:mainfrom
mrousavy:codex/resolve-hoisted-tsc
Open

fix: resolve hoisted tsc binaries#944
mrousavy wants to merge 1 commit into
callstack:mainfrom
mrousavy:codex/resolve-hoisted-tsc

Conversation

@mrousavy
Copy link
Copy Markdown
Contributor

Summary

Resolve the TypeScript compiler binary by walking up ancestor node_modules/.bin directories before falling back to PATH.

This fixes package builds in workspaces where package dependencies are hoisted to the repository root, such as Bun workspaces. Previously Bob only checked <package>/node_modules/.bin/tsc, so a package with hoisted TypeScript could fail even though typescript was installed in the workspace.

Validation

  • PATH="/Users/mrousavy/.nvm/versions/node/v24.16.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Users/mrousavy/.codex/tmp/arg0/codex-arg0piPym5:/opt/homebrew/opt/binutils/bin:/Users/mrousavy/.nvm/versions/node/v20.19.4/bin:/Users/mrousavy/.rbenv/shims:/opt/homebrew/opt/openjdk@17/bin:/Users/mrousavy/Library/Android/sdk/emulator:/Users/mrousavy/Library/Android/sdk/tools:/Users/mrousavy/Library/Android/sdk/tools/bin:/Users/mrousavy/Library/Android/sdk/platform-tools:/Applications/Codex.app/Contents/Resources" yarn workspace react-native-builder-bob test run
  • PATH="/Users/mrousavy/.nvm/versions/node/v24.16.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Users/mrousavy/.codex/tmp/arg0/codex-arg0piPym5:/opt/homebrew/opt/binutils/bin:/Users/mrousavy/.nvm/versions/node/v20.19.4/bin:/Users/mrousavy/.rbenv/shims:/opt/homebrew/opt/openjdk@17/bin:/Users/mrousavy/Library/Android/sdk/emulator:/Users/mrousavy/Library/Android/sdk/tools:/Users/mrousavy/Library/Android/sdk/tools/bin:/Users/mrousavy/Library/Android/sdk/platform-tools:/Applications/Codex.app/Contents/Resources" yarn typecheck
  • PATH="/Users/mrousavy/.nvm/versions/node/v24.16.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Users/mrousavy/.codex/tmp/arg0/codex-arg0piPym5:/opt/homebrew/opt/binutils/bin:/Users/mrousavy/.nvm/versions/node/v20.19.4/bin:/Users/mrousavy/.rbenv/shims:/opt/homebrew/opt/openjdk@17/bin:/Users/mrousavy/Library/Android/sdk/emulator:/Users/mrousavy/Library/Android/sdk/tools:/Users/mrousavy/Library/Android/sdk/tools/bin:/Users/mrousavy/Library/Android/sdk/platform-tools:/Applications/Codex.app/Contents/Resources" yarn lint

@mrousavy mrousavy marked this pull request as ready for review May 28, 2026 13:54
Copy link
Copy Markdown
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR

Few concerns:

  • it'd be good to limit how much we traverse up, e.g., by checking the presence of a known lockfile to identify monorepo root
    • otherwise it may traverse up till ~/node_modules/.bin - which would be unexpected, especially because the warning isn't printed with such lookup
  • iirc most package managers add binaries to PATH
    • I'm curious why it's not getting picked up by which. it may be because typescript isn't specified in the library's devDependencies
    • in case it's not in the immediate node_modules/.bin and the package manager is adding it to PATH, the latter would be a more appropriate pick

I think the appropriate workflow is to add typescript to devDependencies of the package in your project, but we can add manual lookup as well, given:

  • lookup is bound to monorepo root (or we have a warning similar to the current which warning if it's found outside monorepo root)
  • optionally prioritize PATH over node_modules/.bin lookup (package manager knows better than manual lookup)
  • in both cases, we can identify monorepo root with the lockfile approach, and print a warning if the found path was outside so we have proper warning about incorrect setups
  • for tests, let's move it to __tests__ folder and use mock-fs (which is already present) if possible

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