expo / expo-cli

Tools for creating, running, and deploying universal Expo and React Native apps
https://docs.expo.io/workflow/expo-cli/
2.61k stars 477 forks source link

[doctor] Use Powershell to launch `npx expo install --check` on Windows #4705

Closed keith-kurak closed 1 year ago

keith-kurak commented 1 year ago

Why

Expo Doctor's dependency version check isn't working at all on Windows right now (other than in WSL) 🙈 .

When calling npx expo install --check, we had to pipe a "y" into the command so it would finish, necessitating this technique for calling a child process. Since this uses sh, it doesn't work in Windows.

How

powershell can be used just like sh in this situation. So, a) changed InstalledDependencyVersionCheck to use powershell instead when running on Windows, and changed the cmd.exe warning when first running Doctor to something that stops execution entirely (which is in line with our installation requirements specifying PowerShell or WSL).

In the long run, the plan would be to make npx expo install --check work non-interactively, so this syntax is no longer needed. Even then, that would only work with newer SDK versions, so this code would remain.

Test Plan

Failing unit tests

This unit test is currently failing. The mocked return value of isWindowsShell is not getting applied (it's always undefined). It's perplexing because my mock of isRunningOnCmdExeAsync (from the same file) in doctor.test.ts is working. I could get rid of the unit tests, but I'd like to include them, so if anyone has a suggestion as to what's going wrong with my mocking, I'd appreciate the help!

linear[bot] commented 1 year ago
ENG-8570 Doctor not working on windows

Need powershell compat for dependency version check

keith-kurak commented 1 year ago

Close in favor of https://github.com/expo/expo-cli/pull/4706