Improved JSX framework detection (#382)

* Improved JSX framework detection

This improves the JSX detection and adds a bunch of test. The following is improved:

* If an error throws because of a coding mistake, those errors will be reported.
* We properly detect class components for both Preact and React. We don't have to "try to render" these.

It's still possible that error messages might be obscured in this scenario:

A Preact function component that uses hooks (or another preact specific feature) that has a coding mistake. The React renderer might throw when it uses the Preact hook. That error will be reported rather than the real coding mistake.

This is because we can't distinguish between errors that are due to the wrong framework and errors that the user caused.

I might reach out to the Preact community and see if they can think of a better solution to this problem. This will come up when other JSX based frameworks have renderers. I still think that having multiple frameworks in the same project is a feature worth trying to preserve.

* Move try/catch into the __astro_component

14 files changed, 169 insertions, 16 deletions

diff --git a/packages/astro/src/internal/__astro_component.ts b/packages/astro/src/internal/__astro_component.ts
index 191b05cc7..10844db93 100644
--- a/packages/astro/src/internal/__astro_component.ts
+++ b/packages/astro/src/internal/__astro_component.ts
@@ -32,16 +32,27 @@ async function resolveRenderer(Component: any, props: any = {}, children?: strin
     return rendererCache.get(Component);
   }
 
+  const errors: Error[] = [];
   for (const __renderer of __renderers) {
     // Yes, we do want to `await` inside of this loop!
     // __renderer.check can't be run in parallel, it
     // returns the first match and skips any subsequent checks
-    const shouldUse = await __renderer.check(Component, props, children);
-    if (shouldUse) {
-      rendererCache.set(Component, __renderer);
-      return __renderer;
+    try {
+      const shouldUse: boolean = await __renderer.check(Component, props, children);
+
+      if(shouldUse) {
+        rendererCache.set(Component, __renderer);
+        return __renderer;
+      }
+    } catch(err) {
+      errors.push(err);
     }
   }
+
+  if(errors.length) {
+    // For now just throw the first error we encounter.
+    throw errors[0];
+  }
 }
 
 interface AstroComponentProps {
diff --git a/packages/astro/test/fixtures/preact-component/snowpack.config.json b/packages/astro/test/fixtures/preact-component/snowpack.config.json
new file mode 100644
index 000000000..8f034781d
--- /dev/null
+++ b/packages/astro/test/fixtures/preact-component/snowpack.config.json
@@ -0,0 +1,3 @@
+{
+  "workspaceRoot": "../../../../../"
+}
diff --git a/packages/astro/test/fixtures/preact-component/src/components/Class.jsx b/packages/astro/test/fixtures/preact-component/src/components/Class.jsx
new file mode 100644
index 000000000..7cf07c736
--- /dev/null
+++ b/packages/astro/test/fixtures/preact-component/src/components/Class.jsx
@@ -0,0 +1,7 @@
+import { h, Component } from 'preact';
+
+export default class extends Component {
+  render() {
+    return <div id="class-component"></div>
+  }
+}
\ No newline at end of file
diff --git a/packages/astro/test/fixtures/preact-component/src/components/Function.jsx b/packages/astro/test/fixtures/preact-component/src/components/Function.jsx
new file mode 100644
index 000000000..002ce9b51
--- /dev/null
+++ b/packages/astro/test/fixtures/preact-component/src/components/Function.jsx
@@ -0,0 +1,5 @@
+import { h, Component } from 'preact';
+
+export default function() {
+  return <div id="fn-component"></div>;
+}
\ No newline at end of file
diff --git a/packages/astro/test/fixtures/preact-component/src/components/Hooks.jsx b/packages/astro/test/fixtures/preact-component/src/components/Hooks.jsx
new file mode 100644
index 000000000..53b7473b1
--- /dev/null
+++ b/packages/astro/test/fixtures/preact-component/src/components/Hooks.jsx
@@ -0,0 +1,7 @@
+import { h } from 'preact';
+import { useState } from 'preact/hooks';
+
+export default function() {
+  const [val] = useState('world');
+  return <div id={val}></div>;
+}
\ No newline at end of file
diff --git a/packages/astro/test/fixtures/preact-component/src/pages/class.astro b/packages/astro/test/fixtures/preact-component/src/pages/class.astro
new file mode 100644
index 000000000..fae5e846f
--- /dev/null
+++ b/packages/astro/test/fixtures/preact-component/src/pages/class.astro
@@ -0,0 +1,12 @@
+---
+import ClassComponent from '../components/Class.jsx';
+---
+
+<html>
+<head>
+  <title>Preact class component</title>
+</head>
+<body>
+  <ClassComponent />
+</body>
+</html>
\ No newline at end of file
diff --git a/packages/astro/test/fixtures/preact-component/src/pages/fn.astro b/packages/astro/test/fixtures/preact-component/src/pages/fn.astro
new file mode 100644
index 000000000..07a2c7e3e
--- /dev/null
+++ b/packages/astro/test/fixtures/preact-component/src/pages/fn.astro
@@ -0,0 +1,12 @@
+---
+import FunctionComponent from '../components/Function.jsx';
+---
+
+<html>
+<head>
+  <title>Preact function component</title>
+</head>
+<body>
+  <FunctionComponent />
+</body>
+</html>
\ No newline at end of file
diff --git a/packages/astro/test/fixtures/preact-component/src/pages/hooks.astro b/packages/astro/test/fixtures/preact-component/src/pages/hooks.astro
new file mode 100644
index 000000000..7c9d94b93
--- /dev/null
+++ b/packages/astro/test/fixtures/preact-component/src/pages/hooks.astro
@@ -0,0 +1,11 @@
+---
+import Hooks from '../components/Hooks.jsx';
+---
+<html>
+<head>
+  <title>Preact hooks</title>
+</head>
+<body>
+  <Hooks />
+</body>
+</html>
\ No newline at end of file
diff --git a/packages/astro/test/fixtures/react-component/src/components/ForgotImport.jsx b/packages/astro/test/fixtures/react-component/src/components/ForgotImport.jsx
new file mode 100644
index 000000000..d7dfc29f7
--- /dev/null
+++ b/packages/astro/test/fixtures/react-component/src/components/ForgotImport.jsx
@@ -0,0 +1,5 @@
+
+
+export default function ({}) {
+  return <h2>oops</h2>;
+}
\ No newline at end of file
diff --git a/packages/astro/test/fixtures/react-component/src/pages/forgot-import.astro b/packages/astro/test/fixtures/react-component/src/pages/forgot-import.astro
new file mode 100644
index 000000000..de5d319d9
--- /dev/null
+++ b/packages/astro/test/fixtures/react-component/src/pages/forgot-import.astro
@@ -0,0 +1,12 @@
+---
+import ForgotImport from '../components/ForgotImport.jsx';
+---
+
+<html>
+<head>
+  <title>Here we are</title>
+</head>
+<body>
+  <ForgotImport />
+</body>
+</html>
\ No newline at end of file
diff --git a/packages/astro/test/preact-component.test.js b/packages/astro/test/preact-component.test.js
new file mode 100644
index 000000000..2ff4bbfdc
--- /dev/null
+++ b/packages/astro/test/preact-component.test.js
@@ -0,0 +1,34 @@
+import { suite } from 'uvu';
+import * as assert from 'uvu/assert';
+import { doc } from './test-utils.js';
+import { setup } from './helpers.js';
+
+const PreactComponent = suite('Preact component test');
+
+setup(PreactComponent, './fixtures/preact-component');
+
+PreactComponent('Can load class component', async ({ runtime }) => {
+  const result = await runtime.load('/class');
+  if (result.error) throw new Error(result.error);
+
+  const $ = doc(result.contents);
+  assert.equal($('#class-component').length, 1, 'Can use class components');
+});
+
+PreactComponent('Can load function component', async ({ runtime }) => {
+  const result = await runtime.load('/fn');
+  if (result.error) throw new Error(result.error);
+
+  const $ = doc(result.contents);
+  assert.equal($('#fn-component').length, 1, 'Can use function components');
+});
+
+PreactComponent('Can use hooks', async ({ runtime }) => {
+  const result = await runtime.load('/hooks');
+  if (result.error) throw new Error(result.error);
+  
+  const $ = doc(result.contents);
+  assert.equal($('#world').length, 1);
+});
+
+PreactComponent.run();
diff --git a/packages/astro/test/react-component.test.js b/packages/astro/test/react-component.test.js
index 10b44120c..a638bd1c4 100644
--- a/packages/astro/test/react-component.test.js
+++ b/packages/astro/test/react-component.test.js
@@ -49,4 +49,11 @@ React('Can load Vue', async () => {
   assert.equal($('#vue-h2').text(), 'Hasta la vista, baby');
 });
 
+React('Get good error message when react import is forgotten', async () => {
+  const result = await runtime.load('/forgot-import');
+
+  assert.ok(result.error instanceof ReferenceError);
+  assert.equal(result.error.message, 'React is not defined');
+});
+
 React.run();
diff --git a/packages/renderers/renderer-preact/server.js b/packages/renderers/renderer-preact/server.js
index 8fd4ccf2f..989e2d385 100644
--- a/packages/renderers/renderer-preact/server.js
+++ b/packages/renderers/renderer-preact/server.js
@@ -1,13 +1,16 @@
-import { h } from 'preact';
+import { h, Component as BaseComponent } from 'preact';
 import { renderToString } from 'preact-render-to-string';
 import StaticHtml from './static-html.js';
 
 function check(Component, props, children) {
-  try {
-    const { html } = renderToStaticMarkup(Component, props, children);
-    return Boolean(html);
-  } catch (e) {}
-  return false;
+  if(typeof Component !== 'function') return false;
+
+  if(typeof Component.prototype.render === 'function') {
+    return BaseComponent.isPrototypeOf(Component);
+  }
+
+  const { html } = renderToStaticMarkup(Component, props, children);
+  return Boolean(html);
 }
 
 function renderToStaticMarkup(Component, props, children) {
diff --git a/packages/renderers/renderer-react/server.js b/packages/renderers/renderer-react/server.js
index 56f2f5aa8..8ac177c7c 100644
--- a/packages/renderers/renderer-react/server.js
+++ b/packages/renderers/renderer-react/server.js
@@ -1,13 +1,37 @@
-import { createElement as h } from 'react';
+import { Component as BaseComponent, createElement as h } from 'react';
 import { renderToStaticMarkup as renderToString } from 'react-dom/server.js';
 import StaticHtml from './static-html.js';
 
+const reactTypeof = Symbol.for('react.element');
+
 function check(Component, props, children) {
-  try {
-    const { html } = renderToStaticMarkup(Component, props, children);
-    return Boolean(html);
-  } catch (e) {}
-  return false;
+  if(typeof Component !== 'function') return false;
+
+  if(typeof Component.prototype.render === 'function') {
+    return BaseComponent.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) {
+      error = err;
+    }
+
+    return h('div');
+  }
+
+  renderToStaticMarkup(Tester, props, children);
+
+  if(error) {
+    throw error;
+  }
+  return isReactComponent;
 }
 
 function renderToStaticMarkup(Component, props, children) {