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.

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

[cli] Add dependency exclusion to npx expo install #22736

Merged
merged 19 commits into from Jun 13, 2023

Conversation

keith-kurak
Copy link
Contributor

@keith-kurak keith-kurak commented Jun 2, 2023

Why

Fulfills https://linear.app/expo/issue/ENG-8564/add-dependency-exclusion-to-npx-expo-install. Developers will be able to suppress warnings from npx expo install --check via a package.json config, and thus suppress doctor warnings when they intentionally choose to install a different version, especially when using dev builds and not needing to stick to the Expo Go- bundled version.

How

npx expo install --check|fix reads from expo.install.exclude if it's available, and, if one of the packages it thinks it should fix is in that list, it will skip asking/ fixing it. If you run EXPO_DEBUG=1 npx expo install --check, the packages whose checks were actually skipped will be listed in debug output; otherwise there are no warnings or messages about the excluded packages.

The package.json configuration verbiage is intended to be similar to the autolinking config.

It also seemed appropriate that npx expo install should behave differently if you try to install an excluded package, so someone doesn't come along and write over a version that another developer determined was good and thus applied the override for. Currently, it aborts if any of the packages are in the exclusion list. I could see doing a warning and skipping the affected packages instead, but the command outright failing is pretty hard to miss.

npx expo install now installs excluded packages with the latest version, just like a package without a specified native version. It will add a note describing what it's doing and why.

Test Plan

  • test without expo.install.exclude (works the same)
  • test with empty exclusion list (works the same)
  • test with exclusions that aren't actual dependencies (works the same)
  • test expo install --fix|check when excluding an actual dependency (skips it)
  • test with EXPO_DEBUG=1 (lists excluded packages)
  • test expo install [package] with exclusions (installs latest with notes)
  • test expo install [package] without exclusions (proceeds as it normally does)

Output

npx expo install --check, showing note that it is skipping checking packages in the exclude list:
image
Additional debug output for EXPO_DEBUG=1 npx expo install --check:
image
expo install <package> with exclusions:
image

Checklist

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jun 2, 2023
@keith-kurak keith-kurak marked this pull request as ready for review June 2, 2023 19:20
@keith-kurak keith-kurak changed the title [cli] Exclude packages from expo install --check|fix [cli] Add dependency exclusion to npx expo install Jun 2, 2023
Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

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

looks good, let's also add some documentation in this same PR. we can add a note along with it that this field is only supported in sdk 49+

@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jun 2, 2023
docs/pages/more/expo-cli.mdx Outdated Show resolved Hide resolved
docs/pages/more/expo-cli.mdx Outdated Show resolved Hide resolved
@brentvatne
Copy link
Member

another suggestion - maybe we should link to the docs in the validation message? so if someone has an "invalid" dep version and want to have that one be ignored, the information on how they do that is available directly in the message.

keith-kurak and others added 4 commits June 5, 2023 12:18
@keith-kurak
Copy link
Contributor Author

another suggestion - maybe we should link to the docs in the validation message? so if someone has an "invalid" dep version and want to have that one be ignored, the information on how they do that is available directly in the message.

Added!

I was just wondering if I should break any of this text out of this single error/ exit log. Wasn't sure if it's too much red.

image

Copy link
Member

@amandeepmittal amandeepmittal left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the callout issue!

@keith-kurak keith-kurak merged commit 6fb32dd into main Jun 13, 2023
5 of 7 checks passed
@keith-kurak keith-kurak deleted the keith/configure-skip-version-warnings branch June 13, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants