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.

Sign up for GitHub

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

[eas-cli] Makes eas.json configuration to only run on update:configure #1598

Merged
merged 3 commits into from
Jan 4, 2023
Merged

[eas-cli] Makes eas.json configuration to only run on update:configure #1598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a1faa5
Makes eas.json configuration to only run on update:configure
jonsamp Dec 29, 2022
37bdbe8
update CHANGELOG.md
jonsamp Dec 29, 2022
26dcf67
Merge branch 'main' into jon/eng-7125-only-add-channels-to-build-prof…
jonsamp Jan 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ This is the log of notable changes to EAS CLI and related packages.

### 🐛 Bug fixes

- Makes eas.json configuration to only run on `update:configure`. ([#1598](https://github.com/expo/eas-cli/pull/1598) by [@jonsamp](https://github.com/jonsamp))
- Fix issue with invisible build info in some terminals in the `eas build:run` and `eas build:resign` commands. ([#1602](https://github.com/expo/eas-cli/pull/1602) by [@szdziedzic](https://github.com/szdziedzic))

### 🧹 Chores
Expand Down
7 changes: 6 additions & 1 deletion packages/eas-cli/src/commands/update/configure.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import EasCommand from '../../commandUtils/EasCommand';
import { EASNonInteractiveFlag } from '../../commandUtils/flags';
import Log from '../../log';
import { RequestedPlatform } from '../../platform';
import { ensureEASUpdateIsConfiguredAsync } from '../../update/configure';
import {
ensureEASUpdateIsConfiguredAsync,
ensureEASUpdateIsConfiguredInEasJsonAsync,
} from '../../update/configure';

export default class UpdateConfigure extends EasCommand {
static override description = 'configure the project to support EAS Update';
Expand Down Expand Up @@ -46,6 +49,8 @@ export default class UpdateConfigure extends EasCommand {
platform,
});

await ensureEASUpdateIsConfiguredInEasJsonAsync(projectDir);
Copy link
Contributor

Choose a reason for hiding this comment

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

update:configure is supposed to prepare the project for updates, but with this change, you are removing all config steps and only updating eas.json. e.g. You are not checking if expo-updates is installed.

Based on the PR description I would expect that changes in this PR will not affect update:configure method and the only difference for the update call would be skipping that eas.json update.

Copy link
Member Author

@jonsamp jonsamp Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think there may be a misunderstanding here.

This line (52) is inside the runAsync() function on the update:configure command. When running update:configure, we will run two key functions:

  1. ensureEASUpdateIsConfiguredAsync(): This installs expo-updates, configures app.json, etc
  2. ensureEASUpdateIsConfiguredInEasJsonAsync(): This adds channel names to eas.json

Previously, that second function, ensureEASUpdateIsConfiguredInEasJsonAsync(), was inside of ensureEASUpdateIsConfiguredAsync(), which made it run whenever anyone ran update.

This PR removes the eas.json configuration part to only run on update:configure, and makes it so that update does not run the eas.json configuration part.

When running update, we will still install expo-updates + configure app.json etc. Here's where we call that in the update command:

eas-cli/packages/eas-cli/src/commands/update/index.ts

Line 203 in 37bdbe8

await ensureEASUpdateIsConfiguredAsync(graphqlClient, {

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, you are right, sorry. I didn't see that both functions were called in configure


Log.addNewLineIfNone();
Log.log(`🎉 Your app is configured with EAS Update!`);
Log.newLine();
Expand Down
4 changes: 1 addition & 3 deletions packages/eas-cli/src/update/configure.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ async function ensureEASUpdateIsConfiguredNativelyAsync(
* Make sure EAS Build profiles are configured to work with EAS Update by adding channels to build profiles.
*/

async function ensureEASUpdateIsConfiguredInEasJsonAsync(projectDir: string): Promise<void> {
export async function ensureEASUpdateIsConfiguredInEasJsonAsync(projectDir: string): Promise<void> {
const easJsonPath = EasJsonAccessor.formatEasJsonPath(projectDir);

if (!(await fs.pathExists(easJsonPath))) {
Expand Down Expand Up @@ -363,8 +363,6 @@ export async function ensureEASUpdateIsConfiguredAsync(
workflows,
});

await ensureEASUpdateIsConfiguredInEasJsonAsync(projectDir);

if (projectChanged || !hasExpoUpdates) {
await ensureEASUpdateIsConfiguredNativelyAsync(graphqlClient, {
exp: expWithUpdates,
Expand Down