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

Dax911 · 2024-01-29T23:08:23Z

Please verify the following:

yarn test jest tests pass with new tests, if relevant
README.md has been updated with your changes, if relevant

Describe your PR

I cleaned up the old expo global cli tool install check for the doctor command. I added in a warning dialogue for developers who still have it installed.

rm expo and added warning for depr

Dax911 · 2024-01-29T23:09:07Z

yarn test
yarn run v1.22.21
$ TS_JEST_DISABLE_VER_CHECKER=true jest
 PASS  src/tools/packager.test.ts
 PASS  src/tools/demo.test.ts
 PASS  test/vanilla/ignite-help.test.ts (10.631 s)
 PASS  test/vanilla/ignite-remove-demo.test.ts (11.145 s)
 PASS  test/vanilla/ignite-generate.test.ts (37.078 s)
 FAIL  test/vanilla/ignite-new.test.ts (121.722 s)
  ● ignite new › ignite new Foo --debug --packager=bun --yes › should be able to use `generate` command and have pass output pass bun run test, bun run lint, and bun run compile scripts

    Command failed: cd /private/var/folders/32/m_87zp_n3218j0y94jfsywdw0000gn/T/ignite-e1873a30238044ba33c0e3a2e7e33f9b/Foo && bun run test && cd /Users/dax/Code/contribs/ignite
    $ jest
     FAIL  app/models/ModTest.test.ts
      ● Test suite failed to run

        TypeError: Cannot redefine property: performance

          at Object.<anonymous> (node_modules/react-native/jest/setup.js:409:20)

     FAIL  app/models/Episode.test.ts
      ● Test suite failed to run

        TypeError: Cannot redefine property: performance

          at Object.<anonymous> (node_modules/react-native/jest/setup.js:409:20)

     FAIL  app/services/api/apiProblem.test.ts
      ● Test suite failed to run

        TypeError: Cannot redefine property: performance

          at Object.<anonymous> (node_modules/react-native/jest/setup.js:409:20)

     FAIL  app/utils/storage/storage.test.ts
      ● Test suite failed to run

        TypeError: Cannot redefine property: performance

          at Object.<anonymous> (node_modules/react-native/jest/setup.js:409:20)

     FAIL  test/i18n.test.ts
      ● Test suite failed to run

        TypeError: Cannot redefine property: performance

          at Object.<anonymous> (node_modules/react-native/jest/setup.js:409:20)

    Test Suites: 5 failed, 5 total
    Tests:       0 total
    Snapshots:   0 total
    Time:        3 s
    Ran all test suites.
    error: script "test" exited with code 1 (SIGHUP)



Test Suites: 1 failed, 5 passed, 6 total
Tests:       1 failed, 33 passed, 34 total
Snapshots:   31 passed, 31 total
Time:        122.154 s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Getting the above errors this is the only blocker but I don't immediately see how my changes broke these tests.

markrickert · 2024-02-06T20:28:20Z

Hey @Dax911, i had to install the bun binary to get tests passing locally and i think #2625 will fix the CI issues. Mind rebasing on master and pushing the changes back up to try the CI again?

Dax911 · 2024-02-06T21:57:22Z

Will do later in the week. Thanks for insight. I'm sick in the hospital right now.

Dax911 · 2024-02-08T19:06:30Z

Hey @Dax911, i had to install the bun binary to get tests passing locally and i think #2625 will fix the CI issues. Mind rebasing on master and pushing the changes back up to try the CI again?

Hey sorry I thought would be a quick fix so I actually did all this on the master of my fork. So I instead removed a dead else statement I missed before and it ran again. I even reinstalled bun as I had it before when I ran it? Do I need to be doing something different?

markrickert · 2024-02-08T19:18:43Z

Looks like you could get it to pass CI by running yarn format:write and committing the changes. Looks like it's hung up on the linter.

Dax911 · 2024-02-08T22:33:42Z

Ohh I have to force the linter to run idk why I thought it did that itself. Sorry sick brain.

file created when reinstalling bun didnt see it slip in

frankcalise

Hey @Dax911 ! First off thanks for the contribution and hope you are feeling better.

I love the idea of the expo-cli warning, that's a great addition!

However, I think we could still restore the managed/bare expo workflow to the info table, as this was related to the project directory the command is being run in and not part of the global install checks.

Let me know your thoughts and again, be well!

Dax911 · 2024-02-09T03:24:22Z

However, I think we could still restore the managed/bare expo workflow to the info table, as this was related to the project directory the command is being run in and not part of the global install checks.

Yea, I actually did this after chatting w @jamonholmgren about the Expo CLI being returned as I wasn't even sure what tool it was referring to anymore. I totally could have gotten over excited in my sick and slashing things mindset. I will take second look I think you are right.

I am seeing that you are right and that it is unrelated to that specific check... I am curious tho what purpose this managed/bare expo workflow serves/the importance could someone shed some light on that?

Edit: Also it only shows up if the expo CLI is installed and provides value only if ur using a deprecated tool?

Dax911 · 2024-02-09T03:45:19Z

OH, it all just clicked yes your right thank you! Will make that change.

adds warning to doctor when dep cli tool is installed

frankcalise

@Dax911 thanks for the updates! a few code consistency comments and some color to highlight the deprecation warning and I think this will be good to go!

Thanks again

src/commands/doctor.ts

frankcalise · 2024-02-09T16:31:31Z

src/commands/doctor.ts

@@ -110,6 +125,7 @@ module.exports = {
      ...pnpmPackages,
      bunInfo,
      expoInfo,
+      ...(depExpoCLIInfo ? [depExpoCLIInfo] : []),


Suggested change

...(depExpoCLIInfo ? [depExpoCLIInfo] : []),

depExpoCliInfo,

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

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.

Dax911 · 2024-02-09T16:37:58Z

@Dax911 thanks for the updates! a few code consistency comments and some color to highlight the deprecation warning and I think this will be good to go!

Thanks again

I will take care of it. Sorry I'm weird I like to always capitalize CLI and short stuff like that. Totally a me thing didn't realize I was breaking that pattern, thanks for the call out!

frankcalise · 2024-02-09T16:46:25Z

@Dax911 thanks for the updates! a few code consistency comments and some color to highlight the deprecation warning and I think this will be good to go!
Thanks again

I will take care of it. Sorry I'm weird I like to always capitalize CLI and short stuff like that. Totally a me thing didn't realize I was breaking that pattern, thanks for the call out!

All good, you're helping us! And it's appreciated :)

case errors fixed

frankcalise

hey sorry, I might have made a poor suggestion with the yellow color but I think you need the column3 helper still!

src/commands/doctor.ts

forgot to push

Dax911

made fixes and ran tests

src/commands/doctor.ts

Dax911 · 2024-02-15T20:27:01Z

hey sorry, I might have made a poor suggestion with the yellow color but I think you need the column3 helper still!

made this change i just forgot to push it. It should be there.

@frankcalise

## [9.6.1](v9.6.0...v9.6.1) (2024-02-21) ### Bug Fixes * **boilerplate:** jest.config.js remove jsdom test env for better RNTL support out of box ([#2632](#2632) by [@frankcalise](https://github.com/frankcalise)) ([22c7afc](22c7afc)) * **boilerplate:** remove `yarn prebuild` script in favor of clean ([#2635](#2635) by [@frankcalise](https://github.com/frankcalise)) ([f669a32](f669a32)) * **boilerplate:** web hmr fixes ([#2646](#2646) by [@frankcalise](https://github.com/frankcalise)) ([45aabb2](45aabb2)) * **cli:** Global Expo CLI checks in Doctor command ([#2618](#2618) by @Dax911) ([d0b2b2d](d0b2b2d)) * **components:** switch animation slide on web ([#2642](#2642) by [@frankcalise](https://github.com/frankcalise)) ([c0dcc6d](c0dcc6d))

infinitered-circleci · 2024-02-21T17:01:12Z

🎉 This PR is included in version 9.6.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Dax911 added 2 commits January 29, 2024 16:43

👽 Depr Global Expo CLI tool

cca6e1b

rm expo and added warning for depr

🗑️ Cleaned up expo

777b020

Dax911 added 2 commits February 8, 2024 12:42

Merge branch 'infinitered:master' into master

6ccfe2c

⚰️ Rm dead else statement

0a9e8d5

Dax911 added 2 commits February 8, 2024 16:36

🚨 Ran linter

d6efe2b

🔥 Rm bun file

31d0f64

file created when reinstalling bun didnt see it slip in

frankcalise requested changes Feb 9, 2024

View reviewed changes

✨ Add warning

c5a17ff

adds warning to doctor when dep cli tool is installed

frankcalise requested changes Feb 9, 2024

View reviewed changes

Dax911 added 2 commits February 9, 2024 11:17

♻️ Cleaned up code

a8d1897

case errors fixed

♻️ Oops one more

5bf5907

frankcalise requested changes Feb 9, 2024

View reviewed changes

src/commands/doctor.ts Outdated Show resolved Hide resolved

src/commands/doctor.ts Show resolved Hide resolved

💄 Fix column

759a824

forgot to push

Dax911 commented Feb 13, 2024

View reviewed changes

src/commands/doctor.ts Outdated Show resolved Hide resolved

frankcalise approved these changes Feb 21, 2024

View reviewed changes

frankcalise merged commit d0b2b2d into infinitered:master Feb 21, 2024
1 check passed

infinitered-circleci added the released label Feb 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Dax911 commented Jan 29, 2024 •

edited

Dax911 commented Jan 29, 2024 •

edited

markrickert commented Feb 6, 2024

Dax911 commented Feb 6, 2024

Dax911 commented Feb 8, 2024

markrickert commented Feb 8, 2024

Dax911 commented Feb 8, 2024

frankcalise left a comment

Dax911 commented Feb 9, 2024 •

edited

Dax911 commented Feb 9, 2024

frankcalise left a comment

frankcalise Feb 9, 2024

Dax911 Feb 9, 2024 •

edited

Dax911 commented Feb 9, 2024

frankcalise commented Feb 9, 2024

frankcalise left a comment

Dax911 left a comment

Dax911 commented Feb 15, 2024

infinitered-circleci commented Feb 21, 2024

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

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

Conversation

Dax911 commented Jan 29, 2024 • edited

Please verify the following:

Describe your PR

Dax911 commented Jan 29, 2024 • edited

markrickert commented Feb 6, 2024

Dax911 commented Feb 6, 2024

Dax911 commented Feb 8, 2024

markrickert commented Feb 8, 2024

Dax911 commented Feb 8, 2024

frankcalise left a comment

Choose a reason for hiding this comment

Dax911 commented Feb 9, 2024 • edited

Dax911 commented Feb 9, 2024

frankcalise left a comment

Choose a reason for hiding this comment

frankcalise Feb 9, 2024

Choose a reason for hiding this comment

Dax911 Feb 9, 2024 • edited

Choose a reason for hiding this comment

Dax911 commented Feb 9, 2024

frankcalise commented Feb 9, 2024

frankcalise left a comment

Choose a reason for hiding this comment

Dax911 left a comment

Choose a reason for hiding this comment

Dax911 commented Feb 15, 2024

infinitered-circleci commented Feb 21, 2024

Dax911 commented Jan 29, 2024 •

edited

Dax911 commented Jan 29, 2024 •

edited

Dax911 commented Feb 9, 2024 •

edited

Dax911 Feb 9, 2024 •

edited