fix(oxlint-plugin): Resolve the oxlint bin without assuming a pnpm layout - #5066
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…yout The rule-harness tests spawned node_modules/.pnpm/node_modules/.bin/oxlint, a pnpm virtual-store hoist path. oxlint is only a transitive dependency of vite-plus, so no package manager guarantees a stable .bin shim for it: under Bun-style hoisted installs that path does not exist, and the root node_modules/.bin/oxlint is vite-plus's LSP-only wrapper, so all 35 rule tests fail with spawn ENOENT. Resolve oxlint/package.json through vite-plus instead, at module scope so a broken install fails once with a clear resolution error, and run the package's own bin/oxlint through process.execPath so the spawn does not depend on shebang support or whichever node is first on PATH.
ca5f5c6 to
3da07e2
Compare
ApprovabilityVerdict: Approved 3da07e2 Test-only infrastructure fix that replaces hardcoded pnpm paths with proper Node module resolution and adds Windows compatibility. No production code is affected. You can customize Macroscope's approvability policy. Learn more. |
## What's Changed * fix(ci): repair the mobile showcase screenshots workflow by @juliusmarminge in pingdotgg/t3code#5057 * fix(ci): capture iPad App Store screenshots in landscape by @PixPMusic in pingdotgg/t3code#5065 * fix(oxlint-plugin): Resolve the oxlint bin without assuming a pnpm layout by @mwolson in pingdotgg/t3code#5066 **Full Changelog**: pingdotgg/t3code@v0.0.32-nightly.20260731.963...v0.0.32-nightly.20260731.964 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.32-nightly.20260731.964
Summary
the
oxlintbinary. Resolve the real package throughvite-plusinstead andrun its bin through the current Node binary, so the suite runs under any
install layout.
Problem and Fix
oxlint-plugin-t3code/test/utils.tsspawnednode_modules/.pnpm/node_modules/.bin/oxlint, a pnpm-internal hoist path.oxlintis only a transitive dependency (viavite-plus), so no package manager guarantees a stable.binshim for it: under Bun-style hoisted installs that path does not exist, and the rootnode_modules/.bin/oxlintis vite-plus's LSP-only wrapper ("This oxlint wrapper is for IDE extension use only"). All 35 rule tests fail withspawn ... ENOENTon such layouts.oxlint/package.jsonwithcreateRequire, starting fromvite-plus's resolved package directory (the dependency that actually pulls oxlint in), and run the package's ownbin/oxlint. Module resolution works identically through pnpm's virtual store and hoisted layouts, so the harness no longer depends on any package manager's.binplacement. Resolution happens once at module scope, so a broken install fails with a single clear resolution error rather than one opaque defect per test.Defensive Fixes
bin/oxlintis an extensionless#!/usr/bin/env nodescript. Spawning it directly cannot work on Windows (no shebang handling; the old.binPOSIX shim had the same limitation), and on any platform it runs whichevernodeis first onPATH, which under version managers can differ from the Node running the tests.process.execPathwith the resolved bin script as the first argument, pinning the interpreter to the Node running vitest and removing the dependence on shebang support and the file's exec bit.Validation
vp check: pass (0 errors, 11 pre-existing warnings)vp run typecheck: passoxlint-plugin-t3code:vp test runpasses 35/35 under a non-pnpm hoistedinstall layout; before the change all 35 failed with
spawn ENOENT. Under astandard pnpm install the resolved path is the same real
oxlintpackage theold hardcoded shim pointed at (the pnpm shim ends in
exec node "$basedir/../oxlint/bin/oxlint"), and CI's Test job passes undera real pnpm install.
Note
Low Risk
Test-only harness changes with no production runtime or security impact; behavior under a correct pnpm install stays equivalent to the old shim target.
Overview
The oxlint rule-harness in
test/utils.tsno longer spawns a hardcoded pnpm virtual-store path tooxlint. It resolvesoxlint/package.jsonat module load viacreateRequire, anchored onvite-plus(the package that pulls in oxlint), and builds the path to that package’sbin/oxlint.Harness runs invoke
process.execPathwith that script as an argument instead of executing the bin directly, so Windows avoids shebang/extensionless spawn issues and the same Node that runs Vitest interprets the script.A bad install now fails once at import with a resolution error instead of
ENOENTon every rule test.Reviewed by Cursor Bugbot for commit 3da07e2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix oxlint binary resolution in
oxlint-pluginto work without a pnpm layout.pnpm-based path with a dynamic resolution usingnode:module'screateRequire, resolvingoxlint/package.jsonrelative tovite-plusand deriving the bin path from its directory.process.execPath) rather than directly, which avoids shebang issues on Windows.vite-plusoroxlintcannot be resolved.Macroscope summarized 3da07e2.