fix(cli): stop doctor crashing on missing yarn and npm 9+ warnings - #3039
Open
mrpmohiburrahman wants to merge 1 commit into
Open
fix(cli): stop doctor crashing on missing yarn and npm 9+ warnings#3039mrpmohiburrahman wants to merge 1 commit into
mrpmohiburrahman wants to merge 1 commit into
Conversation
`doctor` threw instead of printing its report, for two independent reasons reported by two people in infinitered#3024. When yarn is not installed, `which("yarn")` returns null and `yarnVersion` short-circuits to null, but the `.split(".")` deriving `yarnMajorVersion` sat outside the `yarnPath &&` guard protecting the line above it. Guard the derivation; all four downstream consumers already tolerate absence. Separately, the npm global-package listing strips warning lines before `JSON.parse`, but the regex only matched the legacy uppercase `npm WARN`. npm 9+ emits lowercase `npm warn`, which survived the strip and reached the parser. Make the strip case-insensitive; the existing strategy is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
npx ignite-cli doctorthrows instead of printing the environment report. It isthe command people run because something is already broken, so it fails exactly
when it is needed.
#3024 has two reporters with two different stack traces. They are independent bugs
that happen to sit behind the same command, so fixing one still leaves the other
person crashing — that is why this is one PR.
Problem
1. Crash when yarn is not installed (reported by @xx1906, Windows 10)
which("yarn")returnsnullwhen yarn is absent, soyarnVersionshort-circuitsto
null— but the.split(".")on the next line sits outside theyarnPath &&guard that protects the line above it:
This arrived with the yarn 4 support in #2913. It does not reproduce on a typical
Mac setup, which is why it looked unreproducible — macOS almost always has yarn
present via corepack.
2. Crash parsing npm output (reported by @pietrofxq, Linux, Node 24)
packager.list()strips npm's warning lines beforeJSON.parse, but the regex onlymatched the legacy uppercase
npm WARN. npm 9+ emits lowercasenpm warn, whichsurvives the strip and gets handed to the parser along with the JSON.
Solution
Two one-line changes.
src/commands/doctor.ts— derive the major version only when there is a version toderive it from. The four things downstream already handle absence (
=== 1is falsy,column2renders-,column3rendersnot installed), so the guard belongs atthe derivation rather than at every call site:
src/tools/packager.ts— make the warning strip case-insensitive. The prepended-warningstrategy documented in the comment above it is deliberate and unchanged; only the casing
was wrong:
Testing
Both crashes were reproduced locally first, then confirmed gone.
Missing yarn — running
doctorwith a PATH containingnodebut notyarnthrewthe reported
TypeErrorbefore the change. After it, the full report prints and thecommand exits 0, with the existing rendering preserved:
With yarn present the output is byte-identical to before the change — I captured
doctoron the patched tree, stashed the two fixes, captured it again, and diffed.No difference, both exit 0. That is what shows the guard is purely additive rather
than quietly changing what the table reports.
npm warnings — added a regression test to the existing
src/tools/packager.test.tscovering lowercase
npm warn. I checked it genuinely fails on the old regex rather thanpassing either way:
The two existing uppercase
npm WARNtests in that file still pass, so older npm outputis unaffected. A live
doctorrun also parsed realnpm list --global --jsonoutput fromnpm 10.9.2 through the changed function.
Suite: 7 of 8 suites pass (40 of 41 tests), plus
typecheck,format:checkanddepcruiseclean. The one failure,ignite-new.test.ts › --packager=yarn, failsidentically on master with these changes stashed, so it is pre-existing and not something
this PR introduces.
I did not add a unit test for the
doctorpath itself — there is no command-level testharness in the repo and no
jest.mockanywhere, so covering it would mean introducing amocking pattern that does not exist yet. Happy to add one if you would like it.
Follow-ups (not in this PR)
Two adjacent things I noticed and deliberately left alone to keep this a small crash fix —
happy to open issues or separate PRs if useful:
doctor.ts:54—run("yarn --version")is unguarded and gluegun'srunthrows on anon-zero exit. Inside any project whose
packageManagerfield names something other thanyarn, corepack refuses to run yarn and
doctorstill exits 1. This is the line abovethe one changed here, so it is untouched by this PR.
doctor.ts:58— pnpm is only probed when yarn v1 is present (if (yarnMajorVersion === 1)),so a pnpm-only machine reports pnpm as "not installed". Slightly odd now that Use and recommend pnpm instead of yarn #3016 moved
the repo itself onto pnpm.
Issues: fixes npx ignite-cli doctor throw error #3024
Checklist
are fixed, and verified
doctor's output is byte-identical when yarn is present(see docs).