Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[🐞] Confusing NoMatchingRenderer failure with some components crossing Astro/Qwik component boundary #228

Closed
seleb opened this issue Jan 22, 2025 · 10 comments · Fixed by #229

Comments

@seleb
Copy link

seleb commented Jan 22, 2025

Which component is affected?

Integration

Describe the bug

Hey there, I'm working a project that uses Astro + Qwik via this integration, and we're running into some cases where the SSR fails. The error output appears misleading, but I'm not familiar enough with the integration to guess at a specific cause.

Sample error output:

[ERROR] [NoMatchingRenderer] Unable to render `Counter`.

There is 1 renderer configured in your `astro.config.mjs` file,
but it was not able to server-side render `Counter`.
  Hint:
    Did you mean to enable the `@astrojs/react`, `@astrojs/preact`, `@astrojs/solid-js`, `@astrojs/vue` or `@astrojs/svelte` integration?

    See https://docs.astro.build/en/guides/framework-components/ for more information on how to install and configure integrations.
  Error reference:
    https://docs.astro.build/en/reference/errors/no-matching-renderer/
  Stack trace:
    at renderFrameworkComponent (<workspace>\qwik-astro-ssr-failure-repro\node_modules\astro\dist\runtime\server\render\component.js:175:15)
    [...] See full stack trace in the browser, or rerun with --verbose.

The provided reproduction link demonstrates the error based on the starter: counter.tsx has been modified to be an inline component that renders text inside a fragment, and is itself rendered inside the .astro route. Any of the following changes resolves the issue:

  • Adding the component$ wrapper to counter.tsx
  • Replacing the fragment in counter.tsx with a <div> tag
  • Rendering <Counter /> inside another Qwik component that itself has an existing wrapper <div> (e.g. Wrapper.tsx). Note that this does not avoid the issue if it is rendered via children/slots (e.g. WrapperViaChildren.tsx)

Although the above changes may be workarounds in some cases, they may not always be viable:

  • The component$ wrapper requires children to be converted to slots, which may affect behaviour (e.g. slots have no equivalent for children ? <HasChildren /> : <HasNoChildren />). In our real use case, we've also seen significant decreases in server performance/TTFB when many inline components are converted to use component$, and cannot adopt this workaround
  • Adding wrapper elements may be incompatible with the DOM layout/semantics
  • There may not be any relevant ancestor components available to use as wrappers. Rendering the entire route body in a wrapper route-specific Qwik "page" component is more reliable, but this adds additional boilerplate to every route and is subject to the current limitation where .astro components must be responsible for rendering html/head elements

Reproduction

https://github.com/seleb/qwik-astro-ssr-failure-repro

Steps to reproduce

  1. npm i
  2. npm start
  3. error occurs

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (16) x64 11th Gen Intel(R) Core(TM) i9-11980HK @ 2.60GHz
    Memory: 45.09 GB / 63.71 GB
  Binaries:
    Node: 20.12.2 - C:\Program Files\nodejs\node.EXE
    npm: 10.9.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (127.0.2651.74)
    Internet Explorer: 11.0.19041.4355

Additional Information

No response

@siguici
Copy link
Collaborator

siguici commented Jan 22, 2025

You are missing to wrap your Qwik components with component$().

@seleb
Copy link
Author

seleb commented Jan 22, 2025

You are missing to wrap your Qwik components with component$().

As mentioned in the workarounds section, we're intentionally using inline components in some cases and cannot adopt component$ wrappers everywhere. afaik there's no requirement to use component$ unless you're using other $ features like signals, slots, or want to take advantage of separate chunks - is that incorrect?

@siguici
Copy link
Collaborator

siguici commented Jan 23, 2025

Oh yeah. Sorry, I missed that part.
I think @thejackshelton would be the best person to answer.

@Shane-Donlon
Copy link

Shane-Donlon commented Jan 23, 2025

Hey everyone, 👋

just additional troubleshooting to try and help with this!

Observation on this issue is that there no HTML element being rendered that is potentially the cause of this,

Examples:

this renders fine (fragment returning non-html element and html element)

export const Counter = () => {
  return <>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Explicabo perspiciatis quidem hic porro totam laudantium, necessitatibus harum debitis repellendus, eaque veritatis velit quo voluptatem ipsa excepturi commodi maiores vero incidunt!
    <p>simple inline component</p></>;
};

this renders fine. (fragment returning html element)

export const Counter = () => {
  return <><p>simple inline component</p></>;
}

this fails (fragment returning non-html element)

export const Counter = () => {
  return <>simple inline component</>;
};

@seleb
Copy link
Author

seleb commented Jan 23, 2025

In the provided example there's no HTML element being rendered, though this isn't true for every case where we've seen the error. Here's another case which renders a div with props spreading that results in the same error:

import type { PropsOf } from '@builder.io/qwik';

export const Counter = (props: PropsOf<'div'>) => {
  return <div {...props}>simple inline component</div>;
};

@thejackshelton
Copy link
Member

Hey guys! I've gone ahead and opened a PR that should fix the issue in all cases. I believe this is because we left out a jsx transform function to detect if an inline component is a renderer in Astro.

In an application where everything is just Qwik (or even Qwik React where it becomes a Qwik component), you don't need to do checks like this. But because Astro can include many different components on the page, each from different renderers, we need a way to be able to identify which framework is processing these files.

In this case, I've added _jsxc (a jsx transform function used), to the list of identifiers. Ideally, we can find a way in the future that doesn't need to check internal transform functions.

npm i https://pkg.pr.new/QwikDev/astro/@qwikdev/astro@229

Try this version and let me know if you're still running into any issues.

@seleb
Copy link
Author

seleb commented Jan 24, 2025

Hmm, tried out that version in both the provided minimal repro case and in our actual project: it does fix the minimal repro (🎉) but unfortunately I'm still seeing the issues in our actual project. I'm not sure yet what the significant difference is but I'll spend some time and try to produce another repro case that covers it.

@seleb
Copy link
Author

seleb commented Jan 24, 2025

OK I've updated the repro project with the following changes:

  • Updated the dependencies to use the fix from above
  • Renamed counter to InlineComponentSimple for clarity
  • Added InlineComponentPropsSpread, the other problematic case mentioned in my second reply above

In this update, the error is reproduced specifically for InlineComponentPropsSpread.

@thejackshelton
Copy link
Member

@seleb this should be fixed in the latest version, let me know if you run into any other issues 😄

@seleb
Copy link
Author

seleb commented Jan 25, 2025

Looks like the latest update fixed all the issues we're seeing, thanks for the quick turnaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants