From eb1d5f11278cfee832e888801fae89cf16f92787 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Jan 2024 18:26:44 -0500 Subject: [PATCH 1/2] feat: more helpful errors when using incompatible node modules --- .changeset/light-tips-hang.md | 6 ++ packages/adapter-cloudflare-workers/index.js | 68 ++++++++++++++---- packages/adapter-cloudflare/index.js | 72 +++++++++++++++----- 3 files changed, 116 insertions(+), 30 deletions(-) create mode 100644 .changeset/light-tips-hang.md diff --git a/.changeset/light-tips-hang.md b/.changeset/light-tips-hang.md new file mode 100644 index 000000000000..92cb4d5024d2 --- /dev/null +++ b/.changeset/light-tips-hang.md @@ -0,0 +1,6 @@ +--- +'@sveltejs/adapter-cloudflare-workers': minor +'@sveltejs/adapter-cloudflare': minor +--- + +feat: more helpful errors when using incompatible Node modules diff --git a/packages/adapter-cloudflare-workers/index.js b/packages/adapter-cloudflare-workers/index.js index 53b3e09795fc..c62c8153c4ba 100644 --- a/packages/adapter-cloudflare-workers/index.js +++ b/packages/adapter-cloudflare-workers/index.js @@ -82,21 +82,61 @@ export default function ({ config = 'wrangler.toml' } = {}) { external.push(...compatible_node_modules.map((id) => `node:${id}`)); } - await esbuild.build({ - platform: 'browser', - conditions: ['worker', 'browser'], - sourcemap: 'linked', - target: 'es2022', - entryPoints: [`${tmp}/entry.js`], - outfile: main, - bundle: true, - external, - alias: Object.fromEntries(compatible_node_modules.map((id) => [id, `node:${id}`])), - format: 'esm', - loader: { - '.wasm': 'copy' + try { + const result = await esbuild.build({ + platform: 'browser', + conditions: ['worker', 'browser'], + sourcemap: 'linked', + target: 'es2022', + entryPoints: [`${tmp}/entry.js`], + outfile: main, + bundle: true, + external, + alias: Object.fromEntries(compatible_node_modules.map((id) => [id, `node:${id}`])), + format: 'esm', + loader: { + '.wasm': 'copy' + }, + logLevel: 'silent' + }); + + if (result.warnings.length > 0) { + const formatted = await esbuild.formatMessages(result.warnings, { + kind: 'warning', + color: true + }); + + console.error(formatted.join('\n')); } - }); + } catch (error) { + for (const e of error.errors) { + for (const node of e.notes) { + const match = + /The package "(.+)" wasn't found on the file system but is built into node/.exec( + node.text + ); + if (match) { + let id = match[1]; + if (!id.startsWith('node:')) id = `node:${id}`; + + node.text = `Cannot use "${id}" when deploying to Cloudflare.`; + } + } + } + + const formatted = await esbuild.formatMessages(error.errors, { + kind: 'error', + color: true + }); + + console.error(formatted.join('\n')); + + throw new Error( + `Bundling with esbuild failed with ${error.errors.length} ${ + error.errors.length === 1 ? 'error' : 'errors' + }` + ); + } builder.log.minor('Copying assets...'); const bucket_dir = `${site.bucket}${builder.config.kit.paths.base}`; diff --git a/packages/adapter-cloudflare/index.js b/packages/adapter-cloudflare/index.js index 0acb132d4f82..7c1bca13202c 100644 --- a/packages/adapter-cloudflare/index.js +++ b/packages/adapter-cloudflare/index.js @@ -70,22 +70,62 @@ export default function (options = {}) { const external = ['cloudflare:*', ...compatible_node_modules.map((id) => `node:${id}`)]; - await esbuild.build({ - platform: 'browser', - conditions: ['worker', 'browser'], - sourcemap: 'linked', - target: 'es2022', - entryPoints: [`${tmp}/_worker.js`], - outfile: `${dest}/_worker.js`, - allowOverwrite: true, - format: 'esm', - bundle: true, - loader: { - '.wasm': 'copy' - }, - external, - alias: Object.fromEntries(compatible_node_modules.map((id) => [id, `node:${id}`])) - }); + try { + const result = await esbuild.build({ + platform: 'browser', + conditions: ['worker', 'browser'], + sourcemap: 'linked', + target: 'es2022', + entryPoints: [`${tmp}/_worker.js`], + outfile: `${dest}/_worker.js`, + allowOverwrite: true, + format: 'esm', + bundle: true, + loader: { + '.wasm': 'copy' + }, + external, + alias: Object.fromEntries(compatible_node_modules.map((id) => [id, `node:${id}`])), + logLevel: 'silent' + }); + + if (result.warnings.length > 0) { + const formatted = await esbuild.formatMessages(result.warnings, { + kind: 'warning', + color: true + }); + + console.error(formatted.join('\n')); + } + } catch (error) { + for (const e of error.errors) { + for (const node of e.notes) { + const match = + /The package "(.+)" wasn't found on the file system but is built into node/.exec( + node.text + ); + if (match) { + let id = match[1]; + if (!id.startsWith('node:')) id = `node:${id}`; + + node.text = `Cannot use "${id}" when deploying to Cloudflare.`; + } + } + } + + const formatted = await esbuild.formatMessages(error.errors, { + kind: 'error', + color: true + }); + + console.error(formatted.join('\n')); + + throw new Error( + `Bundling with esbuild failed with ${error.errors.length} ${ + error.errors.length === 1 ? 'error' : 'errors' + }` + ); + } } }; } From 5571c0666b1dfd403c24046f1228ce4c5dfdc90c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Jan 2024 18:40:31 -0500 Subject: [PATCH 2/2] don't bother prefixing the id --- packages/adapter-cloudflare-workers/index.js | 6 ++---- packages/adapter-cloudflare/index.js | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/adapter-cloudflare-workers/index.js b/packages/adapter-cloudflare-workers/index.js index c62c8153c4ba..e5874cf186d5 100644 --- a/packages/adapter-cloudflare-workers/index.js +++ b/packages/adapter-cloudflare-workers/index.js @@ -115,11 +115,9 @@ export default function ({ config = 'wrangler.toml' } = {}) { /The package "(.+)" wasn't found on the file system but is built into node/.exec( node.text ); - if (match) { - let id = match[1]; - if (!id.startsWith('node:')) id = `node:${id}`; - node.text = `Cannot use "${id}" when deploying to Cloudflare.`; + if (match) { + node.text = `Cannot use "${match[1]}" when deploying to Cloudflare.`; } } } diff --git a/packages/adapter-cloudflare/index.js b/packages/adapter-cloudflare/index.js index 7c1bca13202c..612b432c9dc4 100644 --- a/packages/adapter-cloudflare/index.js +++ b/packages/adapter-cloudflare/index.js @@ -104,11 +104,9 @@ export default function (options = {}) { /The package "(.+)" wasn't found on the file system but is built into node/.exec( node.text ); - if (match) { - let id = match[1]; - if (!id.startsWith('node:')) id = `node:${id}`; - node.text = `Cannot use "${id}" when deploying to Cloudflare.`; + if (match) { + node.text = `Cannot use "${match[1]}" when deploying to Cloudflare.`; } } }