fix: make pho alias work across all package managers#19
Conversation
The old approach matched `node_modules` in process.argv[1] and walked hardcoded `..` levels to find the bin directory. This broke under `bun link` (Bun fully resolves the symlink chain, stripping node_modules from argv[1]) and assumed a scoped-package layout. Replace with a realpath-based approach: scan $PATH plus well-known global bin fallbacks for a `photon` entry whose realpath matches ours, then drop `pho` next to it. Package-manager agnostic with no hardcoded directory layouts. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{html,ts,tsx,css}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{html,ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe PR refactors ChangesPhoton Alias Installation Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the runtime pho alias installer to work across more package-manager layouts (notably Bun’s bun link double-symlink behavior) by locating the active photon bin directory via $PATH scanning and realpath matching instead of assuming a node_modules/.../dist/photon.js directory shape.
Changes:
- Replace the previous
node_modules-shape regex/traversal logic with a$PATH(plus fallback dirs) scan that finds aphotonwhoserealpathmatches the running script. - Create
pho -> ./photonnext to the matchedphotonentry, while remaining best-effort and non-clobbering.
Comments suppressed due to low confidence (1)
src/lib/pho-alias.ts:72
- The
lstatSync(pho)check treats any thrown error as “pho doesn’t exist” and proceeds to create the symlink. IflstatSyncfails due to permissions or transient IO issues (not ENOENT), this will repeatedly attemptsymlinkSyncon every run and still fail. It would be more robust to only treatENOENTas “absent” and return early for other errors.
try {
lstatSync(pho);
return; // something already lives here; leave it alone
} catch {
// pho doesn't exist; safe to create
}
symlinkSync("./photon", pho);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
process.argv[1]. Underbun link, the double-symlink (~/.bun/bin/photon → node_modules/…/photon → dev-dir/dist/photon.js) resolved all the way to the dev directory, strippingnode_modulesfrom the path. The old regex guard failed, sophowas never created.node_modulesregex +..traversal approach with arealpathSync-based scan: check$PATHentries (plus well-known global bin fallbacks) for aphotonwhose realpath matches ours, then droppho → ./photonnext to it. No directory layout assumptions.bun link, and localnpx/bunx— tested all six layouts.Test plan
bun add -glayoutbun link(double symlink)npm install -glayoutpnpm add -glayoutyarn global addlayoutnpx/bunxlayoutbun run src/index.ts) does not trigger alias creationphobinarybun run typecheckpassesMade with Cursor
Need help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit