From 541e4d1f45872377926083f4bc29def3d63eab60 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 22:32:24 +0100 Subject: [PATCH 01/13] Upstream Zod error map improvements from Starlight --- .../astro/src/core/errors/zod-error-map.ts | 95 ++++++++++++------- 1 file changed, 61 insertions(+), 34 deletions(-) diff --git a/packages/astro/src/core/errors/zod-error-map.ts b/packages/astro/src/core/errors/zod-error-map.ts index 647fc0db388b..4137191e45ca 100644 --- a/packages/astro/src/core/errors/zod-error-map.ts +++ b/packages/astro/src/core/errors/zod-error-map.ts @@ -29,30 +29,56 @@ export const errorMap: ZodErrorMap = (baseError, ctx) => { } } } - let messages: string[] = [ - prefix( - baseErrorPath, - typeOrLiteralErrByPath.size ? 'Did not match union:' : 'Did not match union.', - ), - ]; + const messages: string[] = [prefix(baseErrorPath, 'Did not match union.')]; + const details: string[] = [...typeOrLiteralErrByPath.entries()] + // If type or literal error isn't common to ALL union types, + // filter it out. Can lead to confusing noise. + .filter(([, error]) => error.expected.length === baseError.unionErrors.length) + .map(([key, error]) => + key === baseErrorPath + ? // Avoid printing the key again if it's a base error + `> ${getTypeOrLiteralMsg(error)}` + : `> ${prefix(key, getTypeOrLiteralMsg(error))}`, + ); + + if (details.length === 0) { + const expectedShapes: string[] = []; + for (const unionError of baseError.unionErrors) { + const expectedShape: string[] = []; + for (const issue of unionError.issues) { + // If the issue is a nested union error, show the associated error message instead of the + // base error message. + if (issue.code === 'invalid_union') { + return errorMap(issue, ctx); + } + const relativePath = flattenErrorPath(issue.path) + .replace(baseErrorPath, '') + .replace(leadingPeriod, ''); + if ('expected' in issue && typeof issue.expected === 'string') { + expectedShape.push( + relativePath ? `${relativePath}: ${issue.expected}` : issue.expected, + ); + } else { + expectedShape.push(relativePath); + } + } + if (expectedShape.length === 1 && !expectedShape[0]?.includes(':')) { + // In this case the expected shape is not an object, but probably a literal type, e.g. `['string']`. + expectedShapes.push(expectedShape.join('')); + } else { + expectedShapes.push(`{ ${expectedShape.join('; ')} }`); + } + } + if (expectedShapes.length) { + details.push('> Expected type `' + expectedShapes.join(' | ') + '`'); + details.push('> Received `' + stringify(ctx.data) + '`'); + } + } + return { - message: messages - .concat( - [...typeOrLiteralErrByPath.entries()] - // If type or literal error isn't common to ALL union types, - // filter it out. Can lead to confusing noise. - .filter(([, error]) => error.expected.length === baseError.unionErrors.length) - .map(([key, error]) => - key === baseErrorPath - ? // Avoid printing the key again if it's a base error - `> ${getTypeOrLiteralMsg(error)}` - : `> ${prefix(key, getTypeOrLiteralMsg(error))}`, - ), - ) - .join('\n'), + message: messages.concat(details).join('\n'), }; - } - if (baseError.code === 'invalid_literal' || baseError.code === 'invalid_type') { + } else if (baseError.code === 'invalid_literal' || baseError.code === 'invalid_type') { return { message: prefix( baseErrorPath, @@ -71,29 +97,30 @@ export const errorMap: ZodErrorMap = (baseError, ctx) => { }; const getTypeOrLiteralMsg = (error: TypeOrLiteralErrByPathEntry): string => { - if (error.received === 'undefined') return 'Required'; + // received could be `undefined` or the string `'undefined'` + if (typeof error.received === 'undefined' || error.received === 'undefined') return 'Required'; const expectedDeduped = new Set(error.expected); switch (error.code) { case 'invalid_type': - return `Expected type \`${unionExpectedVals(expectedDeduped)}\`, received ${JSON.stringify( + return `Expected type \`${unionExpectedVals(expectedDeduped)}\`, received \`${stringify( error.received, - )}`; + )}\``; case 'invalid_literal': - return `Expected \`${unionExpectedVals(expectedDeduped)}\`, received ${JSON.stringify( + return `Expected \`${unionExpectedVals(expectedDeduped)}\`, received \`${stringify( error.received, - )}`; + )}\``; } }; const prefix = (key: string, msg: string) => (key.length ? `**${key}**: ${msg}` : msg); const unionExpectedVals = (expectedVals: Set) => - [...expectedVals] - .map((expectedVal, idx) => { - if (idx === 0) return JSON.stringify(expectedVal); - const sep = ' | '; - return `${sep}${JSON.stringify(expectedVal)}`; - }) - .join(''); + [...expectedVals].map((expectedVal) => stringify(expectedVal)).join(' | '); const flattenErrorPath = (errorPath: (string | number)[]) => errorPath.join('.'); + +/** `JSON.stringify()` a value with spaces around object/array entries. */ +const stringify = (val: unknown) => + JSON.stringify(val, null, 1).split(newlinePlusWhitespace).join(' '); +const newlinePlusWhitespace = /\n\s*/; +const leadingPeriod = /^\./; From 84fbe300274ae10564bb587fa321563e517fe6c0 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:04:21 +0100 Subject: [PATCH 02/13] Update tests --- packages/astro/test/content-collections.test.js | 6 +++--- packages/astro/test/error-map.test.js | 8 ++++---- packages/astro/test/legacy-content-collections.test.js | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/astro/test/content-collections.test.js b/packages/astro/test/content-collections.test.js index 19a47f7e4cc6..43c2e22a6a2c 100644 --- a/packages/astro/test/content-collections.test.js +++ b/packages/astro/test/content-collections.test.js @@ -267,7 +267,7 @@ describe('Content Collections', () => { } catch (e) { error = e.message; } - assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true); + assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/); }); }); describe('With config.mts', () => { @@ -281,7 +281,7 @@ describe('Content Collections', () => { } catch (e) { error = e.message; } - assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true); + assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/); }); }); @@ -296,7 +296,7 @@ describe('Content Collections', () => { } catch (e) { error = e.message; } - assert.equal(error.includes('**title**: Required'), true); + assert.match(error, /\*\*title\*\*: Required/); }); }); diff --git a/packages/astro/test/error-map.test.js b/packages/astro/test/error-map.test.js index 1906aaa210f3..e3f13e737190 100644 --- a/packages/astro/test/error-map.test.js +++ b/packages/astro/test/error-map.test.js @@ -30,7 +30,7 @@ describe('Content Collections - error map', () => { }), { foo: 1 }, ); - assert.deepEqual(messages(error), ['**foo**: Expected type `"string"`, received "number"']); + assert.deepEqual(messages(error), ['**foo**: Expected type `"string"`, received `"number"`']); }); it('Returns formatted error for literal mismatch', () => { const error = getParseError( @@ -39,7 +39,7 @@ describe('Content Collections - error map', () => { }), { lang: 'es' }, ); - assert.deepEqual(messages(error), ['**lang**: Expected `"en"`, received "es"']); + assert.deepEqual(messages(error), ['**lang**: Expected `"en"`, received `"es"`']); }); it('Replaces undefined errors with "Required"', () => { const error = getParseError( @@ -58,7 +58,7 @@ describe('Content Collections - error map', () => { ); assert.deepEqual(messages(error), [ fixLineEndings( - 'Did not match union:\n> Expected type `"boolean" | "number"`, received "string"', + 'Did not match union.\n> Expected type `"boolean" | "number"`, received `"string"`', ), ]); }); @@ -76,7 +76,7 @@ describe('Content Collections - error map', () => { ); assert.deepEqual(messages(error), [ fixLineEndings( - 'Did not match union:\n> **type**: Expected `"tutorial" | "article"`, received "integration-guide"', + 'Did not match union.\n> **type**: Expected `"tutorial" | "article"`, received `"integration-guide"`', ), ]); }); diff --git a/packages/astro/test/legacy-content-collections.test.js b/packages/astro/test/legacy-content-collections.test.js index 6c5335dde583..613d76e1ec25 100644 --- a/packages/astro/test/legacy-content-collections.test.js +++ b/packages/astro/test/legacy-content-collections.test.js @@ -285,7 +285,7 @@ describe('Legacy Content Collections', () => { } catch (e) { error = e.message; } - assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true); + assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/); }); }); describe('With config.mts', () => { @@ -302,7 +302,7 @@ describe('Legacy Content Collections', () => { } catch (e) { error = e.message; } - assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true); + assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/); }); }); From 49dd376af8858971226cb76e6632e7e39b62786e Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:31:59 +0100 Subject: [PATCH 03/13] Use Zod error map when validating user config --- packages/astro/src/core/config/validate.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/astro/src/core/config/validate.ts b/packages/astro/src/core/config/validate.ts index 75a64f1f7d17..c293b5b58f2e 100644 --- a/packages/astro/src/core/config/validate.ts +++ b/packages/astro/src/core/config/validate.ts @@ -1,4 +1,5 @@ import type { AstroConfig } from '../../types/public/config.js'; +import { errorMap } from '../errors/index.js'; import { createRelativeSchema } from './schema.js'; /** Turn raw config values into normalized values */ @@ -10,5 +11,5 @@ export async function validateConfig( const AstroConfigRelativeSchema = createRelativeSchema(cmd, root); // First-Pass Validation - return await AstroConfigRelativeSchema.parseAsync(userConfig); + return await AstroConfigRelativeSchema.parseAsync(userConfig, { errorMap }); } From bab3a8ccdf83b2413e5de287d26cdad9ccea2d49 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:37:30 +0100 Subject: [PATCH 04/13] Update user config error formatting for error map --- packages/astro/src/core/messages.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index 57c44b06504a..c2fd19f5427c 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -232,11 +232,19 @@ function getNetworkLogging(host: string | boolean): 'none' | 'host-to-expose' | } export function formatConfigErrorMessage(err: ZodError) { - const errorList = err.issues.map( - (issue) => ` ! ${bold(issue.path.join('.'))} ${red(issue.message + '.')}`, + const errorList = err.issues.map((issue) => + `${red('!')} ${issue.message}` + // Bold text wrapped in **...** kind of like Markdown. + .replaceAll(/\*\*([^*]+)\*\*/g, red(bold('$1'))) + // Make text wrapped in backticks blue. + .replaceAll(/`([^`]+)`/g, cyan('$1')) + // Dim all lines in an issue except for the first + .split('\n') + .map((line, index) => ' ' + (index > 0 ? dim(line) : line)) + .join('\n'), ); - return `${red('[config]')} Astro found issue(s) with your configuration:\n${errorList.join( - '\n', + return `${red('[config]')} Astro found issue(s) with your configuration:\n\n${errorList.join( + '\n\n', )}`; } From 94dcab6401acb85c4fecb8d8d342180b5fd303dc Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:42:19 +0100 Subject: [PATCH 05/13] Tweak colours --- packages/astro/src/core/messages.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index c2fd19f5427c..7e8f6ed6db4f 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -233,14 +233,14 @@ function getNetworkLogging(host: string | boolean): 'none' | 'host-to-expose' | export function formatConfigErrorMessage(err: ZodError) { const errorList = err.issues.map((issue) => - `${red('!')} ${issue.message}` + `! ${issue.message}` // Bold text wrapped in **...** kind of like Markdown. - .replaceAll(/\*\*([^*]+)\*\*/g, red(bold('$1'))) + .replaceAll(/\*\*([^*]+)\*\*/g, bold('$1')) // Make text wrapped in backticks blue. .replaceAll(/`([^`]+)`/g, cyan('$1')) // Dim all lines in an issue except for the first .split('\n') - .map((line, index) => ' ' + (index > 0 ? dim(line) : line)) + .map((line, index) => ' ' + (index > 0 ? dim(line) : red(line))) .join('\n'), ); return `${red('[config]')} Astro found issue(s) with your configuration:\n\n${errorList.join( From f0aad56ef2e73cf91933953a875ed2f14c5ed31b Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:50:48 +0100 Subject: [PATCH 06/13] Separate colour and indentation steps for clarity --- packages/astro/src/core/messages.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index 7e8f6ed6db4f..3eb885dbafd7 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -238,9 +238,11 @@ export function formatConfigErrorMessage(err: ZodError) { .replaceAll(/\*\*([^*]+)\*\*/g, bold('$1')) // Make text wrapped in backticks blue. .replaceAll(/`([^`]+)`/g, cyan('$1')) - // Dim all lines in an issue except for the first .split('\n') - .map((line, index) => ' ' + (index > 0 ? dim(line) : red(line))) + // Dim all lines in an issue except for the first which should be red. + .map((line, index) => (index > 0 ? dim(line) : red(line))) + // Indent all lines. + .map((line) => ' ' + line) .join('\n'), ); return `${red('[config]')} Astro found issue(s) with your configuration:\n\n${errorList.join( From b6f17bc0bcd0794b90de1730e1aab487319199db Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:52:11 +0100 Subject: [PATCH 07/13] Fix broken link to experimental flag documentation --- packages/astro/src/core/config/schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 464ea3de1ace..467cf137bc2b 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -566,7 +566,7 @@ export const AstroConfigSchema = z.object({ }), }) .strict( - `Invalid or outdated experimental feature.\nCheck for incorrect spelling or outdated Astro version.\nSee https://docs.astro.build/en/reference/configuration-reference/#experimental-flags for a list of all current experiments.`, + `Invalid or outdated experimental feature.\nCheck for incorrect spelling or outdated Astro version.\nSee https://docs.astro.build/en/reference/experimental-flags/ for a list of all current experiments.`, ) .default({}), legacy: z From 00ae85868bda95ad121fd4d9a3e480d6c757d7f8 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:54:58 +0100 Subject: [PATCH 08/13] Add changeset --- .changeset/heavy-buttons-compare.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/heavy-buttons-compare.md diff --git a/.changeset/heavy-buttons-compare.md b/.changeset/heavy-buttons-compare.md new file mode 100644 index 000000000000..d59e7bd37f39 --- /dev/null +++ b/.changeset/heavy-buttons-compare.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Improves error message formatting for user config and content collection frontmatter From 66674d697efedec0f9288d6f057dd2de9a02e2f8 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 4 Dec 2024 23:59:35 +0100 Subject: [PATCH 09/13] Update tests --- .../test/units/config/config-validate.test.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/astro/test/units/config/config-validate.test.js b/packages/astro/test/units/config/config-validate.test.js index 0fa67373a995..ca695a217965 100644 --- a/packages/astro/test/units/config/config-validate.test.js +++ b/packages/astro/test/units/config/config-validate.test.js @@ -23,7 +23,8 @@ describe('Config Validation', () => { assert.equal( formattedError, `[config] Astro found issue(s) with your configuration: - ! site Expected string, received number.`, + + ! site Expected type "string", received "number"`, ); }); @@ -38,8 +39,11 @@ describe('Config Validation', () => { assert.equal( formattedError, `[config] Astro found issue(s) with your configuration: - ! integrations.0 Expected object, received number. - ! build.format Invalid input.`, + + ! integrations.0: Expected type "object", received "number" + + ! build.format: Did not match union. + > Expected "file" | "directory" | "preserve", received "invalid"`, ); }); @@ -118,7 +122,10 @@ describe('Config Validation', () => { process.cwd(), ).catch((err) => err); assert.equal(configError instanceof z.ZodError, true); - assert.equal(configError.errors[0].message, 'Array must contain at least 1 element(s)'); + assert.equal( + configError.errors[0].message, + '**i18n.locales.1.codes**: Array must contain at least 1 element(s)', + ); }); it('errors if the default locale is not in path', async () => { From 7e8c2991bfc3fdb7e0c0a13bfa25505754fa992d Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Thu, 5 Dec 2024 00:00:25 +0100 Subject: [PATCH 10/13] Fix one more test --- packages/astro/test/units/config/config-validate.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/test/units/config/config-validate.test.js b/packages/astro/test/units/config/config-validate.test.js index ca695a217965..87494ac79b61 100644 --- a/packages/astro/test/units/config/config-validate.test.js +++ b/packages/astro/test/units/config/config-validate.test.js @@ -24,7 +24,7 @@ describe('Config Validation', () => { formattedError, `[config] Astro found issue(s) with your configuration: - ! site Expected type "string", received "number"`, + ! site: Expected type "string", received "number"`, ); }); From ecc28a278bac772c0e028937668ae5364d9a7959 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Thu, 5 Dec 2024 00:10:58 +0100 Subject: [PATCH 11/13] Use existing utility for bold and other Markdown formatting --- packages/astro/src/core/messages.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index 3eb885dbafd7..8189c8a02dec 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -233,9 +233,7 @@ function getNetworkLogging(host: string | boolean): 'none' | 'host-to-expose' | export function formatConfigErrorMessage(err: ZodError) { const errorList = err.issues.map((issue) => - `! ${issue.message}` - // Bold text wrapped in **...** kind of like Markdown. - .replaceAll(/\*\*([^*]+)\*\*/g, bold('$1')) + `! ${renderErrorMarkdown(issue.message, 'cli')}` // Make text wrapped in backticks blue. .replaceAll(/`([^`]+)`/g, cyan('$1')) .split('\n') From 2c93b93b89e29386861a396882d39dd15eba117b Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Thu, 5 Dec 2024 00:12:56 +0100 Subject: [PATCH 12/13] Extract codeRegex to top level --- packages/astro/src/core/messages.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index 8189c8a02dec..209fbdabacb7 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -231,11 +231,13 @@ function getNetworkLogging(host: string | boolean): 'none' | 'host-to-expose' | } } +const codeRegex = /`([^`]+)`/g; + export function formatConfigErrorMessage(err: ZodError) { const errorList = err.issues.map((issue) => `! ${renderErrorMarkdown(issue.message, 'cli')}` // Make text wrapped in backticks blue. - .replaceAll(/`([^`]+)`/g, cyan('$1')) + .replaceAll(codeRegex, cyan('$1')) .split('\n') // Dim all lines in an issue except for the first which should be red. .map((line, index) => (index > 0 ? dim(line) : red(line))) From 2170904fe9d094704fc9e5b8ab4b9385c12d47bb Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Thu, 5 Dec 2024 12:58:20 +0100 Subject: [PATCH 13/13] =?UTF-8?q?Don=E2=80=99t=20dim=20text=20and=20tweak?= =?UTF-8?q?=20indentation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/astro/src/core/messages.ts | 8 +++----- packages/astro/test/units/config/config-validate.test.js | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index 209fbdabacb7..4198e96c3794 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -237,12 +237,10 @@ export function formatConfigErrorMessage(err: ZodError) { const errorList = err.issues.map((issue) => `! ${renderErrorMarkdown(issue.message, 'cli')}` // Make text wrapped in backticks blue. - .replaceAll(codeRegex, cyan('$1')) + .replaceAll(codeRegex, blue('$1')) + // Make the first line red and indent the rest. .split('\n') - // Dim all lines in an issue except for the first which should be red. - .map((line, index) => (index > 0 ? dim(line) : red(line))) - // Indent all lines. - .map((line) => ' ' + line) + .map((line, index) => (index === 0 ? red(line) : ' ' + line)) .join('\n'), ); return `${red('[config]')} Astro found issue(s) with your configuration:\n\n${errorList.join( diff --git a/packages/astro/test/units/config/config-validate.test.js b/packages/astro/test/units/config/config-validate.test.js index 87494ac79b61..2ef08e4b6a36 100644 --- a/packages/astro/test/units/config/config-validate.test.js +++ b/packages/astro/test/units/config/config-validate.test.js @@ -24,7 +24,7 @@ describe('Config Validation', () => { formattedError, `[config] Astro found issue(s) with your configuration: - ! site: Expected type "string", received "number"`, +! site: Expected type "string", received "number"`, ); }); @@ -40,9 +40,9 @@ describe('Config Validation', () => { formattedError, `[config] Astro found issue(s) with your configuration: - ! integrations.0: Expected type "object", received "number" +! integrations.0: Expected type "object", received "number" - ! build.format: Did not match union. +! build.format: Did not match union. > Expected "file" | "directory" | "preserve", received "invalid"`, ); });