From ef53a59c3848b0a344ea381b17b4ec693ba7fb5c Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Wed, 13 May 2026 00:50:38 +0000 Subject: [PATCH] [Refactor] Fix incorrect Promise.all usage in app preDeployValidation Corrected the Promise.all([this.allExtensions.map(...)]) anti-pattern to properly await extension validations. Added a regression test to ensure that rejections in extension validation are correctly caught by the app's preDeployValidation method. --- packages/app/src/cli/models/app/app.test.ts | 34 +++++++++++++++++++++ packages/app/src/cli/models/app/app.ts | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/models/app/app.test.ts b/packages/app/src/cli/models/app/app.test.ts index 954c5a22159..ed48a11f709 100644 --- a/packages/app/src/cli/models/app/app.test.ts +++ b/packages/app/src/cli/models/app/app.test.ts @@ -851,3 +851,37 @@ function createPackageJson(tmpDir: string, type: string, version: string) { const dirPath = joinPath(tmpDir, 'node_modules', '@shopify', type) return mkdir(dirPath).then(() => writeFile(packagePath, JSON.stringify(packageJson))) } + +describe('preDeployValidation awaiting', () => { + test('awaits extension validation and catches rejections', async () => { + // Given + const extension = await testUIExtension() + vi.spyOn(extension, 'preDeployValidation').mockRejectedValue(new Error('Validation failed')) + + const app = testApp({ + allExtensions: [extension], + }) + + // When / Then + await expect(app.preDeployValidation()).rejects.toThrow('Validation failed') + }) + + test('awaits validation for multiple extensions and catches rejections', async () => { + // Given + const validExtension = await testUIExtension() + const invalidExtension = await testUIExtension() + const validPreDeployValidation = vi.spyOn(validExtension, 'preDeployValidation').mockResolvedValue() + const invalidPreDeployValidation = vi + .spyOn(invalidExtension, 'preDeployValidation') + .mockRejectedValue(new Error('Validation failed')) + + const app = testApp({ + allExtensions: [validExtension, invalidExtension], + }) + + // When / Then + await expect(app.preDeployValidation()).rejects.toThrow('Validation failed') + expect(validPreDeployValidation).toHaveBeenCalledOnce() + expect(invalidPreDeployValidation).toHaveBeenCalledOnce() + }) +}) diff --git a/packages/app/src/cli/models/app/app.ts b/packages/app/src/cli/models/app/app.ts index df707939e2b..3bf2b55b38b 100644 --- a/packages/app/src/cli/models/app/app.ts +++ b/packages/app/src/cli/models/app/app.ts @@ -405,7 +405,7 @@ export class App< } } - await Promise.all([this.allExtensions.map((ext) => ext.preDeployValidation())]) + await Promise.all(this.allExtensions.map((ext) => ext.preDeployValidation())) } extensionsForType(specification: {identifier: string; externalIdentifier: string}): ExtensionInstance[] {