-
Notifications
You must be signed in to change notification settings - Fork 142
fix: resolve Xcode Node.js detection issues with fnm/homebrew conflicts (#246) #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Prioritize Homebrew node over NVM/system node for build reliability - Add early exit when Homebrew node is found and working - Disable auto-switching for fnm/NVM in CI/Xcode environments - Add comprehensive error handling and fallback logic - Fix issue amantus-ai#246 where NVM was overriding Homebrew node selection Tested: Successfully switches from v24.2.0 to v24.3.0 during builds
Holy moly this looks complicated. By simply adding the various paths in order, wouldn't we achieve the same result with 10% code? Also the reviews here seem valid? |
Yeah, let me clean it up a bit |
@sforsethi I'll do a release later today, do you have time to work on this? Would love to get that in. |
@steipete Sure, I have some time right now I'll try to fix it, but tbh fully just vibe coding at the point, not familiar with the code |
- Remove complex early-exit logic and CI environment handling - Maintain priority order: Homebrew → Volta → fnm → NVM - Keep core functionality while eliminating 85% of code complexity - Address manager feedback about over-engineering
- Add missing NVM_DIR export before sourcing nvm.sh - Restore PATH priority after fnm/NVM modify it - Remove unnecessary comments - Maintain Homebrew → Volta → fnm → NVM priority order
- Eliminate duplicate PATH entries from multiple exports - Remove unused ORIGINAL_PATH variable - Use single PATH export with priority order - Reduce from 23 to 17 lines while maintaining functionality
- Use return instead of exit to avoid terminating parent shell - Set priority paths before loading fnm/NVM to preserve their modifications - Maintain proper priority order while respecting manager PATH changes
- Restore simple approach with clear Homebrew prioritization - Remove over-engineered error handling and validation - Maintain compatibility with zsh build system - Fix PATH priority by restoring Homebrew paths after version managers load - Reduce from 156 to 24 lines while solving original issue amantus-ai#246
- Add error suppression to NVM loading for consistency with fnm - Remove duplicate PATH exports to eliminate redundant entries - Maintain Homebrew priority with single PATH assignment
Tested: Successfully switches from v24.2.0 to v24.3.0 during builds