From 7adb350a37f3975c8c9db89a32bf63b9fd0b78c2 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Fri, 9 Aug 2024 20:01:25 +0800 Subject: [PATCH] Prevent throwing in react and solid component checks (#11624) --- .changeset/few-starfishes-change.md | 5 +++++ .changeset/spotty-lies-eat.md | 5 +++++ packages/astro/e2e/errors.test.js | 7 +++++-- packages/integrations/preact/src/server.ts | 20 +++++++++----------- packages/integrations/react/server-v17.js | 18 +----------------- packages/integrations/react/server.js | 18 +----------------- packages/integrations/solid/src/server.ts | 16 ++++++++++------ 7 files changed, 36 insertions(+), 53 deletions(-) create mode 100644 .changeset/few-starfishes-change.md create mode 100644 .changeset/spotty-lies-eat.md diff --git a/.changeset/few-starfishes-change.md b/.changeset/few-starfishes-change.md new file mode 100644 index 000000000000..5a221fef961d --- /dev/null +++ b/.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 diff --git a/.changeset/spotty-lies-eat.md b/.changeset/spotty-lies-eat.md new file mode 100644 index 000000000000..43ccae2e95fe --- /dev/null +++ b/.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 diff --git a/packages/astro/e2e/errors.test.js b/packages/astro/e2e/errors.test.js index c015ea123342..34cecd816f56 100644 --- a/packages/astro/e2e/errors.test.js +++ b/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 }) => { diff --git a/packages/integrations/preact/src/server.ts b/packages/integrations/preact/src/server.ts index 932cacb99d06..5b38ff590681 100644 --- a/packages/integrations/preact/src/server.ts +++ b/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 - // It also might render an empty sting. - return html == '' ? false : !html.includes(''); - } 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 + // It also might render an empty sting. + return html == '' ? false : !html.includes(''); + } catch { + return false; } finally { finishUsingConsoleFilter(); } diff --git a/packages/integrations/react/server-v17.js b/packages/integrations/react/server-v17.js index 16f6fcdcd9da..4e577883aaf6 100644 --- a/packages/integrations/react/server-v17.js +++ b/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,7 +18,6 @@ function check(Component, props, children) { return React.Component.isPrototypeOf(Component) || React.PureComponent.isPrototypeOf(Component); } - let error = null; let isReactComponent = false; function Tester(...args) { try { @@ -34,20 +25,13 @@ function check(Component, props, children) { 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; } diff --git a/packages/integrations/react/server.js b/packages/integrations/react/server.js index 3907ba6f177e..69b0a8e12cce 100644 --- a/packages/integrations/react/server.js +++ b/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,7 +24,6 @@ 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 { @@ -40,20 +31,13 @@ async function check(Component, props, children) { 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; } diff --git a/packages/integrations/solid/src/server.ts b/packages/integrations/solid/src/server.ts index a352dfa5a64d..295771265659 100644 --- a/packages/integrations/solid/src/server.ts +++ b/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'; }