search

vitejs / vite-plugin-react-swc

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

fix: compatibility with react-refresh runtime in @vitejs/plugin-react #20

Closed ElMassimo closed 1 year ago

ElMassimo commented 1 year ago

Description 📖

This pull request fixes compatibility with the runtime served by @vitejs/plugin-react.

Background 📜

The preamble used in the @vitejs/plugin-react plugin imports the default export from the runtime, calling injectIntoGlobalHook from it.

In contrast, the runtime served by this plugin does not provide a default export.

This affects compatibility with external integrations, such as Vite Ruby and the Laravel Vite integration, which were implemented to use the same preamble as the one used by @vitejs/plugin-react.

The Fix 🔨

Adding a default export to the runtime served by this plugin.

It doesn't affect backwards compatibility, so it can be shipped in a patch release.

Notes ✏️

Although we could potentially modify the @vitejs/plugin-react plugin instead to use named exports, it would remain incompatible with previous versions of the backend integrations and other consumers of the runtime.

This change has significantly less friction, and would allow any users of @vitejs/plugin-react in backend integrations to seamlessly switch to this plugin.

ArnaudBarre commented 1 year ago

Thanks for the PR and the background explanation. I don't particularly like default export but in the case of pure ESM for client that's fine for me.

Can you add a comment before the default export like // For compatibility with Babel plugin (#20) and a line in the changelog?

ElMassimo commented 1 year ago

Will do, although it would be nice to configure automatic changelog generation as in the main Vite repo, it will automatically link related commits and PRs 😃

ArnaudBarre commented 1 year ago

We're discussing that with the team and we will try to move in the other direction. If you don't read esbuild releases notes, you should subscribe, there are really well crafted and after that you really dislike generated changelog!

Edit: For example in the changelog you can says more than in a commit title and tell that this fixes integration for Ruby/Laravel, which you will more clear for users that could have encounter the problem and just discarded the migration after an error that they didn't understood.