Skip to content

Commit c034d57

Browse files
Fix lint and type errors in include_assets step
- Rename short identifiers (k, v, p) to satisfy id-length rule - Add eslint-disable for intentional no-await-in-loop in sequential copy loop - Add null guard for head in buildRelativeEntry to fix TS18048 - Run prettier fixes
1 parent bc14753 commit c034d57

File tree

2 files changed

+23
-46
lines changed

2 files changed

+23
-46
lines changed

packages/app/src/cli/services/build/steps/include-assets-step.test.ts

Lines changed: 11 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,8 @@ describe('executeIncludeAssetsStep', () => {
337337
} as unknown as ExtensionInstance,
338338
}
339339

340-
vi.mocked(fs.fileExists).mockImplementation(async (p) =>
341-
typeof p === 'string' && p.startsWith('/test/extension'),
340+
vi.mocked(fs.fileExists).mockImplementation(
341+
async (path) => typeof path === 'string' && path.startsWith('/test/extension'),
342342
)
343343
vi.mocked(fs.copyFile).mockResolvedValue()
344344
vi.mocked(fs.mkdir).mockResolvedValue()
@@ -613,8 +613,8 @@ describe('executeIncludeAssetsStep', () => {
613613
// Source files exist; destination paths don't yet (so findUniqueDestPath
614614
// resolves on the first candidate without looping). Individual tests can
615615
// override for specific scenarios.
616-
vi.mocked(fs.fileExists).mockImplementation(async (p) =>
617-
typeof p === 'string' && p.startsWith('/test/extension'),
616+
vi.mocked(fs.fileExists).mockImplementation(
617+
async (path) => typeof path === 'string' && path.startsWith('/test/extension'),
618618
)
619619
vi.mocked(fs.copyFile).mockResolvedValue()
620620
vi.mocked(fs.copyDirectoryContents).mockResolvedValue()
@@ -630,17 +630,13 @@ describe('executeIncludeAssetsStep', () => {
630630
configuration: {
631631
extensions: [
632632
{
633-
targeting: [
634-
{target: 'admin.app.intent.link', tools: './tools.json', url: '/editor'},
635-
],
633+
targeting: [{target: 'admin.app.intent.link', tools: './tools.json', url: '/editor'}],
636634
},
637635
],
638636
},
639637
} as unknown as ExtensionInstance,
640638
}
641639

642-
643-
644640
const step: LifecycleStep = {
645641
id: 'gen-manifest',
646642
name: 'Generate Manifest',
@@ -697,8 +693,6 @@ describe('executeIncludeAssetsStep', () => {
697693
} as unknown as ExtensionInstance,
698694
}
699695

700-
701-
702696
const step: LifecycleStep = {
703697
id: 'gen-manifest',
704698
name: 'Generate Manifest',
@@ -763,8 +757,6 @@ describe('executeIncludeAssetsStep', () => {
763757
} as unknown as ExtensionInstance,
764758
}
765759

766-
767-
768760
const step: LifecycleStep = {
769761
id: 'gen-manifest',
770762
name: 'Generate Manifest',
@@ -861,9 +853,7 @@ describe('executeIncludeAssetsStep', () => {
861853
type: 'include_assets',
862854
config: {
863855
generateManifest: true,
864-
inclusions: [
865-
{type: 'pattern', baseDir: 'public', include: ['**/*']},
866-
],
856+
inclusions: [{type: 'pattern', baseDir: 'public', include: ['**/*']}],
867857
},
868858
}
869859

@@ -884,8 +874,6 @@ describe('executeIncludeAssetsStep', () => {
884874
} as unknown as ExtensionInstance,
885875
}
886876

887-
888-
889877
const step: LifecycleStep = {
890878
id: 'gen-manifest',
891879
name: 'Generate Manifest',
@@ -929,19 +917,15 @@ describe('executeIncludeAssetsStep', () => {
929917
type: 'include_assets',
930918
config: {
931919
generateManifest: true,
932-
inclusions: [
933-
{type: 'configKey', key: 'targeting.tools', anchor: 'targeting'},
934-
],
920+
inclusions: [{type: 'configKey', key: 'targeting.tools', anchor: 'targeting'}],
935921
},
936922
}
937923

938924
// When
939925
await executeIncludeAssetsStep(step, contextWithConfig)
940926

941927
// Then — warning logged, inclusion treated as root entry
942-
expect(mockStdout.write).toHaveBeenCalledWith(
943-
expect.stringContaining('anchor without groupBy (or vice versa)'),
944-
)
928+
expect(mockStdout.write).toHaveBeenCalledWith(expect.stringContaining('anchor without groupBy (or vice versa)'))
945929
const manifestContent = JSON.parse(vi.mocked(fs.writeFile).mock.calls[0]![1] as string)
946930
expect(manifestContent).toHaveProperty('tools')
947931
})
@@ -959,8 +943,6 @@ describe('executeIncludeAssetsStep', () => {
959943
} as unknown as ExtensionInstance,
960944
}
961945

962-
963-
964946
const step: LifecycleStep = {
965947
id: 'gen-manifest',
966948
name: 'Generate Manifest',
@@ -1007,8 +989,6 @@ describe('executeIncludeAssetsStep', () => {
1007989
} as unknown as ExtensionInstance,
1008990
}
1009991

1010-
1011-
1012992
const step: LifecycleStep = {
1013993
id: 'gen-manifest',
1014994
name: 'Generate Manifest',
@@ -1047,9 +1027,7 @@ describe('executeIncludeAssetsStep', () => {
10471027
...mockExtension,
10481028
outputPath: '/test/output/extension.js',
10491029
configuration: {
1050-
extensions: [
1051-
{targeting: [{target: 'admin.intent.link', tools: './tools.json'}]},
1052-
],
1030+
extensions: [{targeting: [{target: 'admin.intent.link', tools: './tools.json'}]}],
10531031
},
10541032
} as unknown as ExtensionInstance,
10551033
}
@@ -1089,9 +1067,7 @@ describe('executeIncludeAssetsStep', () => {
10891067
extension: {
10901068
...mockExtension,
10911069
configuration: {
1092-
extensions: [
1093-
{targeting: [{target: 'admin.intent.link', tools: './tools.json'}]},
1094-
],
1070+
extensions: [{targeting: [{target: 'admin.intent.link', tools: './tools.json'}]}],
10951071
},
10961072
} as unknown as ExtensionInstance,
10971073
}
@@ -1138,9 +1114,7 @@ describe('executeIncludeAssetsStep', () => {
11381114
configuration: {
11391115
extensions: [
11401116
{
1141-
targeting: [
1142-
{target: 'admin.intent.link', tools: './tools.json', url: '/editor'},
1143-
],
1117+
targeting: [{target: 'admin.intent.link', tools: './tools.json', url: '/editor'}],
11441118
},
11451119
],
11461120
},

packages/app/src/cli/services/build/steps/include_assets_step.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ export async function executeIncludeAssetsStep(
150150
entry.preserveStructure,
151151
sanitizedDest,
152152
)
153-
result.pathMap.forEach((v, k) => aggregatedPathMap.set(k, v))
153+
result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val))
154154
return result.filesCopied
155155
}
156156

@@ -223,8 +223,12 @@ async function findUniqueDestPath(dir: string, filename: string): Promise<string
223223
const ext = extname(filename)
224224
const base = ext ? filename.slice(0, -ext.length) : filename
225225
let counter = 1
226+
// Sequential loop is intentional: each iteration must check the previous
227+
// result before proceeding to avoid race conditions on concurrent copies.
228+
226229
while (true) {
227230
const next = joinPath(dir, `${base}-${counter}${ext}`)
231+
// eslint-disable-next-line no-await-in-loop
228232
if (!(await fileExists(next))) return next
229233
counter++
230234
}
@@ -283,6 +287,7 @@ async function copyConfigKeyEntry(
283287
const pathMap = new Map<string, string>()
284288
let filesCopied = 0
285289

290+
/* eslint-disable no-await-in-loop */
286291
for (const sourcePath of uniquePaths) {
287292
const fullPath = joinPath(baseDir, sourcePath)
288293
const exists = await fileExists(fullPath)
@@ -317,6 +322,7 @@ async function copyConfigKeyEntry(
317322
filesCopied += copied.length
318323
}
319324
}
325+
/* eslint-enable no-await-in-loop */
320326

321327
return {filesCopied, pathMap}
322328
}
@@ -396,16 +402,15 @@ function buildRelativeEntry(item: {[key: string]: unknown}, relPath: string): {[
396402

397403
const tokens = tokenizePath(relPath)
398404
const [head, ...rest] = tokens
405+
if (!head) return item
399406
const restPath = rest.map((t) => `${t.name}${t.flatten ? '[]' : ''}`).join('.')
400407

401408
const value = item[head.name]
402409

403410
if (head.flatten) {
404411
// Array segment: map over each element with the remaining path
405412
if (!Array.isArray(value)) return {[head.name]: value}
406-
const mapped = (value as {[key: string]: unknown}[]).map((el) =>
407-
restPath ? buildRelativeEntry(el, restPath) : el,
408-
)
413+
const mapped = (value as {[key: string]: unknown}[]).map((el) => (restPath ? buildRelativeEntry(el, restPath) : el))
409414
return {[head.name]: mapped}
410415
}
411416

@@ -438,8 +443,8 @@ function resolveManifestPaths(value: unknown, pathMap: Map<string, string>): unk
438443
if (Array.isArray(value)) return value.map((el) => resolveManifestPaths(el, pathMap))
439444
if (value !== null && typeof value === 'object') {
440445
const result: {[key: string]: unknown} = {}
441-
for (const [k, v] of Object.entries(value as {[key: string]: unknown})) {
442-
result[k] = resolveManifestPaths(v, pathMap)
446+
for (const [key, val] of Object.entries(value as {[key: string]: unknown})) {
447+
result[key] = resolveManifestPaths(val, pathMap)
443448
}
444449
return result
445450
}
@@ -489,9 +494,7 @@ async function generateManifestFile(
489494

490495
// Step 1: partition configKey inclusions
491496
type ConfigKeyEntry = z.infer<typeof ConfigKeyEntrySchema>
492-
const configKeyInclusions = config.inclusions.filter(
493-
(entry): entry is ConfigKeyEntry => entry.type === 'configKey',
494-
)
497+
const configKeyInclusions = config.inclusions.filter((entry): entry is ConfigKeyEntry => entry.type === 'configKey')
495498

496499
type AnchoredEntry = ConfigKeyEntry & {anchor: string; groupBy: string}
497500

0 commit comments

Comments
 (0)