Skip to content

Conversation

@plvaldes
Copy link
Contributor

@plvaldes plvaldes commented Nov 3, 2025

WHY are these changes introduced?

Fixes https://github.com/shop/issues-develop/issues/21234

The CLI was sending the package.json name as the app version name on shopify app deploy, which was creating some confusion to users

WHAT is this pull request doing?

This PR changes the way the app version name is set and now follows these rules:

  1. The handle from the TOML file is used
  2. If there is no handle, the name is used as a fallback

How to test your changes?

  • Create an app with the CLI
  • Add a handle to the TOML file
  • Run shopify app deploy and check that this handle is the prefix for the app version name (see the output from the command)
Captura de pantalla 2025-11-04 a las 12 23 42

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Contributor Author

plvaldes commented Nov 3, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@plvaldes plvaldes force-pushed the 11-03-fix_how_app_name_is_set_from_the_apploader branch 4 times, most recently from 82771ed to b0bc6ac Compare November 4, 2025 09:56
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
79.25% (+0.03% 🔼)
13568/17120
🟡 Branches
73.11% (+0.07% 🔼)
6623/9059
🟡 Functions
79.4% (+0.04% 🔼)
3499/4407
🟡 Lines
79.6% (+0.02% 🔼)
12813/16096
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / loader.ts
93.12% 85.64%
97.06% (-0.03% 🔻)
94.13%
🔴
... / custom-oclif-loader.ts
28.13% (-2.18% 🔻)
11.9% (-1.73% 🔻)
33.33%
28.13% (-2.18% 🔻)

Test suite run success

3353 tests passing in 1372 suites.

Report generated by 🧪jest coverage report action from 64d1fc7

@plvaldes plvaldes force-pushed the 11-03-fix_how_app_name_is_set_from_the_apploader branch from b0bc6ac to 802d390 Compare November 4, 2025 10:04
}

const appConfiguration = `
name = "my_app"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many changes in the test files are like this one: name is now required in test fixtures to be able to load the app

}
}

export async function loadAppName(appDirectory: string): Promise<string> {
Copy link
Contributor Author

@plvaldes plvaldes Nov 4, 2025

Choose a reason for hiding this comment

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

We will always have at least a name, so we don't need this fallback. Thus, this is dead code

@plvaldes plvaldes force-pushed the 11-03-fix_how_app_name_is_set_from_the_apploader branch 6 times, most recently from f0f51bc to 5b2d4f9 Compare November 4, 2025 12:55
@plvaldes plvaldes marked this pull request as ready for review November 4, 2025 12:57
@plvaldes plvaldes requested a review from a team as a code owner November 4, 2025 12:57
@github-actions

This comment has been minimized.

@plvaldes plvaldes force-pushed the 11-03-fix_how_app_name_is_set_from_the_apploader branch from 5b2d4f9 to 48aad4e Compare November 4, 2025 13:03
@plvaldes plvaldes requested a review from a team as a code owner November 4, 2025 13:03
@plvaldes plvaldes force-pushed the 11-03-fix_how_app_name_is_set_from_the_apploader branch from 48aad4e to 64d1fc7 Compare November 4, 2025 14:01
@gonzaloriestra gonzaloriestra added this pull request to the merge queue Nov 4, 2025
Merged via the queue into main with commit 7a4bf98 Nov 4, 2025
30 checks passed
@gonzaloriestra gonzaloriestra deleted the 11-03-fix_how_app_name_is_set_from_the_apploader branch November 4, 2025 14:38
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.

2 participants