vitejs / vite-plugin-react-swc

Speed up your Vite dev server with SWC
MIT License
781 stars 50 forks source link

fix: transform JSX living in `node_modules/` #21

Closed brillout closed 1 year ago

brillout commented 1 year ago

I understand the performance optimization of skipping node_modules/ but AFAICT if (!parser) return; works equally well.

One use case for this is React Server Components where JSX files are living in node_modules/. (If you're curious: discussion with the React team about this.)

(Thanks for vite-plugin-react-swc, I'm really liking it and already using it production.)

ArnaudBarre commented 1 year ago

Yeah now that JS files are skipped this is fine (and I hope people will not put JSX inside .js files and then expect every tools to use a jsx parser just in case).

I want to check how this behaves with deps pre-bundling before merging. Do you already have/know a library shipping non-transpiled jsx?

brillout commented 1 year ago

I hope people will not put JSX inside .js files and then expect every tools to use a jsx parser just in case

Yes.

I want to check how this behaves with deps pre-bundling before merging. Do you already have/know a library shipping non-transpiled jsx?

There are probably already some but I don't know which one already do. My best guess would be NextAuth.js since they recently released React Server Components support.

ArnaudBarre commented 1 year ago

Thanks. I will look at this in the coming days and add a test case for that.

I will release a patch before the end of the week, and merge this as is if I don't have time to setup a test before as this looks relatively safe.

brillout commented 1 year ago

No rush on my side, it's not blocking.

silverwind commented 1 year ago

Do you already have/know a library shipping non-transpiled jsx?

I do have a few of these in private repositories, fwiw. I just seems natural to skip the whole transpilation madness 😉.

ArnaudBarre commented 1 year ago

So I tried the plugin with a fake local lib using .jsx code. What happens is that esbuild already transpile JSX during pre-bundling.

In case the lib was expecting automatic runtime and didn't have import React, you need to add: optimizeDeps: { esbuildOptions: { jsx: "automatic"} } in the config.

Build also works, with automatic runtime enforced by the plugin

Closing as already handled, feel free to open a new issue if you have a real world use case that doesn't work!

brillout commented 1 year ago

esbuild already transpile JSX during pre-bundling

I believe code living in node_modules/ isn't always pre-bundled. (I'm not sure in what circumstances though.)

FYI I did have a concrete use case that needed that PR. But it's quite an edge case that I don't need anymore.

Makes sense to close this for now since we don't want to unnecessarily transpile twice 👍.

The current status quo is fine with me, but I'll let you know if that changes.