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

feat(cli)!: use Expo CLI to bundle production apps #21396

Merged
merged 31 commits into from Mar 6, 2023

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Feb 25, 2023

Why

Most of our new Metro bundler features won't work unless we use Expo CLI for every bundler operation (ex: aliases). Right now this is the case when building for production on both platforms or building for development on iOS.

Using npx expo start when building from Xcode will be added in another PR.

How

This PR introduces a new "export:embed" command which is hidden from the --help prompt. npx expo export:embed accepts the same arguments as npx react-native bundle and passes them to the same internal function, but it ensures we use the correct variation of Metro before doing such.

This change (and start PR) will add more steps for migrating to "Expo CLI" but it will also remove the need for us to generate the metro.config.js file in the project on npx expo prebuild since we can now reliably default to @expo/metro-config.

The change should only apply to Metro bundler features (all application code could be affected), but it won't obstruct Expo Modules Core or using Expo CLI.

Test Plan

  • All of our existing E2E tests should use the new command, if they contain features like aliases then they'll work.
  • Copied the changes from @expo/cli into a new project's node_modules, added template changes:
    • Built for production from Android Studio and Xcode.

Checklist

@expo-bot
Copy link
Collaborator

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing changelog entries


Your changes should be noted in the changelog. Read Updating Changelogs guide and consider adding an appropriate entry to the following changelogs:

⚠️ Suggestion: Missing links in changelog entries


I've added some suggestions below, you can just apply and commit them 😉


Generated by ExpoBot 🤖 against 847d575

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Feb 25, 2023
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@expo-bot

This comment was marked as duplicate.

@expo-bot

This comment was marked as duplicate.

@expo-bot

This comment was marked as duplicate.

@expo-bot

This comment was marked as duplicate.

@EvanBacon EvanBacon removed the request for review from Simek February 28, 2023 22:40
@expo-bot
Copy link
Collaborator

expo-bot commented Mar 1, 2023

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing links in changelog entries


I've added some suggestions below, you can just apply and commit them 😉


Generated by ExpoBot 🤖 against f1f6e11

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
Copy link
Contributor

@Kudo Kudo left a comment

Choose a reason for hiding this comment

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

appreciated if you could also update fabric-test's project.pbxproj.

apps/bare-expo/ios/BareExpo.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
docs/pages/versions/unversioned/config/metro.mdx Outdated Show resolved Hide resolved
docs/pages/versions/unversioned/config/metro.mdx Outdated Show resolved Hide resolved
docs/pages/versions/unversioned/config/metro.mdx Outdated Show resolved Hide resolved
packages/@expo/cli/bin/cli.ts Outdated Show resolved Hide resolved
packages/@expo/cli/src/export/embed/index.ts Show resolved Hide resolved
@Kudo
Copy link
Contributor

Kudo commented Mar 2, 2023

since in #21397 you've introduced expo/scripts/*, maybe we could later move these additionals of project.pbxproj into our react-native-xcode.sh.

@expo-bot
Copy link
Collaborator

expo-bot commented Mar 3, 2023

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing changelog entries


Your changes should be noted in the changelog. Read Updating Changelogs guide and consider adding an appropriate entry to the following changelogs:


Generated by ExpoBot 🤖 against c6d50ba

@expo-bot
Copy link
Collaborator

expo-bot commented Mar 3, 2023

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing links in changelog entries


I've added some suggestions below, you can just apply and commit them 😉


Generated by ExpoBot 🤖 against c85b1b3

@EvanBacon EvanBacon merged commit b6b91c5 into main Mar 6, 2023
@EvanBacon EvanBacon deleted the @evanbacon/cli/bundle-proxy-command branch March 6, 2023 18:28
@thespacemanatee
Copy link
Contributor

thespacemanatee commented Mar 12, 2023

Hey @EvanBacon, I'm getting Unknown or unexpected option: --platform when using @expo/cli instead of rn-cli and it looks like it's caused by this PR, should I create an issue?

EvanBacon added a commit that referenced this pull request Mar 13, 2023
)

# Why

- Related #21396 
- Use `npx expo start --dev-client` instead of `npx react-native start`
when building a project from Xcode. This ensures we use the correct
version of Metro regardless of where the project is started.
- It's unclear if building from Android Studio is supposed to trigger
the bundler in development but I couldn't get it to do so, so this PR is
iOS-only.
<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

# How

- Copy over a bunch of the default scripts from `react-native/scripts`
into the same locations in `expo/scripts` to make it easy to maintain
this change and simple to migrate.

I decided to omit the following as it appeared to be unused (sets
RCT_METRO_PORT):

```
source "$THIS_DIR/.packager.env"
```

I also dropped:
- Support for `$RCT_PACKAGER_LOGS_DIR`.
- The `Process terminated. Press <enter> to close the window` log that
is presented after closing the dev server.
- Support for a custom React Native CLI config (this is not supported in
Expo CLI).

<!--
How did you build this feature or fix this bug and why?
-->

# Test Plan

- TBD
- Building for development from Xcode causes a terminal window running
`npx expo start` will open instead of the community version.

<!--
Please describe how you tested this change and how a reviewer could
reproduce your test, especially if this PR does not include automated
tests! If possible, please also provide terminal output and/or
screenshots demonstrating your test/reproduction.
-->

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
Co-authored-by: Kudo Chien <kudo@expo.dev>
EvanBacon added a commit that referenced this pull request May 17, 2023
# Why

- Possible since #21396
- If the `metro.config.js` is missing, then the default,
`@expo/metro-config`, will be used.
- Drop `metro.config.js` copy step in `expo prebuild` in favor of `expo
export:embed` and the new Xcode start script using Expo CLI--this only
works when using Expo CLI for all bundling (SDK +49).
- Maybe we could add a metro linting step to `npx expo-doctor` in the
future.

# Test Plan

- tbd

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
Co-authored-by: Aman Mittal <amandeepmittal@live.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: suggestions ExpoBot has some suggestions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants