Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Old depreciated Global Expo CLI checks in Doctor command changed #2618

Merged
merged 10 commits into from
Feb 21, 2024
Merged

Old depreciated Global Expo CLI checks in Doctor command changed #2618

Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cca6e1b
:alien: Depr Global Expo CLI tool
Dax911 Jan 29, 2024
777b020
:wastebasket: Cleaned up expo
Dax911 Jan 29, 2024
6ccfe2c
Merge branch 'infinitered:master' into master
Dax911 Feb 8, 2024
0a9e8d5
:coffin: Rm dead else statement
Dax911 Feb 8, 2024
d6efe2b
:rotating_light: Ran linter
Dax911 Feb 8, 2024
31d0f64
:fire: Rm bun file
Dax911 Feb 8, 2024
c5a17ff
:sparkles: Add warning
Dax911 Feb 9, 2024
a8d1897
:recycle: Cleaned up code
Dax911 Feb 9, 2024
5bf5907
:recycle: Oops one more
Dax911 Feb 9, 2024
759a824
:lipstick: Fix column
Dax911 Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/commands/doctor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,24 @@ module.exports = {
expoVersion = "-"
expoWorkflow = "not installed"
}

const expoInfo = [column1("expo"), column2(expoVersion), column3(expoWorkflow)]

let depExpoCLIInfo = null
Dax911 marked this conversation as resolved.
Show resolved Hide resolved

try {
const expoCLIVersionOutput = await run("expo-cli --version", { trim: true })
Dax911 marked this conversation as resolved.
Show resolved Hide resolved
if (expoCLIVersionOutput) {
const expoCLIVersion = expoCLIVersionOutput.replace("v", "")
depExpoCLIInfo = [
column1("expo global cli"),
column2(expoCLIVersion),
column3("Deprecated: Found 'expo-cli' installed. Please remove it."),
Dax911 marked this conversation as resolved.
Show resolved Hide resolved
]
}
} catch (_) {
// No action needed if 'expo-cli' is not installed or there's an error checking its version.
}

info("")
info(colors.cyan(`JavaScript${haveGlobalPackages ? " (and globally-installed packages)" : ""}`))
table([
Expand All @@ -110,6 +125,7 @@ module.exports = {
...pnpmPackages,
bunInfo,
expoInfo,
...(depExpoCLIInfo ? [depExpoCLIInfo] : []),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
...(depExpoCLIInfo ? [depExpoCLIInfo] : []),
depExpoCliInfo,

I believe table knows what to do with a null value here, so we can just simplify this up!

Copy link
Contributor Author

@Dax911 Dax911 Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe you are correct here. And I was right I tested it with your change and the result was

ignite-cli doctor
System
  platform           darwin
  arch               arm64
  cpu                8 cores      Apple M2
  directory          ignite       /Users/dax/Code/contribs/ignite

JavaScript (and globally-installed packages)
/Users/dax/Code/contribs/ignite/node_modules/gluegun/node_modules/cli-table3/src/layout-manager.js:143
    return rows.map(function (row) {
                ^
TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at /Users/dax/Code/contribs/ignite/node_modules/gluegun/node_modules/cli-table3/src/layout-manager.js:145:26
    at Table.map (<anonymous>)
    at generateCells (/Users/dax/Code/contribs/ignite/node_modules/gluegun/node_modules/cli-table3/src/layout-manager.js:143:17)
    at Object.makeTableLayout (/Users/dax/Code/contribs/ignite/node_modules/gluegun/node_modules/cli-table3/src/layout-manager.js:161:20)
    at Table.toString (/Users/dax/Code/contribs/ignite/node_modules/gluegun/node_modules/cli-table3/src/table.js:23:29)
    at table (/Users/dax/Code/contribs/ignite/node_modules/gluegun/src/toolbox/print-tools.ts:136:17)
    at Command.<anonymous> (/Users/dax/Code/contribs/ignite/src/commands/doctor.ts:118:5)
    at step (/Users/dax/Code/contribs/ignite/src/commands/doctor.ts:33:23)
    at Object.throw (/Users/dax/Code/contribs/ignite/src/commands/doctor.ts:14:53)

I can leave it as is, create another PR to fit the table handling a null case, since its from Gluegun I can check the docs to see if does have info on gracefully handling null otherwise we can revert please advise.

])

// -=-=-=- ignite -=-=-=-
Expand Down