Skip to content

Commit 7850d24

Browse files
committed
Name the extension in the invalid project metadata error
The validation error reported the raw schema failure with no hint of which extension caused it. Wrap it in a single-line message that names the extension and the rejected keys. Keep mergeExtensionMetadata private, so the tolerant wrapper is the only entry point, add the render-path and multi-file cases for a malformed extension, and make the malformed extension contribute a valid key too, so the tests prove the whole contribution is dropped. Say in the changelog that every command that builds a project context now applies the contributed metadata, and give the same suppression hint in both output-dir warnings.
1 parent 4b0accd commit 7850d24

4 files changed

Lines changed: 100 additions & 9 deletions

File tree

news/changelog-1.11.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ All changes included in 1.11:
44

55
### `quarto preview`
66

7-
- ([#14783](https://github.com/quarto-dev/quarto-cli/issues/14783)): Fix project metadata contributed by an extension, such as `brand`, being ignored during `quarto preview`. `quarto render` already applied it.
7+
- ([#14783](https://github.com/quarto-dev/quarto-cli/issues/14783)): Fix project metadata contributed by an extension, such as `brand`, being ignored during `quarto preview`. `quarto render` already applied it. Every command that builds a project context now applies it as well, so `quarto inspect` also reports it.
88

99
## Engines
1010

src/project/project-context.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,32 @@ import { createTempContext } from "../core/temp.ts";
108108

109109
import { onCleanup } from "../core/cleanup.ts";
110110
import { warning } from "../deno_ral/log.ts";
111+
import { ZodError } from "zod";
112+
import { extensionIdString } from "../extension/extension-shared.ts";
113+
import { Extension } from "../extension/types.ts";
111114
import { Zod } from "../resources/types/zod/schema-types.ts";
112115
import { ExternalEngine } from "../resources/types/schema-types.ts";
113116

114-
export const mergeExtensionMetadata = async (
117+
// Turn a validation failure into one line that names the offending keys, so
118+
// the caller can report it without dumping the raw validation error
119+
const projectMetadataError = (extension: Extension, err: unknown) => {
120+
const reason = err instanceof ZodError
121+
? err.issues.map((issue) =>
122+
issue.path.length
123+
? `${issue.path.join(".")}: ${issue.message}`
124+
: issue.message
125+
).join("; ")
126+
: err instanceof Error
127+
? err.message
128+
: String(err);
129+
return new Error(
130+
`The extension ${
131+
extensionIdString(extension.id)
132+
} contributes invalid project metadata (${reason}).`,
133+
);
134+
};
135+
136+
const mergeExtensionMetadata = async (
115137
context: ProjectContext,
116138
extensionContext: ExtensionContext,
117139
) => {
@@ -128,7 +150,11 @@ export const mergeExtensionMetadata = async (
128150
const projectMetadata = extensions.filter((extension) =>
129151
extension.contributes.metadata?.project
130152
).map((extension) => {
131-
return Zod.ProjectConfig.parse(extension.contributes.metadata!.project);
153+
try {
154+
return Zod.ProjectConfig.parse(extension.contributes.metadata!.project);
155+
} catch (err) {
156+
throw projectMetadataError(extension, err);
157+
}
132158
});
133159
context.config.project = mergeProjectMetadata(
134160
context.config.project,
@@ -153,7 +179,7 @@ export const mergeExtensionMetadataForContext = async (
153179
throw err;
154180
}
155181
warning(
156-
`Ignoring the project metadata contributed by extensions: ${
182+
`Ignoring the project metadata contributed by extensions. ${
157183
err instanceof Error ? err.message : String(err)
158184
}`,
159185
);

src/project/types/single-file/single-file.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ export async function singleFileProjectContext(
141141
} else {
142142
warning(
143143
`An extension contributed 'output-dir: ${outputDir}' metadata for a single file.\n` +
144-
"Output will go to that directory when the file is rendered.",
144+
"Output will go to that directory when the file is rendered.\n" +
145+
"To suppress this warning, use --output-dir flag instead of extension metadata.",
145146
);
146147
}
147148
}

tests/unit/preview-extension-brand.test.ts

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*/
1515

1616
import { unitTest } from "../test.ts";
17-
import { assert, assertEquals } from "testing/asserts";
17+
import { assert, assertEquals, assertRejects } from "testing/asserts";
1818
import { join } from "../../src/deno_ral/path.ts";
1919
import { LightDarkBrandDarkFlag } from "../../src/core/brand/brand.ts";
2020
import { singleFileProjectContext } from "../../src/project/types/single-file/single-file.ts";
@@ -221,6 +221,11 @@ unitTest(
221221
},
222222
);
223223

224+
// One valid key and one key the strict project schema rejects, so a test can
225+
// tell "the whole contribution was dropped" from "it was partially applied".
226+
const MALFORMED_METADATA = ' output-dir: "_out"\n' +
227+
" not-a-project-key: true\n";
228+
224229
unitTest(
225230
"singleFileProjectContext: a malformed extension does not break a context built without renderOptions (#14783)",
226231
async () => {
@@ -232,20 +237,79 @@ unitTest(
232237

233238
try {
234239
Deno.writeTextFileSync(file, DOCUMENT);
235-
// `not-a-project-key` is rejected by the strict project schema.
236-
writeExtension(tmpDir, " not-a-project-key: true\n");
240+
writeExtension(tmpDir, MALFORMED_METADATA);
237241

238242
// preview and inspect must degrade to "metadata not applied" here.
239243
project = await singleFileProjectContext(file, notebookContext());
240244

241245
assertEquals(
242246
project.config?.project?.["output-dir"],
243247
undefined,
244-
"no metadata from the malformed extension must be applied",
248+
"the whole contribution of a malformed extension must be dropped",
245249
);
246250
} finally {
247251
project?.cleanup?.();
248252
safeRemoveSync(tmpDir, { recursive: true });
249253
}
250254
},
251255
);
256+
257+
unitTest(
258+
"projectContext: a malformed extension does not break a context built without renderOptions (#14783)",
259+
async () => {
260+
await initYamlIntelligenceResourcesFromFilesystem();
261+
262+
const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" });
263+
let project;
264+
265+
try {
266+
Deno.writeTextFileSync(
267+
join(tmpDir, "_quarto.yml"),
268+
"project:\n type: default\n",
269+
);
270+
Deno.writeTextFileSync(join(tmpDir, "index.qmd"), DOCUMENT);
271+
writeExtension(tmpDir, MALFORMED_METADATA);
272+
273+
project = await projectContext(tmpDir, notebookContext());
274+
assert(
275+
project !== undefined,
276+
"projectContext must still resolve with a malformed extension",
277+
);
278+
assertEquals(
279+
project.config?.project?.["output-dir"],
280+
undefined,
281+
"the whole contribution of a malformed extension must be dropped",
282+
);
283+
} finally {
284+
project?.cleanup?.();
285+
safeRemoveSync(tmpDir, { recursive: true });
286+
}
287+
},
288+
);
289+
290+
unitTest(
291+
"singleFileProjectContext: a malformed extension is fatal on the render path (#14783)",
292+
async () => {
293+
await initYamlIntelligenceResourcesFromFilesystem();
294+
295+
const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" });
296+
const file = join(tmpDir, "test.qmd");
297+
const nbContext = notebookContext();
298+
const services = renderServices(nbContext);
299+
const renderOptions = { services, flags: {} } as RenderOptions;
300+
301+
try {
302+
Deno.writeTextFileSync(file, DOCUMENT);
303+
writeExtension(tmpDir, MALFORMED_METADATA);
304+
305+
await assertRejects(
306+
() => singleFileProjectContext(file, nbContext, renderOptions),
307+
Error,
308+
"contributes invalid project metadata",
309+
);
310+
} finally {
311+
services.cleanup();
312+
safeRemoveSync(tmpDir, { recursive: true });
313+
}
314+
},
315+
);

0 commit comments

Comments
 (0)