ignite icon indicating copy to clipboard operation
ignite copied to clipboard

Old depreciated Global Expo CLI checks in Doctor command changed

Open Dax911 opened this issue 1 year ago • 11 comments

Please verify the following:

  • [x] yarn test jest tests pass with new tests, if relevant
  • [x] 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.

Dax911 avatar Jan 29 '24 23:01 Dax911

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.

Dax911 avatar Jan 29 '24 23:01 Dax911

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?

markrickert avatar Feb 06 '24 20:02 markrickert

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

Dax911 avatar Feb 06 '24 21:02 Dax911

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?

Dax911 avatar Feb 08 '24 19:02 Dax911

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.

markrickert avatar Feb 08 '24 19:02 markrickert

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

Dax911 avatar Feb 08 '24 22:02 Dax911

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 avatar Feb 09 '24 03:02 Dax911

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

Dax911 avatar Feb 09 '24 03:02 Dax911

@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!

Dax911 avatar Feb 09 '24 16:02 Dax911

@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 :)

frankcalise avatar Feb 09 '24 16:02 frankcalise

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.

Dax911 avatar Feb 15 '24 20:02 Dax911

:tada: This PR is included in version 9.6.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

infinitered-circleci avatar Feb 21 '24 17:02 infinitered-circleci