-
Notifications
You must be signed in to change notification settings - Fork 207
Abort dev on app uninstall #6357
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
base: main
Are you sure you want to change the base?
Conversation
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/common/version.d.ts@@ -1 +1 @@
-export declare const CLI_KIT_VERSION = "3.84.0";
\ No newline at end of file
+export declare const CLI_KIT_VERSION = "3.84.1";
\ No newline at end of file
|
Coverage report
Test suite run success3230 tests passing in 1358 suites. Report generated by 🧪jest coverage report action from 6400230 |
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
} else if (result.status === 'remote-error' || result.status === 'unknown-error') { | ||
await this.logger.logUserErrors(result.error, event?.app.allExtensions ?? []) | ||
|
||
// Check for app uninstallation error | ||
if (result.status === 'remote-error') { | ||
const errors = result.error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If remote-error
and unknown-error
no longer do the same thing (so you need this extra if
here), let's split them in different else-ifs
, that way you don't need to check for remote-error
twice
WHY are these changes introduced?
When a developer uninstalls an app while
shopify app dev
is running, the CLI continues trying to update the dev session, resulting in repeated error messages "The app isn't installed on the specified store" on every file change. This creates a confusing experience where the dev session appears to continue but is actually broken.WHAT is this pull request doing?
This PR detects when an app has been uninstalled from the store and gracefully shuts down the dev session, similar to when a user presses 'q'.
When the uninstall error is detected:
How to test your changes?
shopify app dev
Post-release steps
None required.
Measuring impact
How do we know this change was effective? Please choose one:
Checklist