search

vitejs / vite-plugin-react-swc

Speed up your Vite dev server with SWC
MIT License
830 stars 54 forks source link

feat: add parserConfig option #186

Closed ArnaudBarre closed 8 months ago

ArnaudBarre commented 9 months ago

Fixes #182

I didn't go with the classic include/exclude because this allow to return parser object and giving full control for the user at very low cost

bluwy commented 9 months ago

Personally I'd still lean on include/exclude for consistency and the easier migration from plugin-react. Plus, perhaps one would want the filter function to return more plugin options in the future? Also thinking about Rolldown, there's a chance we could be passing raw glob strings or regex in its API to minimize native<->js calls like esbuild?

ArnaudBarre commented 9 months ago

My opinion was always to wait the maximum to add this kind of option so when implementing in native we don't need complex configuration and can rely on "standards". This is theoretically nice to be able to switch the config on a per file basis, but once you once used esbuild to bundle a complete React codebase without any plugins you feel the cost of being outside of standards and I want to help people be on the right track for sub-second bundle of real world codebases

XantreDev commented 9 months ago

I also would like to have an opportunity to use swc for deps to unblock usage of swc plugins

bluwy commented 9 months ago

@ArnaudBarre I think that's a fair goal when introducing new options. But if we're introducing a new option that's almost identical to how the ecosystem already implements it, I think it's better to reach directly to it. If we waited for the maximum, we'd have to deprecate filter and introduce a migration path which isn't quite nice 🤔

Re-reading the changes, perhaps an option named parserConfig would fit better? filter for me implies returning a boolean. Also if the API is used, you'd also have to manually replicate the existing TS/MDX handling, is that intentional?

ArnaudBarre commented 9 months ago

Yes this is intentional so that it can act as an exclude at the same time and gives you back immediately full control for people that want to oup-out of the default without being complex in the code for us. I'm ok for changing the name so that it's doesn't mislead people that this is can predicate callback

bluwy commented 9 months ago

If others are ok I'd prefer renaming the option then 👍 It would also be nice to mention that they need to re-define the config for general TSX/MDX files etc too with this option.

sapphi-red commented 9 months ago

Renaming the option to prevent misleading sounds good to me.