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

Qwik and other JSX renderers in Astro #13

Closed
thejackshelton opened this issue Nov 12, 2023 · 11 comments
Closed

Qwik and other JSX renderers in Astro #13

thejackshelton opened this issue Nov 12, 2023 · 11 comments

Comments

@thejackshelton
Copy link
Member

thejackshelton commented Nov 12, 2023

There is currently an issue using Qwik alongside other JSX-based framework integrations in Astro. My understanding is that the user needs to change their file structure and astro config for this to work.

The problem, is that the renderToStaticMarkup function that Astro provides, will also try to read / render other JSX components.

For example, if we add a React component to index.astro, the @qwikdev/astro integration will log out the following from server.ts

Error in renderToStaticMarkup function of @qwikdev/astro:  TypeError: Cannot read properties of null (reading 'useState')
    at exports.useState (/home/jackshelton/dev/open-source/astro/qwikdev-astro/node_modules/.pnpm/react@18.2.0/node_modules/react/cjs/react.production.min.js:25:394)
    at ReactCounter (file:///home/jackshelton/dev/open-source/astro/qwikdev-astro/apps/astro-demo/dist/server/chunks/pages/index_886e2bc3.mjs:4409:29)

If we have it return nothing when it is not a Qwik component like this:

    if (Component.constructor.name !== "QwikComponent") {
      // If it's not a Qwik component, return an object with an empty 'html' property
      return { html: "" };
    }

Then it will still error out, as renderFrameworkComponent (being used internally by Astro), tries to look for the React client entrypoint.

NoClientEntrypoint: `ReactCounter` component has a `client:load` directive, but no client entrypoint was provided by `@qwikdev/astro`.
    at renderFrameworkComponent (file:///home/jackshelton/dev/open-source/astro/qwikdev-astro/apps/astro-demo/dist/server/chunks/astro_dfd94c1b.mjs:1573:11)
    at async renderComponent (file:///home/jackshelton/dev/open-source/astro/qwikdev-astro/apps/astro-demo/dist/server/chunks/astro_dfd94c1b.mjs:1714:10)
    at async renderChild (file:///home/jackshelton/dev/open-source/astro/qwikdev-astro/apps/astro-demo/dist/server/chunks/astro_dfd94c1b.mjs:1108:11)

My understanding, is that we need to add an include option for consumers in the astro.config.mjs, which I then think would need to be provided to qwikVite? or a new setting in the plugin?

This is where I'm a bit lost,

https://docs.astro.build/en/guides/upgrade-to/v3/#what-should-i-do-24

I saw this in the Solid integration:

plugins: [ solid({ include, exclude, dev: isDev, ssr: true }),    {
name:
  '@astrojs/solid:config-overrides', enforce : 'post', config() {
    return {
    esbuild : {  // To support using alongside other JSX frameworks, still let
   // // esbuild compile stuff. Solid goes first anyways. include:
   // /\.(m?ts|[jt]sx)$/,       },      };     },    },   
],

And then in their vite plugin:

export interface Options {
  /**
   * A [picomatch](https://github.com/micromatch/picomatch) pattern, or array of patterns, which specifies the files
   * the plugin should operate on.
   */
  include?: FilterPattern;

Which uses this:
https://github.com/micromatch/picomatch

@thejackshelton thejackshelton changed the title Qwik and other JSX renderers Qwik and other JSX renderers in Astro Nov 12, 2023
siguici added a commit to siguici/qwik-astro that referenced this issue Nov 14, 2023
thejackshelton added a commit that referenced this issue Nov 14, 2023
Fix #13: Add include and exclude options to filter entrypoints.
@thejackshelton
Copy link
Member Author

thejackshelton commented Nov 16, 2023

For those reading this, you can use the exclude keyword with the integration in the astro config with the wildcard paths of other framework folders, and that should allow you to use Qwik and other frameworks for now.

@em-jones
Copy link

em-jones commented Nov 29, 2023

There seems to be a lot of "If-then-else" logic between this thread and this documentation involving multiple pieces: tsconfig, astro.config, and .jsx files...

I don't think these instructions are accessible.

What would feel accessible would be an examples directory with the different configs for the different scenarios.

It would probably be of use for testing as well :D

ETA - Sharing this feedback as a native english speaker with 5+ years polyglot experience(most in node and .net) that is struggling to grok what the paths are.

@thejackshelton
Copy link
Member Author

There seems to be a lot of "If-then-else" logic between this thread and this documentation involving multiple pieces: tsconfig, astro.config, and .jsx files...

I don't think these instructions are accessible.

What would feel accessible would be an examples directory with the different configs for the different scenarios.

It would probably be of use for testing as well :D

ETA - Sharing this feedback as a native english speaker with 5+ years polyglot experience(most in node and .net) that is struggling to grok what the paths are.

Sounds like an awesome idea. Want to take a crack at it?

@em-jones
Copy link

I'd love to.
Can you help me fill in the missing bits of this chart ?
I'm looking for confirmation on the no path and for better clarification of what needs doing for the [x]/[xx] & [y] steps

  flowchart LR
    A["Add astro Qwik using cli `astro add` command"] --> B
    B--"yes(react)"-->C
    B--no-->D
  B{Already rendering JSX components in another framework}
  C["✔️ Do [x] with tsconfig 
✔️ do [y] with react jsx files "]
  D["✔️ Do [xx] with tsconfig"]
Loading

@thejackshelton
Copy link
Member Author

thejackshelton commented Nov 30, 2023

I'd love to. Can you help me fill in the missing bits of this chart ? I'm looking for confirmation on the no path and for better clarification of what needs doing for the [x]/[xx] & [y] steps

  flowchart LR
    A["Add astro Qwik using cli `astro add` command"] --> B
    B--"yes(react)"-->C
    B--no-->D
  B{Already rendering JSX components in another framework}
  C["✔️ Do [x] with tsconfig 
✔️ do [y] with react jsx files "]
  D["✔️ Do [xx] with tsconfig"]
Loading

This is great!

I would write down:

add @qwikdev/astro using cli npx astro add @qwikdev/astro command.

Using Qwik as your primary jsxImportSource

If you intend to use Qwik the most add:

"compilerOptions": {
  "jsx": "react-jsx",
  "jsxImportSource": "@builder.io/qwik"
}

as your jsxImportSource in tsconfig.json

This is when you have more Qwik components on the page than other JSX frameworks. OR if it's the only JSX framework you are using alongside Qwik in your Astro project.

If you have any React components, add:

/** @jsxImportSource react */
at the top of each React component file

If you have any solid components add:

/** @jsxImportSource solid-js */
at the top of each solid component file

If you have any preact components add:

/** @jsxImportSource preact */
at the top of each preact component file

When Qwik isn't the primary jsxImportSource

If you don't intend to use Qwik as your primary jsxImportSource, add:
/** @jsxImportSource @builder.io/qwik */
at the top of each Qwik component file.

This is when you may not have that many Qwik components compared to other JSX frameworks on the page.

@em-jones
Copy link

Cool. I'm going to take a crack at setting up these examples over the weekend. I'll touch base if I run into any issues.
Thanks for the guidance!

@thejackshelton
Copy link
Member Author

Cool. I'm going to take a crack at setting up these examples over the weekend. I'll touch base if I run into any issues. Thanks for the guidance!

Sounds good! 👍

@thejackshelton
Copy link
Member Author

Hey @em-jones! Any progress?

@apiel
Copy link

apiel commented Dec 14, 2023

Maybe it should be by default in the example:

/** @jsxImportSource @builder.io/qwik */ // necessary if you don't intend to use Qwik as your primary jsxImportSource

import { component$, useSignal } from "@builder.io/qwik";

export const Counter = component$(() => {
  const counter = useSignal(0);

  return <button onClick$={() => counter.value++}>My counter {counter.value}</button>;
});

I was close to give up about using Qwik with Astro because it didn't work out of the box and seem like I am the kind of person that doesn't read the whole README file... In reality, it is more that I am just looking at the different options and I cannot go deep in every documentation of each solution.

@thejackshelton
Copy link
Member Author

thejackshelton commented Dec 14, 2023

Maybe it should be by default in the example:

/** @jsxImportSource @builder.io/qwik */ // necessary if you don't intend to use Qwik as your primary jsxImportSource

import { component$, useSignal } from "@builder.io/qwik";

export const Counter = component$(() => {
  const counter = useSignal(0);

  return <button onClick$={() => counter.value++}>My counter {counter.value}</button>;
});

I was close to give up about using Qwik with Astro because it didn't work out of the box and seem like I am the kind of person that doesn't read the whole README file... In reality, it is more that I am just looking at the different options and I cannot go deep in every documentation of each solution.

I agree that this definitely needs to be included in the readme, preferably under the tsconfig section. I will do that now.

If it was in the counter example my thoughts are it would likely cause more confusion as before, as it should only be added in the case of another JSX framework being used, and if this was copy pasted without a thought, consumers might skip the actual tsconfig setup, or think that this is the default way to add types to your Qwik components.

All in all I am not happy with this solution that typescript provides us. It seems to be the same with other jsx frameworks, but hoping there is an alternative in the future.

@thejackshelton
Copy link
Member Author

thejackshelton commented Dec 29, 2023

Okay, I've found that in most scenarios after the latest changes Qwik should work alongside any other JSX framework.

The edge cases I've found seem to be caused by the other renderers.

If you come across any issues, I suggest putting the Qwik integration first in your astro.config.mjs

integrations: [qwik(), react({ include: ["**/react/*"] })]

You should not need to use an include or exclude option for qwik.

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

No branches or pull requests

3 participants