From 0f9051e1dcf91075ba3957ce21620cb8b4371de3 Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 5 Aug 2024 17:53:09 +0800 Subject: [PATCH 1/4] Prevent throwing in react and solid component checks --- .changeset/few-starfishes-change.md | 5 +++++ .changeset/spotty-lies-eat.md | 5 +++++ 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 ++++++++++------ 6 files changed, 31 insertions(+), 51 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/integrations/preact/src/server.ts b/packages/integrations/preact/src/server.ts index 88e012d02047..7e13d295f9f9 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 : !//.test(html); - } catch (err) { + 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 : !//.test(html); + } catch (err) { + 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 16bf6785e28a..961315591df3 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 a58d7e17dc9a..42bbb8b23f26 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'; } From 0809321087c777068c06c2062d5f8d2cd23680f8 Mon Sep 17 00:00:00 2001 From: bluwy Date: Mon, 5 Aug 2024 18:22:58 +0800 Subject: [PATCH 2/4] Fix test --- packages/astro/e2e/errors.test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/astro/e2e/errors.test.js b/packages/astro/e2e/errors.test.js index 2f1358548974..ff29d380294f 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 }) => { From ad64f7df76f5726f208ea7ddb83f127ff9381e7a Mon Sep 17 00:00:00 2001 From: bluwy Date: Thu, 8 Aug 2024 21:38:56 +0800 Subject: [PATCH 3/4] Format --- packages/integrations/preact/src/server.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/integrations/preact/src/server.ts b/packages/integrations/preact/src/server.ts index 5f720b9f7046..7de8589897ac 100644 --- a/packages/integrations/preact/src/server.ts +++ b/packages/integrations/preact/src/server.ts @@ -15,7 +15,7 @@ async function check( this: RendererContext, Component: any, props: Record, - children: any + children: any, ) { if (typeof Component !== 'function') return false; if (Component.name === 'QwikComponent') return false; @@ -53,7 +53,7 @@ async function renderToStaticMarkup( Component: any, props: Record, { default: children, ...slotted }: Record, - metadata: AstroComponentMetadata | undefined + metadata: AstroComponentMetadata | undefined, ) { const ctx = getContext(this.result); @@ -83,7 +83,7 @@ async function renderToStaticMarkup( hydrate: shouldHydrate(metadata), value: children, }) - : children + : children, ); const html = await renderToStringAsync(vNode); From f132ffd4d644c7969b05f760774aecabc4f716ca Mon Sep 17 00:00:00 2001 From: bluwy Date: Thu, 8 Aug 2024 21:49:45 +0800 Subject: [PATCH 4/4] Fix lint --- packages/integrations/preact/src/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integrations/preact/src/server.ts b/packages/integrations/preact/src/server.ts index 7de8589897ac..5b38ff590681 100644 --- a/packages/integrations/preact/src/server.ts +++ b/packages/integrations/preact/src/server.ts @@ -36,7 +36,7 @@ async function check( // but components would be // It also might render an empty sting. return html == '' ? false : !html.includes(''); - } catch (err) { + } catch { return false; } finally { finishUsingConsoleFilter();