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

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

Closed
1 task
toondkn opened this issue Sep 1, 2022 · 5 comments · Fixed by #4759
Closed
1 task

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

toondkn opened this issue Sep 1, 2022 · 5 comments · Fixed by #4759
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@toondkn
Copy link

toondkn commented Sep 1, 2022

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
Copy link
Contributor

JohnDaly commented Sep 2, 2022

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

  1. Checking for pragma comments in the file
  2. Checking for imports in the file

Heres the code where that happens:

if (defaultJSXRendererEntry && jsxRenderersIntegrationOnly.size === 1) {
// downlevel any non-standard syntax, but preserve JSX
const { code: jsxCode } = await esbuild.transform(code, {
loader: getEsbuildLoader(path.extname(id)) as esbuild.Loader,
jsx: 'preserve',
sourcefile: id,
sourcemap: 'inline',
});
return transformJSX({
code: jsxCode,
id,
renderer: defaultJSXRendererEntry[1],
mode,
ssr,
});
}
let importSource = detectImportSourceFromComments(code);
if (!importSource && IMPORT_KEYWORD_REGEX.test(code)) {
importSource = await detectImportSourceFromImports(code, id, jsxRenderers);
}
// if we still can’t tell the import source, now is the time to throw an error.

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
Copy link
Author

toondkn commented Sep 5, 2022

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
Copy link
Contributor

JohnDaly commented Sep 5, 2022

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:

Unable to resolve a renderer that handles this file! With more than one renderer enabled, you should include an import or use a pragma comment.

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

@matthewp
Copy link
Contributor

matthewp commented Sep 6, 2022

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

@matthewp matthewp added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Sep 6, 2022
@matthewp matthewp self-assigned this Sep 12, 2022
@matthewp
Copy link
Contributor

This PR will read from the tsconfig to determine which jsxImportSource to use: #4759

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants