Skip to content

Commit f7ea873

Browse files
committed
fix(assets): strict template rendering and deterministic sort
Address review feedback on the AssetManager: - Handlebars strict mode: a template referencing an undefined variable now throws instead of silently rendering an empty string into a generated project file. - drop the speculative `docker` branch from the ignore-template rename; only git/npm ignores ship as assets today. - use code-unit ordering for embedded files so both list paths (embedded and filesystem) sort identically and deterministically.
1 parent 356105c commit f7ea873

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

src/assetManager/AssetManager.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,19 @@ describe("AssetManager", () => {
7474
);
7575
});
7676

77+
test("throws on a template referencing an undefined variable", async () => {
78+
const root = await makeTempDirectory();
79+
const source = join(root, "assets", "cdk");
80+
const destination = join(root, "output");
81+
82+
await mkdir(source, { recursive: true });
83+
await Bun.write(join(source, "README.md"), "Hello {{projectName}}");
84+
85+
await expect(
86+
new AssetManager([], join(root, "assets")).render("cdk", destination),
87+
).rejects.toThrow(/projectName/);
88+
});
89+
7790
test("rejects an embedded path that escapes the destination", async () => {
7891
const root = await makeTempDirectory();
7992
const destination = join(root, "output");

src/assetManager/AssetManager.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ export class AssetManager {
4242
const resolvedDestination = resolve(destination);
4343
for (const file of assetFiles) {
4444
const outputPath = this.resolveOutputPath(resolvedDestination, file.relativePath);
45-
const rendered = this.handlebars.compile(await file.text(), { noEscape: true })(variables);
45+
const rendered = this.handlebars.compile(await file.text(), {
46+
noEscape: true,
47+
strict: true,
48+
})(variables);
4649

4750
await mkdir(dirname(outputPath), { recursive: true });
4851
await atomicWrite(outputPath, rendered);
@@ -81,7 +84,7 @@ export class AssetManager {
8184
relativePath: file.name.slice(prefix.length),
8285
text: () => file.text(),
8386
}))
84-
.sort((a, b) => a.relativePath.localeCompare(b.relativePath));
87+
.sort((a, b) => (a.relativePath < b.relativePath ? -1 : 1));
8588
}
8689

8790
private resolveOutputPath(resolvedDestination: string, relativePath: string): string {
@@ -102,7 +105,7 @@ export class AssetManager {
102105

103106
private resolveTemplateName(relativePath: string): string {
104107
const filename = basename(relativePath);
105-
const ignore = filename.match(/^(docker|git|npm)ignore\.template$/);
108+
const ignore = filename.match(/^(git|npm)ignore\.template$/);
106109
return join(dirname(relativePath), ignore ? `.${ignore[1]}ignore` : filename);
107110
}
108111
}

0 commit comments

Comments
 (0)