From 09eca9be5e27e57d49e5af6a8fa9c4018ceb0b85 Mon Sep 17 00:00:00 2001
From: Matthew Phillips <matthew@skypack.dev>
Date: Wed, 3 Aug 2022 14:33:31 -0400
Subject: [PATCH] Only rethrow renderer error when no renderer found (#4131)

* Only rethrow renderer error when no renderer found

* Adds a changeset
---
 .changeset/lazy-cameras-float.md              |  5 ++++
 packages/astro/src/runtime/server/index.ts    |  4 ++-
 .../multiple-renderers/astro.config.mjs       |  6 ++++
 .../fixtures/multiple-renderers/package.json  |  8 ++++++
 .../renderers/one/index.mjs                   | 15 ++++++++++
 .../renderers/one/package.json                |  5 ++++
 .../renderers/one/server.mjs                  | 11 ++++++++
 .../renderers/two/index.mjs                   | 15 ++++++++++
 .../renderers/two/package.json                |  5 ++++
 .../renderers/two/server.mjs                  | 11 ++++++++
 .../multiple-renderers/src/pages/index.astro  | 14 ++++++++++
 .../astro/test/multiple-renderers.test.js     | 21 ++++++++++++++
 pnpm-lock.yaml                                | 28 +++++++++++++++++++
 13 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/lazy-cameras-float.md
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/astro.config.mjs
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/package.json
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/renderers/one/index.mjs
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/renderers/one/package.json
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/renderers/one/server.mjs
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/renderers/two/index.mjs
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/renderers/two/package.json
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/renderers/two/server.mjs
 create mode 100644 packages/astro/test/fixtures/multiple-renderers/src/pages/index.astro
 create mode 100644 packages/astro/test/multiple-renderers.test.js

diff --git a/.changeset/lazy-cameras-float.md b/.changeset/lazy-cameras-float.md
new file mode 100644
index 000000000..4e680c003
--- /dev/null
+++ b/.changeset/lazy-cameras-float.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes use of multiple renderers when one throws
diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts
index ebec0487a..9a655d380 100644
--- a/packages/astro/src/runtime/server/index.ts
+++ b/packages/astro/src/runtime/server/index.ts
@@ -310,7 +310,9 @@ Did you mean to add ${formatList(probableRendererNames.map((r) => '`' + r + '`')
 				}
 			}
 
-			if (error) {
+			// If no renderer is found and there is an error, throw that error because
+			// it is likely a problem with the component code.
+			if (!renderer && error) {
 				throw error;
 			}
 		}
diff --git a/packages/astro/test/fixtures/multiple-renderers/astro.config.mjs b/packages/astro/test/fixtures/multiple-renderers/astro.config.mjs
new file mode 100644
index 000000000..526323dbf
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/astro.config.mjs
@@ -0,0 +1,6 @@
+import one from '@astrojs/renderer-one';
+import two from '@astrojs/renderer-two';
+
+export default {
+	integrations: [one(), two()]
+};
diff --git a/packages/astro/test/fixtures/multiple-renderers/package.json b/packages/astro/test/fixtures/multiple-renderers/package.json
new file mode 100644
index 000000000..9372a6e94
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "@test/multiple-renderers",
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/renderer-one": "file:./renderers/one",
+    "@astrojs/renderer-two": "file:./renderers/two"
+  }
+}
diff --git a/packages/astro/test/fixtures/multiple-renderers/renderers/one/index.mjs b/packages/astro/test/fixtures/multiple-renderers/renderers/one/index.mjs
new file mode 100644
index 000000000..3e39696bf
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/renderers/one/index.mjs
@@ -0,0 +1,15 @@
+
+export default function() {
+	return {
+		name: 'renderer-one',
+		hooks: {
+			'astro:config:setup': ({ addRenderer }) => {
+				addRenderer({
+					name: 'renderer-one',
+					clientEntrypoint: null,
+					serverEntrypoint: '@astrojs/renderer-one/server.mjs',
+				});
+			}
+		}
+	};
+}
diff --git a/packages/astro/test/fixtures/multiple-renderers/renderers/one/package.json b/packages/astro/test/fixtures/multiple-renderers/renderers/one/package.json
new file mode 100644
index 000000000..30a2cf6de
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/renderers/one/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "@astrojs/renderer-one",
+  "version": "1.0.0",
+  "main": "index.mjs"
+}
diff --git a/packages/astro/test/fixtures/multiple-renderers/renderers/one/server.mjs b/packages/astro/test/fixtures/multiple-renderers/renderers/one/server.mjs
new file mode 100644
index 000000000..52db653fa
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/renderers/one/server.mjs
@@ -0,0 +1,11 @@
+
+export default {
+	check() {
+		throw new Error(`Oops this did not work`);
+	},
+	renderToStaticMarkup(Component) {
+		return {
+			html: Component()
+		};
+	},
+};
diff --git a/packages/astro/test/fixtures/multiple-renderers/renderers/two/index.mjs b/packages/astro/test/fixtures/multiple-renderers/renderers/two/index.mjs
new file mode 100644
index 000000000..729356d24
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/renderers/two/index.mjs
@@ -0,0 +1,15 @@
+
+export default function() {
+	return {
+		name: 'renderer-two',
+		hooks: {
+			'astro:config:setup': ({ addRenderer }) => {
+				addRenderer({
+					name: 'renderer-two',
+					clientEntrypoint: null,
+					serverEntrypoint: '@astrojs/renderer-two/server.mjs',
+				});
+			}
+		}
+	};
+}
diff --git a/packages/astro/test/fixtures/multiple-renderers/renderers/two/package.json b/packages/astro/test/fixtures/multiple-renderers/renderers/two/package.json
new file mode 100644
index 000000000..51216ac57
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/renderers/two/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "@astrojs/renderer-two",
+  "version": "1.0.0",
+  "main": "index.mjs"
+}
diff --git a/packages/astro/test/fixtures/multiple-renderers/renderers/two/server.mjs b/packages/astro/test/fixtures/multiple-renderers/renderers/two/server.mjs
new file mode 100644
index 000000000..cba48ac24
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/renderers/two/server.mjs
@@ -0,0 +1,11 @@
+
+export default {
+	check() {
+		return true;
+	},
+	renderToStaticMarkup(Component) {
+		return {
+			html: Component()
+		};
+	},
+};
diff --git a/packages/astro/test/fixtures/multiple-renderers/src/pages/index.astro b/packages/astro/test/fixtures/multiple-renderers/src/pages/index.astro
new file mode 100644
index 000000000..1708d1cc9
--- /dev/null
+++ b/packages/astro/test/fixtures/multiple-renderers/src/pages/index.astro
@@ -0,0 +1,14 @@
+---
+	function Component() {
+		return `<div id="component">works</div>`;
+	}
+---
+<html>
+	<head>
+		<title>Testing</title>
+	</head>
+	<body>
+		<h1>Testing</h1>
+		<Component />
+	</body>
+</html>
diff --git a/packages/astro/test/multiple-renderers.test.js b/packages/astro/test/multiple-renderers.test.js
new file mode 100644
index 000000000..3b542aab8
--- /dev/null
+++ b/packages/astro/test/multiple-renderers.test.js
@@ -0,0 +1,21 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('Multiple renderers', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/multiple-renderers/',
+		});
+		await fixture.build();
+	});
+
+	it('when the first throws but the second does not, use the second renderer', async () => {
+		const html = await fixture.readFile('/index.html');
+		const $ = cheerio.load(html);
+		expect($('#component')).to.have.a.lengthOf(1);
+	});
+});
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index e7d7f5fba..4c08e5882 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -1634,6 +1634,22 @@ importers:
       '@astrojs/preact': link:../../../../integrations/preact
       astro: link:../../..
 
+  packages/astro/test/fixtures/multiple-renderers:
+    specifiers:
+      '@astrojs/renderer-one': file:./renderers/one
+      '@astrojs/renderer-two': file:./renderers/two
+      astro: workspace:*
+    dependencies:
+      '@astrojs/renderer-one': file:packages/astro/test/fixtures/multiple-renderers/renderers/one
+      '@astrojs/renderer-two': file:packages/astro/test/fixtures/multiple-renderers/renderers/two
+      astro: link:../../..
+
+  packages/astro/test/fixtures/multiple-renderers/renderers/one:
+    specifiers: {}
+
+  packages/astro/test/fixtures/multiple-renderers/renderers/two:
+    specifiers: {}
+
   packages/astro/test/fixtures/page-level-styles:
     specifiers:
       astro: workspace:*
@@ -17079,3 +17095,15 @@ packages:
     name: '@astrojs/test-font-awesome-package'
     version: 0.0.1
     dev: false
+
+  file:packages/astro/test/fixtures/multiple-renderers/renderers/one:
+    resolution: {directory: packages/astro/test/fixtures/multiple-renderers/renderers/one, type: directory}
+    name: '@astrojs/renderer-one'
+    version: 1.0.0
+    dev: false
+
+  file:packages/astro/test/fixtures/multiple-renderers/renderers/two:
+    resolution: {directory: packages/astro/test/fixtures/multiple-renderers/renderers/two, type: directory}
+    name: '@astrojs/renderer-two'
+    version: 1.0.0
+    dev: false