Bugfix: JSX renderers can be declared in any order (#1686)

packages/astro/src/core/build/index.ts

@@ -53,7 +53,7 @@ class AstroBuilder {
 
   async build() {
     const { logging, origin } = this;
-    const timer: Record<string, number> = {viteStart: performance.now()};
+    const timer: Record<string, number> = { viteStart: performance.now() };
     const viteConfig = await createVite(
       {
         mode: this.mode,
@@ -97,8 +97,8 @@ class AstroBuilder {
     );
 
     // After all routes have been collected, start building them.
-    // TODO: test parallel vs. serial performance. Promise.all() may be 
-    // making debugging harder without any perf gain. If parallel is best, 
+    // TODO: test parallel vs. serial performance. Promise.all() may be
+    // making debugging harder without any perf gain. If parallel is best,
     // then we should set a max number of parallel builds.
     const input: InputHTMLOptions[] = [];
     await Promise.all(
@@ -126,7 +126,7 @@ class AstroBuilder {
     debug(logging, 'build', timerMessage('All pages rendered', timer.renderStart));
 
     // Bundle the assets in your final build: This currently takes the HTML output
-    // of every page (stored in memory) and bundles the assets pointed to on those pages. 
+    // of every page (stored in memory) and bundles the assets pointed to on those pages.
     timer.buildStart = performance.now();
     await vite.build({
       logLevel: 'error',

packages/astro/src/vite-plugin-jsx/index.ts

@@ -11,7 +11,7 @@ import path from 'path';
 import { error } from '../core/logger.js';
 import { parseNpmName } from '../core/util.js';
 
-const JSX_RENDERERS = new Map<string, Renderer>();
+const JSX_RENDERER_CACHE = new WeakMap<AstroConfig, Map<string, Renderer>>();
 const JSX_EXTENSIONS = new Set(['.jsx', '.tsx']);
 const IMPORT_STATEMENTS: Record<string, string> = {
   react: "import React from 'react'",
@@ -22,11 +22,6 @@ const IMPORT_STATEMENTS: Record<string, string> = {
 // be careful about esbuild not treating h, React, Fragment, etc. as unused.
 const PREVENT_UNUSED_IMPORTS = ';;(React,Fragment,h);';
 
-interface AstroPluginJSXOptions {
-  config: AstroConfig;
-  logging: LogOptions;
-}
-
 // https://github.com/vitejs/vite/discussions/5109#discussioncomment-1450726
 function isSSR(options: undefined | boolean | { ssr: boolean }): boolean {
   if (options === undefined) {
@@ -41,6 +36,11 @@ function isSSR(options: undefined | boolean | { ssr: boolean }): boolean {
   return false;
 }
 
+interface AstroPluginJSXOptions {
+  config: AstroConfig;
+  logging: LogOptions;
+}
+
 /** Use Astro config to allow for alternate or multiple JSX renderers (by default Vite will assume React) */
 export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin {
   let viteConfig: ResolvedConfig;
@@ -58,11 +58,13 @@ export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin
       }
 
       const { mode } = viteConfig;
+      let jsxRenderers = JSX_RENDERER_CACHE.get(config);
 
       // load renderers (on first run only)
-      if (JSX_RENDERERS.size === 0) {
-        const jsxRenderers = await loadJSXRenderers(config.renderers);
-        if (jsxRenderers.size === 0) {
+      if (!jsxRenderers) {
+        jsxRenderers = new Map();
+        const possibleRenderers = await loadJSXRenderers(config.renderers);
+        if (possibleRenderers.size === 0) {
           // note: we have filtered out all non-JSX files, so this error should only show if a JSX file is loaded with no matching renderers
           throw new Error(
             `${colors.yellow(
@@ -70,20 +72,21 @@ export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin
             )}\nUnable to resolve a renderer that handles JSX transforms! Please include a \`renderer\` plugin which supports JSX in your \`astro.config.mjs\` file.`
           );
         }
-        for (const [importSource, renderer] of jsxRenderers) {
-          JSX_RENDERERS.set(importSource, renderer);
+        for (const [importSource, renderer] of possibleRenderers) {
+          jsxRenderers.set(importSource, renderer);
         }
+        JSX_RENDERER_CACHE.set(config, jsxRenderers);
       }
 
       // Attempt: Single JSX renderer
       // If we only have one renderer, we can skip a bunch of work!
-      if (JSX_RENDERERS.size === 1) {
+      if (jsxRenderers.size === 1) {
         // downlevel any non-standard syntax, but preserve JSX
         const { code: jsxCode } = await esbuild.transform(code, {
           loader: getLoader(path.extname(id)),
           jsx: 'preserve',
         });
-        return transformJSX({ code: jsxCode, id, renderer: [...JSX_RENDERERS.values()][0], mode, ssr });
+        return transformJSX({ code: jsxCode, id, renderer: [...jsxRenderers.values()][0], mode, ssr });
       }
 
       // Attempt: Multiple JSX renderers
@@ -105,7 +108,7 @@ export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin
         for (let { n: spec } of imports) {
           const pkg = spec && parseNpmName(spec);
           if (!pkg) continue;
-          if (JSX_RENDERERS.has(pkg.name)) {
+          if (jsxRenderers.has(pkg.name)) {
             importSource = pkg.name;
             break;
           }
@@ -126,7 +129,7 @@ export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin
 
       // if JSX renderer found, then use that
       if (importSource) {
-        const jsxRenderer = JSX_RENDERERS.get(importSource);
+        const jsxRenderer = jsxRenderers.get(importSource);
         // if renderer not installed for this JSX source, throw error
         if (!jsxRenderer) {
           error(logging, 'renderer', `${colors.yellow(id)} No renderer installed for ${importSource}. Try adding \`@astrojs/renderer-${importSource}\` to your dependencies.`);
@@ -137,11 +140,11 @@ export default function jsx({ config, logging }: AstroPluginJSXOptions): Plugin
           loader: getLoader(path.extname(id)),
           jsx: 'preserve',
         });
-        return transformJSX({ code: jsxCode, id, renderer: JSX_RENDERERS.get(importSource) as Renderer, mode, ssr });
+        return transformJSX({ code: jsxCode, id, renderer: jsxRenderers.get(importSource) as Renderer, mode, ssr });
       }
 
       // if we still can’t tell, throw error
-      const defaultRenderer = [...JSX_RENDERERS.keys()][0];
+      const defaultRenderer = [...jsxRenderers.keys()][0];
       error(
         logging,
         'renderer',

packages/astro/test/astro-jsx.test.js

@@ -0,0 +1,41 @@
+import { expect } from 'chai';
+import { loadFixture } from './test-utils.js';
+
+let cwd = './fixtures/astro-jsx/';
+let orders = [
+  ['preact', 'react', 'solid'],
+  ['preact', 'solid', 'react'],
+  ['react', 'preact', 'solid'],
+  ['react', 'solid', 'preact'],
+  ['solid', 'react', 'preact'],
+  ['solid', 'preact', 'react'],
+];
+let fixtures = {};
+
+before(async () => {
+  await Promise.all(
+    orders.map((renderers, n) =>
+      loadFixture({
+        projectRoot: cwd,
+        renderers: renderers.map((name) => `@astrojs/renderer-${name}`),
+        dist: new URL(`${cwd}dist-${n}/`, import.meta.url),
+      }).then((fixture) => {
+        fixtures[renderers.toString()] = fixture;
+        return fixture.build();
+      })
+    )
+  );
+});
+
+it('Renderer order', () => {
+  it('JSX renderers can be defined in any order', async () => {
+    if (!Object.values(fixtures).length) {
+      throw new Error(`JSX renderers didn’t build properly`);
+    }
+
+    for (const [name, fixture] of Object.entries(fixtures)) {
+      const html = await fixture.readFile('/index.html');
+      expect(html, name).to.be.ok;
+    }
+  });
+});

packages/astro/test/astro-styles-ssr.test.js

@@ -105,7 +105,7 @@ describe('Styles SSR', () => {
       });
 
     // test 1: Astro component has some scoped class
-    expect(scopedClass).to.be.ok();
+    expect(scopedClass).to.be.ok;
 
     // test 2–3: children get scoped class
     expect(el1.attr('class')).to.equal(`blue ${scopedClass}`);

packages/astro/test/fixtures/astro-jsx/.gitignore

@@ -0,0 +1 @@
+/dist-*

packages/astro/test/fixtures/astro-jsx/src/components/PreactCounter.tsx

@@ -0,0 +1,20 @@
+import { h, Fragment } from 'preact';
+import { useState } from 'preact/hooks';
+
+/** a counter written in Preact */
+export default function PreactCounter() {
+  const [count, setCount] = useState(0);
+  const add = () => setCount((i) => i + 1);
+  const subtract = () => setCount((i) => i - 1);
+
+  return (
+    <div id="preact">
+      <div className="counter">
+        <button onClick={subtract}>-</button>
+        <pre>{count}</pre>
+        <button onClick={add}>+</button>
+      </div>
+      <div className="children">Preact</div>
+    </div>
+  );
+}

packages/astro/test/fixtures/astro-jsx/src/components/ReactCounter.jsx

@@ -0,0 +1,19 @@
+import React, { useState } from 'react';
+
+/** a counter written in React */
+export default function ReactCounter() {
+  const [count, setCount] = useState(0);
+  const add = () => setCount((i) => i + 1);
+  const subtract = () => setCount((i) => i - 1);
+
+  return (
+    <div id="react">
+      <div className="counter">
+        <button onClick={subtract}>-</button>
+        <pre>{count}</pre>
+        <button onClick={add}>+</button>
+      </div>
+      <div className="children">React</div>
+    </div>
+  );
+}

packages/astro/test/fixtures/astro-jsx/src/components/SolidCounter.jsx

@@ -0,0 +1,19 @@
+import { createSignal } from 'solid-js';
+
+/** a counter written with Solid */
+export default function SolidCounter() {
+  const [count, setCount] = createSignal(0);
+  const add = () => setCount(count() + 1);
+  const subtract = () => setCount(count() - 1);
+
+  return (
+    <div id="solid">
+      <div class="counter">
+        <button onClick={subtract}>-</button>
+        <pre>{count()}</pre>
+        <button onClick={add}>+</button>
+      </div>
+      <div class="children">Solid</div>
+    </div>
+  );
+}

packages/astro/test/fixtures/astro-jsx/src/pages/index.astro

@@ -0,0 +1,9 @@
+---
+import ReactCounter from '../components/ReactCounter.jsx';
+import PreactCounter from '../components/PreactCounter.tsx';
+import SolidCounter from '../components/SolidCounter.jsx';
+---
+
+<ReactCounter />
+<PreactCounter />
+<SolidCounter />

packages/astro/test/lit-element.test.js

@@ -45,13 +45,13 @@ describe('LitElement test', () => {
     // test 2: shadow rendered
     expect($('my-element').html()).to.include(`<div>Testing...</div>`);
   });
-
-  after(async () => {
-    // The Lit renderer adds browser globals that interfere with other tests, so remove them now.
-    const globals = Object.keys(globalThis.window || {});
-    globals.splice(globals.indexOf('global'), 1);
-    for (let name of globals) {
-      delete globalThis[name];
-    }
-  });
+});
+
+after(async () => {
+  // The Lit renderer adds browser globals that interfere with other tests, so remove them now.
+  const globals = Object.keys(globalThis.window || {});
+  globals.splice(globals.indexOf('global'), 1);
+  for (let name of globals) {
+    delete globalThis[name];
+  }
 });

packages/astro/test/solid-component.test.js

@@ -2,17 +2,17 @@ import { expect } from 'chai';
 import cheerio from 'cheerio';
 import { loadFixture } from './test-utils.js';
 
-describe.skip('Solid component', () => {
-  let fixture;
+let fixture;
 
-  before(async () => {
-    fixture = await loadFixture({
-      projectRoot: './fixtures/solid-component/',
-      renderers: ['@astrojs/renderer-solid'],
-    });
-    await fixture.build();
+before(async () => {
+  fixture = await loadFixture({
+    projectRoot: './fixtures/solid-component/',
+    renderers: ['@astrojs/renderer-solid'],
   });
+  await fixture.build();
+});
 
+describe('Solid component', () => {
   it('Can load a component', async () => {
     const html = await fixture.readFile('/index.html');
     const $ = cheerio.load(html);

packages/renderers/renderer-preact/server.js

@@ -11,7 +11,6 @@ function check(Component, props, children) {
 
   try {
     const { html } = renderToStaticMarkup(Component, props, children);
-
     if (typeof html !== 'string') {
       return false;
     }
@@ -20,7 +19,7 @@ function check(Component, props, children) {
     // but components would be <undefined></undefined>
 
     return !/\<undefined\>/.test(html);
-  } catch {
+  } catch (err) {
     return false;
   }
 }

packages/renderers/renderer-react/server.js

@@ -5,7 +5,7 @@ import StaticHtml from './static-html.js';
 const reactTypeof = Symbol.for('react.element');
 
 function errorIsComingFromPreactComponent(err) {
-  return err.message && err.message.startsWith("Cannot read property '__H'");
+  return err.message && (err.message.startsWith("Cannot read property '__H'") || err.message.includes("(reading '__H')"));
 }
 
 function check(Component, props, children) {
@@ -24,7 +24,7 @@ function check(Component, props, children) {
         isReactComponent = true;
       }
     } catch (err) {
-      if(!errorIsComingFromPreactComponent(err)) {
+      if (!errorIsComingFromPreactComponent(err)) {
         error = err;
       }
     }

packages/renderers/renderer-solid/server.js

@@ -2,9 +2,12 @@ import { renderToString, ssr, createComponent } from 'solid-js/web/dist/server.j
 
 function check(Component, props, children) {
   if (typeof Component !== 'function') return false;
-
-  const { html } = renderToStaticMarkup(Component, props, children);
-  return typeof html === 'string';
+  try {
+    const { html } = renderToStaticMarkup(Component, props, children);
+    return typeof html === 'string';
+  } catch (err) {
+    return false;
+  }
 }
 
 function renderToStaticMarkup(Component, props, children) {