Conversation
🦋 Changeset detectedLatest commit: 7626af2 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| }, | ||
| }, | ||
| ], | ||
| renderers: [jsxRenderer], |
There was a problem hiding this comment.
No longer added by default, only used by the mdx plugin.
packages/astro/test/fixtures/jsx/src/components/preact/PreactCounter.tsx
Outdated
Show resolved
Hide resolved
|
Amazing! This will be so huge for anyone building more complex interactive islands/SPAs with Astro! What happens if a JSX file is unmatched/unclaimed by any integration? Is it possible that I could have one integration act as the "default"? This would map to a user who uses React for everything, except for a couple of Solid or Preact components that they have on the page. // Example:
// src/components/preact/*.jsx (the special preact components)
// src/components/**/*.jsx (the rest of your codebase)
export default defineConfig({
integrations: [
preact({include: 'src/components/preact/**/*.jsx'}),
react(),
]
}) |
|
I'm pretty sure this already works, if you don't use an include the Vite plugins don't filter anything out. So you need to put that integration last. I would expect your example to work as written. So i think this is a documentation thing. |
|
The example would work, but probably not ideal because the preact JSX file will be transformed twice by Preact and React plugins? It would be better to explicitly configure the includes. |
|
@bluwy You mean for the Preact plugin? The React pass will occur after the JSX is already compiled away. So it's just a noop. Is that what you mean? Also, from my debugging, the React plugin (and possibly others) only run Babel when there's certain configuration to do so, otherwise it falls back to the esbuild plugin. |
|
Yeah it's the noop that I'm concerned, since there's still effort and perf cost there to process it. |
|
Cool, thanks for checking! |
|
This is ready for review. The lint error shown is wrong, eslint is drunk in CI (fine locally). |
natemoo-re
left a comment
There was a problem hiding this comment.
Code looks great, super excited for this! Just a few small cleanup things.
| import type { LogOptions } from '../core/logger/core.js'; | ||
| import type { PluginMetadata } from '../vite-plugin-astro/types'; | ||
|
|
||
| import babel from '@babel/core'; |
There was a problem hiding this comment.
Any chance we can drop @babel/core if this is the only place we're using it?
There was a problem hiding this comment.
In favor of esbuild you mean?
There was a problem hiding this comment.
esbuild already runs. I'm not sure what implications there are if we remove this code to be honest.
There was a problem hiding this comment.
I think the issue is that we have this tagExportsPlugin that is probably built to use babel, and maybe we cannot convert it to an esbuild plugin. Maybe let's explore it in another PR
|
Also could we add |
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
|
I think I've addressed everything I can here, would appreciate another review when people are ready. |
bluwy
left a comment
There was a problem hiding this comment.
Some minor nits, but looks good overall!
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Changes
include/excludeobjects lets you target only certain files.Testing
Docs