Prevent throwing in react and solid component checks (#11624)

Author: bluwy

.changeset/few-starfishes-change.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/solid-js': patch
+---
+
+Prevents throwing errors when checking if a component is a Solid component in runtime

.changeset/spotty-lies-eat.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/react': patch
+---
+
+Prevents throwing errors when checking if a component is a React component in runtime

packages/astro/e2e/errors.test.js

@@ -67,15 +67,18 @@ test.describe('Error display', () => {
 		expect(fileLocation).toMatch(/^pages\/import-not-found\.astro/);
 	});
 
+	// NOTE: It's not possible to detect some JSX components if they have errors because
+	// their renderers' check functions run the render directly, and if a runtime error is
+	// thrown, it assumes that it's simply not that renderer's component and skips it
 	test('shows correct file path when a component has an error', async ({ page, astro }) => {
-		await page.goto(astro.resolveUrl('/preact-runtime-error'), { waitUntil: 'networkidle' });
+		await page.goto(astro.resolveUrl('/vue-runtime-error'), { waitUntil: 'networkidle' });
 
 		const { fileLocation, absoluteFileLocation } = await getErrorOverlayContent(page);
 		const absoluteFileUrl = 'file://' + absoluteFileLocation.replace(/:\d+:\d+$/, '');
 		const fileExists = astro.pathExists(absoluteFileUrl);
 
 		expect(fileExists).toBeTruthy();
-		expect(fileLocation).toMatch(/^preact\/PreactRuntimeError.jsx/);
+		expect(fileLocation).toMatch(/^vue\/VueRuntimeError.vue/);
 	});
 
 	test('shows correct line when a style preprocess has an error', async ({ page, astro }) => {

packages/integrations/preact/src/server.ts

@@ -27,19 +27,17 @@ async function check(
 	useConsoleFilter();
 
 	try {
-		try {
-			const { html } = await renderToStaticMarkup.call(this, Component, props, children, undefined);
-			if (typeof html !== 'string') {
-				return false;
-			}
-
-			// There are edge cases (SolidJS) where Preact *might* render a string,
-			// but components would be <undefined></undefined>
-			// It also might render an empty sting.
-			return html == '' ? false : !html.includes('<undefined>');
-		} catch {
+		const { html } = await renderToStaticMarkup.call(this, Component, props, children, undefined);
+		if (typeof html !== 'string') {
 			return false;
 		}
+
+		// There are edge cases (SolidJS) where Preact *might* render a string,
+		// but components would be <undefined></undefined>
+		// It also might render an empty sting.
+		return html == '' ? false : !html.includes('<undefined>');
+	} catch {
+		return false;
 	} finally {
 		finishUsingConsoleFilter();
 	}

packages/integrations/react/server-v17.js

@@ -5,14 +5,6 @@ import StaticHtml from './static-html.js';
 const slotName = (str) => str.trim().replace(/[-_]([a-z])/g, (_, w) => w.toUpperCase());
 const reactTypeof = Symbol.for('react.element');
 
-function errorIsComingFromPreactComponent(err) {
-	return (
-		err.message &&
-		(err.message.startsWith("Cannot read property '__H'") ||
-			err.message.includes("(reading '__H')"))
-	);
-}
-
 function check(Component, props, children) {
 	// Note: there are packages that do some unholy things to create "components".
 	// Checking the $$typeof property catches most of these patterns.
@@ -26,28 +18,20 @@ function check(Component, props, children) {
 		return React.Component.isPrototypeOf(Component) || React.PureComponent.isPrototypeOf(Component);
 	}
 
-	let error = null;
 	let isReactComponent = false;
 	function Tester(...args) {
 		try {
 			const vnode = Component(...args);
 			if (vnode && vnode['$$typeof'] === reactTypeof) {
 				isReactComponent = true;
 			}
-		} catch (err) {
-			if (!errorIsComingFromPreactComponent(err)) {
-				error = err;
-			}
-		}
+		} catch {}
 
 		return React.createElement('div');
 	}
 
 	renderToStaticMarkup(Tester, props, children, {});
 
-	if (error) {
-		throw error;
-	}
 	return isReactComponent;
 }
 

packages/integrations/react/server.js

@@ -7,14 +7,6 @@ import StaticHtml from './static-html.js';
 const slotName = (str) => str.trim().replace(/[-_]([a-z])/g, (_, w) => w.toUpperCase());
 const reactTypeof = Symbol.for('react.element');
 
-function errorIsComingFromPreactComponent(err) {
-	return (
-		err.message &&
-		(err.message.startsWith("Cannot read property '__H'") ||
-			err.message.includes("(reading '__H')"))
-	);
-}
-
 async function check(Component, props, children) {
 	// Note: there are packages that do some unholy things to create "components".
 	// Checking the $$typeof property catches most of these patterns.
@@ -32,28 +24,20 @@ async function check(Component, props, children) {
 		return React.Component.isPrototypeOf(Component) || React.PureComponent.isPrototypeOf(Component);
 	}
 
-	let error = null;
 	let isReactComponent = false;
 	function Tester(...args) {
 		try {
 			const vnode = Component(...args);
 			if (vnode && vnode['$$typeof'] === reactTypeof) {
 				isReactComponent = true;
 			}
-		} catch (err) {
-			if (!errorIsComingFromPreactComponent(err)) {
-				error = err;
-			}
-		}
+		} catch {}
 
 		return React.createElement('div');
 	}
 
 	await renderToStaticMarkup(Tester, props, children, {});
 
-	if (error) {
-		throw error;
-	}
 	return isReactComponent;
 }
 

packages/integrations/solid/src/server.ts

@@ -28,12 +28,16 @@ async function check(
 	// In general, components from other frameworks (eg, MDX, React, etc.) tend to render as "undefined",
 	// so we take advantage of this trick to decide if this is a Solid component or not.
 
-	const { html } = await renderToStaticMarkup.call(this, Component, props, children, {
-		// The purpose of check() is just to validate that this is a Solid component and not
-		// React, etc. We should render in sync mode which should skip Suspense boundaries
-		// or loading resources like external API calls.
-		renderStrategy: 'sync' as RenderStrategy,
-	});
+	let html: string | undefined;
+	try {
+		const result = await renderToStaticMarkup.call(this, Component, props, children, {
+			// The purpose of check() is just to validate that this is a Solid component and not
+			// React, etc. We should render in sync mode which should skip Suspense boundaries
+			// or loading resources like external API calls.
+			renderStrategy: 'sync' as RenderStrategy,
+		});
+		html = result.html;
+	} catch {}
 
 	return typeof html === 'string';
 }