JSX import source goes wrong when using Astro with React + SolidJS + TypeScript

toondkn commented 2 years ago

What version of `astro` are you using?

1.1.3

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

Compiling an Astro site (live, with astro dev) that consumes React and SolidJS components written in TypeScript causes errors.

More specifically, it throws the following error: "Failed to parse source for import analysis because the content contains invalid JS syntax. If you are using JSX, make sure to name the file with the .jsx or .tsx extension.".

All React and SolidJS files have a .tsx extension, this error is misleading. It at least appears to me there is something more going on.

Some more details about the setup:

jsxImportSource is defined in tsconfig.json, as solid-js
for React components, the magic comment /** @jsxImportSource react */ is used
adding the magic comment /** @jsxImportSource solid-js*/ to the SolidJS component works around the issue, but this is not expected behavior, as the jsx import source is expected to be read from tsconfig.json
even when providing said magic comment to a SolidJS component, if that component in turn imports and uses another SolidJS component, the errors become vague, lacking any correct description of what/where goes wrong, the limited info seems to point in the direction of the wrong JSX import source being used for the consumed component
adding a magic comment to the consumed component does not get rid of the vague errors, the developer is stuck

Thankfully, this behavior seems consistent and is reproducible on stackblitz, see the provided link.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-jhuzfh-anftcy?file=src/pages/index.astro

Participation

[ ] I am willing to submit a pull request for this issue.

JohnDaly commented 2 years ago

I think the problem is that when there is more than one JSX renderer configured, Astro attempts to detect the import source by:

Checking for pragma comments in the file
Checking for imports in the file

Heres the code where that happens: https://github.com/withastro/astro/blob/7310e8a1780dff2ffb57f8a6cfd3d021d019f6b8/packages/astro/src/vite-plugin-jsx/index.ts#L204-L226

This breaks down in your Solid example, because the solid-js import is stripped away since VoidComponent is not needed at runtime. As a result, Astro isn't able to determine the jsxImportSource, and you get the error.

If you add an import that is needed at runtime (e.g. createSignal), and use it in your Solid component, the jsxImportSource should be resolved as expected.

toondkn commented 2 years ago

Thanks for the insight. Is this also the cause for the last 2 bullet points of this report?

Do you see value in having Astro read jsxImportSource from the closest-up-the-tree tsconfig.json for each component file?

This would make working with Astro in a monorepo with separate workspaces for React and SolidJS components easier because it prevents the need to tag every single component with a pragma. Relying on an import statement that might not always exist for basic components seems like an anti-pattern to be honest.

JohnDaly commented 2 years ago

Is this also the cause for the last 2 bullet points of this report?

I believe so. All components whose jsxImportSource cannot be determined by pragma comment or imports will have this issue, when multiple JSX renderers are configured

Do you see value in having Astro read jsxImportSource from the closest-up-the-tree tsconfig.json for each component file?

Yeah, that seems like a reasonable behavior to me.

The Astro docs suggest that the tsconfig.json should be consulted for the default jsxImportSource, when there are multiple frameworks: https://github.com/withastro/docs/blob/82fa78f37f3fc306a3d5ac74c0b21a9f973a15e8/src/pages/en/guides/typescript.md#errors-typing-multiple-jsx-frameworks-at-the-same-time

However, the error message that is thrown recommends setting an import or adding a pragma comment when there are multiple renderers: https://github.com/withastro/astro/blob/e76c2afd1fb679ab2f6c6ed69a04aacd789ac28a/packages/astro/src/vite-plugin-jsx/index.ts#L233

We'll need some clarification about what the intended behavior should be here.

matthewp commented 2 years ago

I think this might be a deeper problem. That error is coming from Vite which is pretty unexpected.

matthewp commented 2 years ago

This PR will read from the tsconfig to determine which jsxImportSource to use: https://github.com/withastro/astro/pull/4759

withastro / astro