fix: allow running quartz via bunx - #2523
Conversation
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
There was a problem hiding this comment.
Pull request overview
Adds a postinstall bootstrap step so the quartz CLI is available via bunx quartz (and other runners that rely on node_modules/.bin) even when the package manager doesn’t auto-link bin entries (as Bun currently doesn’t).
Changes:
- Add
quartz/bootstrap-postinstall.mjsto create/refreshnode_modules/.bin/quartzpointing atquartz/bootstrap-cli.mjs. - Wire the script into
npmlifecycle viapostinstallinpackage.json. - Update
package-lock.jsonto reflect the presence of an install script.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| quartz/bootstrap-postinstall.mjs | Implements the postinstall symlink/bootstrap logic for node_modules/.bin/quartz. |
| package.json | Adds a postinstall hook to run the bootstrap script after installs. |
| package-lock.json | Records that the package has an install script (hasInstallScript). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const binDir = path.join(root, "node_modules", ".bin") | ||
| mkdirSync(binDir, { recursive: true }) | ||
|
|
||
| // only tested on mac, someone on windows will have to add a .cmd shim for this | ||
| const linkPath = path.join(binDir, "quartz") | ||
| const relativeTarget = path.relative(binDir, target) |
| } catch (err) { | ||
| console.warn(`[quartz] could not link node_modules/.bin/quartz: ${err.message}`) | ||
| } |
|
Pushed an update which should make the code more robust and fix the issues mentioned by copilot
Retested on mac with node 26, Windows still untested as I don't have a machine available :/ |
The Quartz CI tests run on all three platforms. Tests targeting Windows can be verified that way. |
|
Got hold of a Windows machine and did a fresh install of the fork there:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
quartz/bootstrap-postinstall.mjs:2
mkdirSync(binDir, { recursive: true })will createnode_modules/.bin(and potentially the entirenode_modulesdirectory) under the package root wheneverpostinstallruns. If Quartz is installed as a dependency via a git/file reference (where the package directory typically does not already contain its ownnode_modules), this can create a nestednode_modules/insidenode_modules/@jackyzha0/quartz, which is usually undesirable. Consider bailing out unlessroot/node_modulesalready exists (i.e. only run in a repo checkout / root install).
import { chmodSync, mkdirSync, readlinkSync, symlinkSync, unlinkSync, writeFileSync } from "fs"
quartz/bootstrap-postinstall.mjs:48
- In the
EEXISTcase,readlinkSync(linkPath)will throw iflinkPathexists but is not a symlink (e.g. a regular file created by another tool). That exception currently escapes the innercatch, so the script won't replace the incorrect entry. Handle the non-symlink case by wrappingreadlinkSyncin its own try/catch and recreating the link when it fails.
try {
symlinkSync(relativeTarget, linkPath)
} catch (err) {
if (err.code !== "EEXIST") {
throw err
}
if (readlinkSync(linkPath) !== relativeTarget) {
unlinkSync(linkPath)
symlinkSync(relativeTarget, linkPath)
}
}
.github/workflows/ci.yaml:55
- The new CI check runs
bunxafternpm install, butnpm installalready createsnode_modules/.bin/quartz, so this step likely would have passed even before the newpostinstallscript. To actually validate the behavior being fixed (Bun not creating the shim), consider deleting the shim(s) and re-runningquartz/bootstrap-postinstall.mjsbefore invokingbunx.
- run: npm install
# --no-install because a bare `bunx quartz` falls back to an unrelated
# package on npm instead of failing
- name: Ensure bunx resolves the local CLI
run: bunx --no-install quartz --help
|
First two comments by copilot are valid and i fixed both:
ALSO, I found a worse error in my own code while messing around with that. The Windows branch wrote to Third comment I don't think is right though. npm only links bins of dependencies, not the root packag's own, which is basically why this PR exists. Checked with a small package with a bin field and one dep, after |
Problem:
npm links a package's own bin entries into
node_modules/.binautomatically, which is why npx quartz works right after cloning the repo. Bun doesn't do that, so bunx quartz fails with could not determine executable to run for package quartz.Solution:
Added a small postinstall script that creates the
node_modules/.bin/quartzsymlink by hand, so it works regardless of which package manager did the install.Only tested on macOS, so it's Unix only for now. Windows would need a .cmd shim, happy to add that if someone can test it.