Skip to content

Commit ef53a59

Browse files
[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.
1 parent 25e9a22 commit ef53a59

2 files changed

Lines changed: 35 additions & 1 deletion

File tree

packages/app/src/cli/models/app/app.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,3 +851,37 @@ function createPackageJson(tmpDir: string, type: string, version: string) {
851851
const dirPath = joinPath(tmpDir, 'node_modules', '@shopify', type)
852852
return mkdir(dirPath).then(() => writeFile(packagePath, JSON.stringify(packageJson)))
853853
}
854+
855+
describe('preDeployValidation awaiting', () => {
856+
test('awaits extension validation and catches rejections', async () => {
857+
// Given
858+
const extension = await testUIExtension()
859+
vi.spyOn(extension, 'preDeployValidation').mockRejectedValue(new Error('Validation failed'))
860+
861+
const app = testApp({
862+
allExtensions: [extension],
863+
})
864+
865+
// When / Then
866+
await expect(app.preDeployValidation()).rejects.toThrow('Validation failed')
867+
})
868+
869+
test('awaits validation for multiple extensions and catches rejections', async () => {
870+
// Given
871+
const validExtension = await testUIExtension()
872+
const invalidExtension = await testUIExtension()
873+
const validPreDeployValidation = vi.spyOn(validExtension, 'preDeployValidation').mockResolvedValue()
874+
const invalidPreDeployValidation = vi
875+
.spyOn(invalidExtension, 'preDeployValidation')
876+
.mockRejectedValue(new Error('Validation failed'))
877+
878+
const app = testApp({
879+
allExtensions: [validExtension, invalidExtension],
880+
})
881+
882+
// When / Then
883+
await expect(app.preDeployValidation()).rejects.toThrow('Validation failed')
884+
expect(validPreDeployValidation).toHaveBeenCalledOnce()
885+
expect(invalidPreDeployValidation).toHaveBeenCalledOnce()
886+
})
887+
})

packages/app/src/cli/models/app/app.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ export class App<
405405
}
406406
}
407407

408-
await Promise.all([this.allExtensions.map((ext) => ext.preDeployValidation())])
408+
await Promise.all(this.allExtensions.map((ext) => ext.preDeployValidation()))
409409
}
410410

411411
extensionsForType(specification: {identifier: string; externalIdentifier: string}): ExtensionInstance[] {

0 commit comments

Comments
 (0)