Skip to content

Commit 3d4bf3a

Browse files
Avoid problems with them extensions self built by external vite
1 parent 0810f84 commit 3d4bf3a

File tree

4 files changed

+271
-59
lines changed

4 files changed

+271
-59
lines changed

packages/app/src/cli/services/dev/app-events/app-event-watcher.ts

Lines changed: 65 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -234,48 +234,74 @@ export class AppEventWatcher extends EventEmitter {
234234
* buildForBundle method.
235235
*/
236236
private async buildExtensions(extensionEvents: ExtensionEvent[]) {
237-
const promises = extensionEvents.map(async (extEvent) => {
238-
const ext = extEvent.extension
239-
return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => {
240-
try {
241-
if (this.esbuildManager.contexts?.[ext.uid]?.length) {
242-
await this.esbuildManager.rebuildContext(ext)
243-
this.options.stdout.write(`Build successful`)
244-
} else {
245-
await this.buildExtension(ext)
246-
}
247-
extEvent.buildResult = {status: 'ok', uid: ext.uid}
248-
// eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any
249-
} catch (error: any) {
250-
// If there is an `errors` array, it's an esbuild error, format it and log it
251-
// If not, just print the error message to stderr.
252-
const errors: Message[] = error.errors ?? []
253-
let errorMessage = error.message
254-
let errorFile: string | undefined
255-
256-
if (errors.length) {
257-
// Use the first error for the main message and file
258-
const firstError = errors[0]
259-
errorMessage = firstError?.text
260-
errorFile = firstError?.location?.file
261-
262-
const formattedErrors = formatMessagesSync(errors, {kind: 'error', color: !isUnitTest()})
263-
formattedErrors.forEach((error) => {
264-
this.options.stderr.write(error)
237+
// Group events by extension to handle multiple events for the same extension
238+
const groupedExtensionEvents = extensionEvents.reduce<{extension: ExtensionInstance; events: ExtensionEvent[]}[]>(
239+
(grouped: {extension: ExtensionInstance; events: ExtensionEvent[]}[], event: ExtensionEvent) => {
240+
const existingGroup = grouped.find((group) => group.extension.uid === event.extension.uid)
241+
if (existingGroup) {
242+
existingGroup.events.push(event)
243+
} else {
244+
grouped.push({
245+
extension: event.extension,
246+
events: [event],
247+
})
248+
}
249+
return grouped
250+
},
251+
[],
252+
)
253+
254+
const promises = groupedExtensionEvents
255+
.filter(({events}) => events.length > 0)
256+
.map(async ({extension: ext, events}) => {
257+
return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => {
258+
try {
259+
if (this.esbuildManager.contexts?.[ext.uid]?.length) {
260+
await this.esbuildManager.rebuildContext(ext)
261+
this.options.stdout.write(`Build successful`)
262+
} else {
263+
await this.buildExtension(ext)
264+
}
265+
// Update all events for this extension with success result
266+
const buildResult = {status: 'ok' as const, uid: ext.uid}
267+
events.forEach((event) => {
268+
event.buildResult = buildResult
269+
})
270+
// eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any
271+
} catch (error: any) {
272+
// If there is an `errors` array, it's an esbuild error, format it and log it
273+
// If not, just print the error message to stderr.
274+
const errors: Message[] = error.errors ?? []
275+
let errorMessage = error.message
276+
let errorFile: string | undefined
277+
278+
if (errors.length) {
279+
// Use the first error for the main message and file
280+
const firstError = errors[0]
281+
errorMessage = firstError?.text
282+
errorFile = firstError?.location?.file
283+
284+
const formattedErrors = formatMessagesSync(errors, {kind: 'error', color: !isUnitTest()})
285+
formattedErrors.forEach((error) => {
286+
this.options.stderr.write(error)
287+
})
288+
} else {
289+
this.options.stderr.write(error.message)
290+
}
291+
292+
// Update all events for this extension with error result
293+
const buildResult = {
294+
status: 'error' as const,
295+
error: errorMessage,
296+
file: errorFile,
297+
uid: ext.uid,
298+
}
299+
events.forEach((event) => {
300+
event.buildResult = buildResult
265301
})
266-
} else {
267-
this.options.stderr.write(error.message)
268-
}
269-
270-
extEvent.buildResult = {
271-
status: 'error',
272-
error: errorMessage,
273-
file: errorFile,
274-
uid: ext.uid,
275302
}
276-
}
303+
})
277304
})
278-
})
279305
return Promise.all(promises)
280306
}
281307

packages/app/src/cli/services/extensions/bundle.ts

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,38 @@ export async function bundleThemeExtension(
6868
options.stdout.write(`Bundling theme extension ${extension.localIdentifier}...`)
6969
const files = await themeExtensionFiles(extension)
7070

71-
await Promise.all(
72-
files.map(function (filepath) {
71+
const results = await Promise.allSettled(
72+
files.map(async function (filepath) {
7373
const relativePathName = relativePath(extension.directory, filepath)
7474
const outputFile = joinPath(extension.outputPath, relativePathName)
75-
if (filepath === outputFile) return
76-
return copyFile(filepath, outputFile)
75+
if (filepath === outputFile) return {status: 'skipped', filepath}
76+
77+
try {
78+
await copyFile(filepath, outputFile)
79+
return {status: 'success', filepath}
80+
// eslint-disable-next-line no-catch-all/no-catch-all
81+
} catch (error) {
82+
// Log warning but don't fail the entire process
83+
// We intentionally catch all errors here to continue copying other files
84+
outputDebug(`Failed to copy file ${filepath}: ${error}`)
85+
return {status: 'failed', filepath, error}
86+
}
7787
}),
7888
)
89+
90+
// Report any failures as warnings
91+
const failures = results.filter((result) => {
92+
return result.status === 'rejected' || (result.status === 'fulfilled' && result.value?.status === 'failed')
93+
})
94+
95+
if (failures.length > 0) {
96+
const failedFiles = failures.map((failure) => {
97+
if (failure.status === 'rejected') return 'unknown file'
98+
const value = (failure as PromiseFulfilledResult<{status: string; filepath: string}>).value
99+
return value.filepath
100+
})
101+
outputDebug(`Warning: ${failures.length} file(s) could not be copied: ${failedFiles.join(', ')}`)
102+
}
79103
}
80104

81105
export async function copyFilesForExtension(
@@ -93,14 +117,39 @@ export async function copyFilesForExtension(
93117
ignore: ignored,
94118
})
95119

96-
await Promise.all(
97-
files.map(function (filepath) {
120+
const results = await Promise.allSettled(
121+
files.map(async function (filepath) {
98122
const relativePathName = relativePath(extension.directory, filepath)
99123
const outputFile = joinPath(extension.outputPath, relativePathName)
100-
if (filepath === outputFile) return
101-
return copyFile(filepath, outputFile)
124+
if (filepath === outputFile) return {status: 'skipped', filepath}
125+
126+
try {
127+
await copyFile(filepath, outputFile)
128+
return {status: 'success', filepath}
129+
// eslint-disable-next-line no-catch-all/no-catch-all
130+
} catch (error) {
131+
// Log warning but don't fail the entire process
132+
// We intentionally catch all errors here to continue copying other files
133+
outputDebug(`Failed to copy file ${filepath}: ${error}`)
134+
return {status: 'failed', filepath, error}
135+
}
102136
}),
103137
)
138+
139+
// Report any failures as warnings
140+
const failures = results.filter((result) => {
141+
return result.status === 'rejected' || (result.status === 'fulfilled' && result.value?.status === 'failed')
142+
})
143+
144+
if (failures.length > 0) {
145+
const failedFiles = failures.map((failure) => {
146+
if (failure.status === 'rejected') return 'unknown file'
147+
const value = (failure as PromiseFulfilledResult<{status: string; filepath: string}>).value
148+
return value.filepath
149+
})
150+
outputDebug(`Warning: ${failures.length} file(s) could not be copied: ${failedFiles.join(', ')}`)
151+
}
152+
104153
options.stdout.write(`${extension.localIdentifier} successfully built`)
105154
}
106155

packages/cli-kit/src/public/node/archiver.integration.test.ts

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import {zip, brotliCompress} from './archiver.js'
2-
import {fileExists, inTemporaryDirectory, mkdir, touchFile} from './fs.js'
2+
import {fileExists, inTemporaryDirectory, mkdir, touchFile, removeFile, readFile} from './fs.js'
33
import {joinPath, dirname} from './path.js'
44
import {exec} from './system.js'
5-
import {describe, expect, test} from 'vitest'
5+
import {describe, expect, test, vi} from 'vitest'
66
import StreamZip from 'node-stream-zip'
77
import brotli from 'brotli'
88
import fs from 'fs'
9+
import {glob} from './fs.js'
910

1011
describe('zip', () => {
1112
test('zips a directory', async () => {
@@ -57,6 +58,51 @@ describe('zip', () => {
5758
expect(expectedEntries.sort()).toEqual(archiveEntries.sort())
5859
})
5960
})
61+
62+
test('gracefully handles files deleted during archiving', async () => {
63+
await inTemporaryDirectory(async (tmpDir) => {
64+
// Given
65+
const zipPath = joinPath(tmpDir, 'output.zip')
66+
const outputDirectoryName = 'output'
67+
const outputDirectoryPath = joinPath(tmpDir, outputDirectoryName)
68+
const structure = ['file1.js', 'file2.js', 'file3.js']
69+
70+
await createFiles(structure, outputDirectoryPath)
71+
72+
const file2Path = joinPath(outputDirectoryPath, 'file2.js')
73+
74+
// Spy on readFile to simulate a file being deleted during archiving
75+
// We'll make readFile throw ENOENT for file2.js specifically
76+
const fsModule = await import('./fs.js')
77+
const originalReadFile = fsModule.readFile
78+
const readFileSpy = vi.spyOn(fsModule, 'readFile')
79+
readFileSpy.mockImplementation(async (path: string) => {
80+
if (path === file2Path) {
81+
const error: NodeJS.ErrnoException = new Error(`ENOENT: no such file or directory, open '${path}'`)
82+
error.code = 'ENOENT'
83+
throw error
84+
}
85+
return originalReadFile(path)
86+
})
87+
88+
// When - should not throw even though file2.js throws ENOENT
89+
await expect(
90+
zip({
91+
inputDirectory: outputDirectoryPath,
92+
outputZipPath: zipPath,
93+
}),
94+
).resolves.not.toThrow()
95+
96+
// Then - archive should contain remaining files only
97+
const archiveEntries = await readArchiveFiles(zipPath)
98+
expect(archiveEntries).toContain('file1.js')
99+
expect(archiveEntries).toContain('file3.js')
100+
// file2.js should not be in archive since readFile failed
101+
expect(archiveEntries).not.toContain('file2.js')
102+
103+
readFileSpy.mockRestore()
104+
})
105+
})
60106
})
61107

62108
describe('brotliCompress', () => {
@@ -141,6 +187,48 @@ describe('brotliCompress', () => {
141187
expect(fs.existsSync(joinPath(extractPath, 'test.json'))).toBeFalsy()
142188
})
143189
})
190+
191+
test('gracefully handles files deleted during compression', async () => {
192+
await inTemporaryDirectory(async (tmpDir) => {
193+
// Given
194+
const brotliPath = joinPath(tmpDir, 'output.br')
195+
const outputDirectoryName = 'output'
196+
const outputDirectoryPath = joinPath(tmpDir, outputDirectoryName)
197+
const structure = ['file1.js', 'file2.js', 'file3.js']
198+
199+
await createFiles(structure, outputDirectoryPath)
200+
201+
const file2Path = joinPath(outputDirectoryPath, 'file2.js')
202+
203+
// Spy on readFile to simulate a file being deleted during compression
204+
// We'll make readFile throw ENOENT for file2.js specifically
205+
const fsModule = await import('./fs.js')
206+
const originalReadFile = fsModule.readFile
207+
const readFileSpy = vi.spyOn(fsModule, 'readFile')
208+
readFileSpy.mockImplementation(async (path: string) => {
209+
if (path === file2Path) {
210+
const error: NodeJS.ErrnoException = new Error(`ENOENT: no such file or directory, open '${path}'`)
211+
error.code = 'ENOENT'
212+
throw error
213+
}
214+
return originalReadFile(path)
215+
})
216+
217+
// When - should not throw even though file2.js throws ENOENT
218+
await expect(
219+
brotliCompress({
220+
inputDirectory: outputDirectoryPath,
221+
outputPath: brotliPath,
222+
}),
223+
).resolves.not.toThrow()
224+
225+
// Then - compressed file should exist and be valid
226+
const exists = await fileExists(brotliPath)
227+
expect(exists).toBeTruthy()
228+
229+
readFileSpy.mockRestore()
230+
})
231+
})
144232
})
145233

146234
async function createFiles(structure: string[], directory: string) {

0 commit comments

Comments
 (0)