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

Option to skip fast refresh #346

Closed
4 tasks done
michaelboyles opened this issue Jul 9, 2024 · 6 comments
Closed
4 tasks done

Option to skip fast refresh #346

michaelboyles opened this issue Jul 9, 2024 · 6 comments

Comments

@michaelboyles
Copy link

Description

I have a custom Babel transformation which I'd like to apply using this plugin. I'm also using Remix

plugins: [
    remix(),
    viteReact({
        babel: {
           plugins: ['my-babel-plugin']
        },
    })
]

With this setup, I get this error: "Uncaught SyntaxError: Identifier 'RefreshRuntime' has already been declared"

I think the problem is that both plugins are attempting to import the same thing. I tried playing with the order, but doesn't matter.

Here's the fragment containing the problem:

import { createHotContext as __vite__createHotContext } from "/@vite/client";import.meta.hot = __vite__createHotContext("/app/root.tsx");import RefreshRuntime from "/@id/__x00__virtual:remix/hmr-runtime";const inWebWorker = typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope;let prevRefreshReg;let prevRefreshSig;if (import.meta.hot && !inWebWorker) {  if (!window.__vite_plugin_react_preamble_installed__) {    throw new Error(      "Remix Vite plugin can't detect preamble. Something is wrong."    );  }  prevRefreshReg = window.$RefreshReg$;  prevRefreshSig = window.$RefreshSig$;  window.$RefreshReg$ = (type, id) => {    RefreshRuntime.register(type, "/path/to/root.tsx" + " " + id)  };  window.$RefreshSig$ = RefreshRuntime.createSignatureFunctionForTransform;}var _s2 = $RefreshSig$();
import __vite__cjsImport2_react_jsxDevRuntime from "/node_modules/.vite/deps/react_jsx-dev-runtime.js?v=06d7f284"; const jsxDEV = __vite__cjsImport2_react_jsxDevRuntime["jsxDEV"];
import RefreshRuntime from "/@react-refresh";

Suggested solution

Add a config option e.g. enableFastRefresh, defaulting to true (current behaviour), to opt-out of fast refresh behaviour

Alternative

I could write my own vite plugin which is just a copy-paste of this one with the fast refresh stuff removed

Additional context

Since viteReact returns an array of plugins, I tried hackily using the 0-th item (the babel one), but it doesn't work because the two are tied together: the babel one applies some things required for fast refresh.

Validations

@ArnaudBarre
Copy link
Member

For me remix should either provide passing babel plugins or not vendor the fast refresh transformation. Running twice Babel is not something we want to make easy to do. cc @pcattori

@pcattori
Copy link

pcattori commented Jul 9, 2024

Yes I agree with @ArnaudBarre .

We're already done some exploratory work in Remix to better interop with vite-plugin-react : remix-run/remix#9092 . At the moment, the Remix team is working towards a release of React Router v7. The plan is to bring these improvements to Remix and React Router after v7 lands.

That said, I don't have any problem with running user-defined Babel transformations from users separately via something like https://www.npmjs.com/package/vite-plugin-babel . Of course, the optimal way is to only parse the Babel AST once and then run all the transforms. So in the future when Remix/RR get better interop with vite-plugin-react, you'll be able to do so. But in the meantime, using vite-plugin-babel could be a workaround.

@michaelboyles
Copy link
Author

Thanks for the fast replies.

@pcattori

We're already done some exploratory work in Remix to better interop with vite-plugin-react : remix-run/remix#9092

The actual change so far (understand it's a WIP) looks a bit different from what you described. It's vite-plugin-react-swc so far, so babel's not involved. Similar work will be done for vite-plugin-react too? And which plugin is wrapped by remix will be based on what they have installed?

Just extrapolating from that PR, my impression is that my vite config would end up looking something like this, where the remix plugin passes the config along

plugins: [
    remix({
        viteReact: {
            babel: {
                plugins: ['my-babel-plugin']
            }
        }
    })
]

And someone using SWC might do (just an arbitrary config property)

plugins: [
    remix({
        viteReactSwc: { tsDecorators: true }
    })
]

If so, then that works for my use-case.

@ArnaudBarre
Copy link
Member

Yeah similar API will be offer for both plugins so that Remix can configure the plugin installed by the user (babel or SWC) without needing to manage the transform part, so the final config will look like your initial code but will not have issues because remix will not try to transform files

@michaelboyles
Copy link
Author

That would be fine, but what you're describing and the current WIP don't seem to line up to me. Maybe I'm missing something.

It just includes swc-plugin inbetween the remix ones. https://github.com/remix-run/remix/blob/cef3e66cad89e31b5550b1c3ab51fbb311251353/packages/remix-dev/vite/plugin.ts#L1589

I suppose if the user declared it as well (to specify config) then it would just be applied twice. (Unless vite is able to de-dupe them? But I don't think so)

So then I guess that line I linked to would be removed in the future? Meaning remix would no longer work as a standalone plugin, and would require users to opt into 1 of the react-* ones. That's a breaking change for remix, right? Is there a scenario where the react-* plugin may need to be interleaved with the remix ones? It's currently 5th out of 7 - not sure if that's just arbitrary.

From what was described, I was picturing remix doing something more like this. It would give the flexibility for remix to adjust the order in future if that was necessary

const importSwc = async (conf) => {
    try {
        const swc = await import("@vitejs/plugin-react-swc");
        return swc.default(conf);
    }
    catch (e) { return null; }
}
const importReact = async (conf) => {
    try {
        const react = await import("@vitejs/plugin-react");
        return react.default(conf);
    }
    catch (e) { return null; }
}
export const remixVitePlugin = async (conf) => {
    const swc = await importSwc(conf.swc);
    const react = await importReact(conf.react);
    if (swc && react) throw new Error("both...")
    const reactPlugin = swc || react;
    if (!reactPlugin) throw new Error("neither...");

    return [
        { name: 'some-remix-before' },
        reactPlugin,
        { name: 'some-remix-after' }
    ]
}

@ArnaudBarre
Copy link
Member

I will let @pcattori answer for the Remix point of view, we maybe need to think on this but I yeah in the future I think it's better if remix/RRv7 is just plugin on top a base plugin that is framework/transpiler dependent. For example in one year if rolldown replace esbuild as the default TS transpiler, I will make sure it ships with builtin Fast refresh transformation so we could have a zero-deps react plugin!

@github-actions github-actions bot locked and limited conversation to collaborators Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants