From 65ca9b2a1c5a756ca8587b364636ff732db19c22 Mon Sep 17 00:00:00 2001 From: Mitchell Hentges Date: Mon, 16 Jan 2023 15:51:06 -0800 Subject: [PATCH 1/4] Remove `react-dev-utils` error compatibility workaround To work around a `react-dev-utils` compatibility problem, errors and warnings had context stripped before being formatted. Since this workaround was was only needed until `react-dev-utils` v5 was release (now over a year ago, nice! [1]), I think that it can be removed. [1] https://github.com/facebook/create-react-app/releases/tag/v5.0.0 Signed-off-by: Mitchell Hentges --- packages/cli/src/lib/bundler/bundle.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/lib/bundler/bundle.ts b/packages/cli/src/lib/bundler/bundle.ts index a8967bb250..32e4c1a181 100644 --- a/packages/cli/src/lib/bundler/bundle.ts +++ b/packages/cli/src/lib/bundler/bundle.ts @@ -122,15 +122,9 @@ async function build(config: webpack.Configuration, isCi: boolean) { warnings: true, errors: true, }); - // NOTE(freben): The code below that extracts the message part of the errors, - // is due to react-dev-utils not yet being compatible with webpack 5. This - // may be possible to remove (just passing the serialized stats object - // directly into the format function) after a new release of react-dev-utils - // has been made available. - // See https://github.com/facebook/create-react-app/issues/9880 const { errors, warnings } = formatWebpackMessages({ - errors: serializedStats.errors?.map(e => (e.message ? e.message : e)), - warnings: serializedStats.warnings?.map(e => (e.message ? e.message : e)), + errors: serializedStats.errors, + warnings: serializedStats.warnings, }); if (errors.length) { From edd9c270b63b1c7c5dd19f36adf432f3216bafb2 Mon Sep 17 00:00:00 2001 From: Mitchell Hentges Date: Wed, 18 Jan 2023 14:09:37 -0800 Subject: [PATCH 2/4] Simplify redundant `catch()` during CLI bundling Remove extra "Failed to compile" message that was redundant, and inline "Failed to compile" message into upstream errors. Note that now _other_ errors (such as `webpack(...)` failures) will not be prefixed with "Failed to compile". However, it should be sufficiently obvious that they're serious failures, so we should be OK without the extra information. Signed-off-by: Mitchell Hentges --- packages/cli/src/lib/bundler/bundle.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/lib/bundler/bundle.ts b/packages/cli/src/lib/bundler/bundle.ts index 32e4c1a181..9b146aa115 100644 --- a/packages/cli/src/lib/bundler/bundle.ts +++ b/packages/cli/src/lib/bundler/bundle.ts @@ -63,10 +63,7 @@ export async function buildBundle(options: BuildOptions) { ); } - const { stats } = await build(config, isCi).catch(error => { - console.log(chalk.red('Failed to compile.\n')); - throw new Error(`Failed to compile.\n${error.message || error}`); - }); + const { stats } = await build(config, isCi); if (!stats) { throw new Error('No stats returned'); @@ -114,7 +111,7 @@ async function build(config: webpack.Configuration, isCi: boolean) { ); if (!stats) { - throw new Error('No stats provided'); + throw new Error('Failed to compile: No stats provided'); } const serializedStats = stats.toJson({ @@ -130,7 +127,7 @@ async function build(config: webpack.Configuration, isCi: boolean) { if (errors.length) { // Only keep the first error. Others are often indicative // of the same problem, but confuse the reader with noise. - throw new Error(errors[0]); + throw new Error(`Failed to compile.\n${errors[0]}`); } if (isCi && warnings.length) { console.log( @@ -138,7 +135,7 @@ async function build(config: webpack.Configuration, isCi: boolean) { '\nTreating warnings as errors because process.env.CI = true.\n', ), ); - throw new Error(warnings.join('\n\n')); + throw new Error(`Failed to compile.\n${warnings.join('\n\n')}`); } return { stats }; From 281598105708d1db80ab6d77cfd4f99806d329fe Mon Sep 17 00:00:00 2001 From: Mitchell Hentges Date: Tue, 17 Jan 2023 16:18:34 -0800 Subject: [PATCH 3/4] Make build error cause more clear by prepending with module name Simplify the diagnosis of build failures by showing the module name encountering the issue right before the error itself is printed. Also apply the fix to warnings. Fixes #15815 Signed-off-by: Mitchell Hentges --- .changeset/eight-hotels-sparkle.md | 5 +++++ packages/cli/src/lib/bundler/bundle.ts | 18 ++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 .changeset/eight-hotels-sparkle.md diff --git a/.changeset/eight-hotels-sparkle.md b/.changeset/eight-hotels-sparkle.md new file mode 100644 index 0000000000..6846630e02 --- /dev/null +++ b/.changeset/eight-hotels-sparkle.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Show module name causing error during build diff --git a/packages/cli/src/lib/bundler/bundle.ts b/packages/cli/src/lib/bundler/bundle.ts index 9b146aa115..ba553b3b76 100644 --- a/packages/cli/src/lib/bundler/bundle.ts +++ b/packages/cli/src/lib/bundler/bundle.ts @@ -32,6 +32,10 @@ import chalk from 'chalk'; const WARN_AFTER_BUNDLE_GZIP_SIZE = 512 * 1024; const WARN_AFTER_CHUNK_GZIP_SIZE = 1024 * 1024; +function applyContextToError(error: string, moduleName: string): string { + return `Failed to compile '${moduleName}':\n ${error}`; +} + export async function buildBundle(options: BuildOptions) { const { statsJsonEnabled, schema: configSchema } = options; @@ -127,15 +131,25 @@ async function build(config: webpack.Configuration, isCi: boolean) { if (errors.length) { // Only keep the first error. Others are often indicative // of the same problem, but confuse the reader with noise. - throw new Error(`Failed to compile.\n${errors[0]}`); + const errorWithContext = applyContextToError( + errors[0], + serializedStats.errors?.[0].moduleName || '', + ); + throw new Error(errorWithContext); } if (isCi && warnings.length) { + const warningsWithContext = warnings.map((warning, i) => { + return applyContextToError( + warning, + serializedStats.warnings?.[i].moduleName || '', + ); + }); console.log( chalk.yellow( '\nTreating warnings as errors because process.env.CI = true.\n', ), ); - throw new Error(`Failed to compile.\n${warnings.join('\n\n')}`); + throw new Error(warningsWithContext.join('\n\n')); } return { stats }; From 77c980b3ea2c3222c7ac943022e1af609f01ec4a Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 25 Jan 2023 11:01:00 +0100 Subject: [PATCH 4/4] chore: fix code review comments whilst mitch is away Signed-off-by: blam --- packages/cli/src/lib/bundler/bundle.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lib/bundler/bundle.ts b/packages/cli/src/lib/bundler/bundle.ts index ba553b3b76..b26c2f9f0d 100644 --- a/packages/cli/src/lib/bundler/bundle.ts +++ b/packages/cli/src/lib/bundler/bundle.ts @@ -133,7 +133,7 @@ async function build(config: webpack.Configuration, isCi: boolean) { // of the same problem, but confuse the reader with noise. const errorWithContext = applyContextToError( errors[0], - serializedStats.errors?.[0].moduleName || '', + serializedStats.errors?.[0]?.moduleName ?? '', ); throw new Error(errorWithContext); } @@ -141,7 +141,7 @@ async function build(config: webpack.Configuration, isCi: boolean) { const warningsWithContext = warnings.map((warning, i) => { return applyContextToError( warning, - serializedStats.warnings?.[i].moduleName || '', + serializedStats.warnings?.[i]?.moduleName ?? '', ); }); console.log(