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] fix ignored existing plugins on expo install #17936

Merged

Conversation

kbrandwijk
Copy link
Member

@kbrandwijk kbrandwijk commented Jun 21, 2022

Why

Existing plugins were ignored retrieving the config for expo install.

How

No longer skip resolving plugins on install.
See expo/expo-cli#4429 for the sister fix for the existing expo-cli

Test Plan

  • Initialize a blank project
  • npx expo install expo-camera
  • Add expo-camera plugin to app.json: "plugins": ["expo-camera"]
  • Run npx expo install sentry-expo
  • Notice that both plugins exists in the plugins array

Will look into automated testing, but suggestions are welcome.

Checklist

  • Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md).
  • This diff will work correctly for expo build (eg: updated @expo/xdl).
  • This diff will work correctly for expo prebuild & EAS Build (eg: updated a module plugin).

@linear
Copy link

linear bot commented Jun 21, 2022

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jun 21, 2022
@kbrandwijk kbrandwijk marked this pull request as ready for review June 21, 2022 22:51
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jun 21, 2022
@kbrandwijk kbrandwijk requested review from brentvatne and removed request for byCedric June 21, 2022 22:52
Copy link
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

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

This feature was added specifically for this purpose.

Need to skip plugins (at least once) otherwise expo install cannot be used with plugins in the array, for example, add expo-camera to the plugins array, delete the node_modules/, and run expo install.

Will need to ensure this solution does not break the use-case I just mentioned, otherwise should be fine.

@kbrandwijk
Copy link
Member Author

This feature was added specifically for this purpose.

Need to skip plugins (at least once) otherwise expo install cannot be used with plugins in the array, for example, add expo-camera to the plugins array, delete the node_modules/, and run expo install.

Will need to ensure this solution does not break the use-case I just mentioned, otherwise should be fine.

yarn add ... is ran first when you run expo install, before the config plugins array is parsed, so the dependencies are always installed by the time this line is executed.

@EvanBacon EvanBacon merged commit 0f1a896 into main Jun 30, 2022
@EvanBacon EvanBacon deleted the @kbrandwijk/eng-5370-versioned-cli-automatically-added-config branch June 30, 2022 17:48
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

3 participants