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(eas-cli): expose expo export dev flag as an option in eas update #2050

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

nderscore
Copy link
Contributor

@nderscore nderscore commented Sep 7, 2023

Why

When __DEV__ is unset or false, expo-dev-menu's registerDevMenuItems option is a no-op.

In order to be able to use custom expo-dev-menu menu items in updates/previews for a debug build, it is necessary for the __DEV__ global variable to be set to true in the generated updates and preview bundles.

EAS does not currently expose the --dev option from Expo CLI's export command, which is what determines whether or not __DEV__ gets stripped during bundling.

How

Adds a new CLI flag (--dev) to the eas update command.

When set, an additional CLI argument --dev is passed to expo export.

Test Plan

I tested this in my own project and it successfully deployed an update where __DEV__ is set to true and my custom dev menu items are being registered correctly.

@@ -208,6 +210,7 @@ export async function buildBundlesAsync({
'--dump-sourcemap',
'--dump-assetmap',
`--platform=${platformFlag}`,
...(dev ? ['--dev'] : []),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this flag exists on the legacy Expo CLI

Copy link
Member

Choose a reason for hiding this comment

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

@wschurman wschurman requested review from quinlanj and removed request for byCedric, khamilowicz and szdziedzic September 7, 2023 19:19
@quinlanj quinlanj requested a review from douglowder September 11, 2023 18:12
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #2050 (4a49eb0) into main (cc822b1) will decrease coverage by 0.01%.
Report is 1 commits behind head on main.
The diff coverage is 33.34%.

@@            Coverage Diff             @@
##             main    #2050      +/-   ##
==========================================
- Coverage   53.98%   53.97%   -0.01%     
==========================================
  Files         508      508              
  Lines       18593    18600       +7     
  Branches     3909     3915       +6     
==========================================
+ Hits        10036    10037       +1     
- Misses       8537     8543       +6     
  Partials       20       20              
Files Changed Coverage Δ
packages/eas-cli/src/project/publish.ts 76.72% <0.00%> (-1.06%) ⬇️
packages/eas-cli/src/commands/update/index.ts 71.58% <100.00%> (ø)

... and 1 file with indirect coverage changes

Copy link
Member

@quinlanj quinlanj left a comment

Choose a reason for hiding this comment

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

some nits, otherwise lgtm!

packages/eas-cli/src/commands/update/index.ts Outdated Show resolved Hide resolved
@@ -208,6 +210,7 @@ export async function buildBundlesAsync({
'--dump-sourcemap',
'--dump-assetmap',
`--platform=${platformFlag}`,
...(dev ? ['--dev'] : []),
Copy link
Member

Choose a reason for hiding this comment

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

@wschurman
Copy link
Member

@nderscore - can you manually add an entry to https://github.com/expo/eas-cli/blob/main/CHANGELOG.md

Then we can go ahead an merge!

@nderscore
Copy link
Contributor Author

@wschurman sorry for missing that part of the contributing docs! 😅

Updated with a change log entry.

@wschurman wschurman merged commit 8751a28 into expo:main Sep 13, 2023
8 of 9 checks passed
@wschurman
Copy link
Member

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants