Skip to content

Commit ef2ca4e

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 13b52d1 commit ef2ca4e

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
@@ -122,7 +122,7 @@ export async function executeIncludeAssetsStep(
122122
entry.preserveStructure,
123123
entry.destination,
124124
)
125-
result.pathMap.forEach((v, k) => aggregatedPathMap.set(k, v))
125+
result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val))
126126
return result.filesCopied
127127
}
128128

@@ -195,8 +195,12 @@ async function findUniqueDestPath(dir: string, filename: string): Promise<string
195195
const ext = extname(filename)
196196
const base = ext ? filename.slice(0, -ext.length) : filename
197197
let counter = 1
198+
// Sequential loop is intentional: each iteration must check the previous
199+
// result before proceeding to avoid race conditions on concurrent copies.
200+
198201
while (true) {
199202
const next = joinPath(dir, `${base}-${counter}${ext}`)
203+
// eslint-disable-next-line no-await-in-loop
200204
if (!(await fileExists(next))) return next
201205
counter++
202206
}
@@ -255,6 +259,7 @@ async function copyConfigKeyEntry(
255259
const pathMap = new Map<string, string>()
256260
let filesCopied = 0
257261

262+
/* eslint-disable no-await-in-loop */
258263
for (const sourcePath of uniquePaths) {
259264
const fullPath = joinPath(baseDir, sourcePath)
260265
const exists = await fileExists(fullPath)
@@ -289,6 +294,7 @@ async function copyConfigKeyEntry(
289294
filesCopied += copied.length
290295
}
291296
}
297+
/* eslint-enable no-await-in-loop */
292298

293299
return {filesCopied, pathMap}
294300
}
@@ -363,16 +369,15 @@ function buildRelativeEntry(item: {[key: string]: unknown}, relPath: string): {[
363369

364370
const tokens = tokenizePath(relPath)
365371
const [head, ...rest] = tokens
372+
if (!head) return item
366373
const restPath = rest.map((t) => `${t.name}${t.flatten ? '[]' : ''}`).join('.')
367374

368375
const value = item[head.name]
369376

370377
if (head.flatten) {
371378
// Array segment: map over each element with the remaining path
372379
if (!Array.isArray(value)) return {[head.name]: value}
373-
const mapped = (value as {[key: string]: unknown}[]).map((el) =>
374-
restPath ? buildRelativeEntry(el, restPath) : el,
375-
)
380+
const mapped = (value as {[key: string]: unknown}[]).map((el) => (restPath ? buildRelativeEntry(el, restPath) : el))
376381
return {[head.name]: mapped}
377382
}
378383

@@ -405,8 +410,8 @@ function resolveManifestPaths(value: unknown, pathMap: Map<string, string>): unk
405410
if (Array.isArray(value)) return value.map((el) => resolveManifestPaths(el, pathMap))
406411
if (value !== null && typeof value === 'object') {
407412
const result: {[key: string]: unknown} = {}
408-
for (const [k, v] of Object.entries(value as {[key: string]: unknown})) {
409-
result[k] = resolveManifestPaths(v, pathMap)
413+
for (const [key, val] of Object.entries(value as {[key: string]: unknown})) {
414+
result[key] = resolveManifestPaths(val, pathMap)
410415
}
411416
return result
412417
}
@@ -456,9 +461,7 @@ async function generateManifestFile(
456461

457462
// Step 1: partition configKey inclusions
458463
type ConfigKeyEntry = z.infer<typeof ConfigKeyEntrySchema>
459-
const configKeyInclusions = config.inclusions.filter(
460-
(entry): entry is ConfigKeyEntry => entry.type === 'configKey',
461-
)
464+
const configKeyInclusions = config.inclusions.filter((entry): entry is ConfigKeyEntry => entry.type === 'configKey')
462465

463466
type AnchoredEntry = ConfigKeyEntry & {anchor: string; groupBy: string}
464467

0 commit comments

Comments
 (0)