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

Add option to disable full reload (hot only) #6695

Closed
4 tasks done
cdauth opened this issue Jan 31, 2022 · 17 comments
Closed
4 tasks done

Add option to disable full reload (hot only) #6695

cdauth opened this issue Jan 31, 2022 · 17 comments
Labels
enhancement: pending triage p2-to-be-discussed Enhancement under consideration (priority)

Comments

@cdauth
Copy link

cdauth commented Jan 31, 2022

Clear and concise description of the problem

I'm developing an app that renders some large forms. Hot module reload is great for this, but full page reloads are quite annoying, because they cause the form state to be lost whenever I edit some unrelated file, and they cause the beforeunload handler to trigger when I save a file, causing the browser to steal the focus. Generally, I personally find full page reloads rather undesirable and annoying, while I find hot module reloads really amazing and helpful.

Suggested solution

I would like to have an option to disable full reloads all together, but keep hot module replacement enabled, similar to webpack’s hot only option.

Alternative

No response

Additional context

No response

Validations

@tomitrescak
Copy link

I second that. Vite is now doing full page reloads and we do not understand why.

Is there at least some debug info which can say why the full page refresh happens?

@wqcstrong
Copy link

wqcstrong commented Feb 16, 2022

Is there at least some debug info which can say why the full page refresh happens?

Same problem, same support request.

@bluwy
Copy link
Member

bluwy commented Feb 16, 2022

Is there at least some debug info which can say why the full page refresh happens?

You can run DEBUG="vite:*" npm run dev to show Vite logs and find at what stage it triggers a fill page reload.

@tomitrescak
Copy link

@bluwy Thanks, I tried that, but I do not see anything striking on why does it do a full page reload:

https://gist.github.com/tomitrescak/0ffaddf56c858305c400020d09720459

Check out that log, is it possible to read anything from there?

@wqcstrong
Copy link

In my case, I mistakenly think that the react plugin would merge the include option, the following is my config:

import react from '@vitejs/plugin-react';

export default defineConfig({
  ...
  plugins: [
    react({
      jsxRuntime: 'classic',
      include: ['**/*.json', '**/*.less'] // it's unnecessary and cause the page full-reload
    })
  ]
})

@bluwy
Copy link
Member

bluwy commented Feb 16, 2022

Yeah the include needs to specify **/*.tsx too, which should fix it.

@wqcstrong
Copy link

@bluwy The statement in the @vitejs/plugin-react doc may be need to adjust:

In some situations, you may not want a file to act as a HMR boundary, instead preferring that the changes propagate higher in the stack before being handled. In these cases, you can provide an include and/or exclude option, which can be a regex, a picomatch pattern, or an array of either. Files matching include and not exclude will use Fast Refresh. The defaults are always applied.

The last statement misled me.

@patak-dev
Copy link
Member

PR Welcome to improve the wording.

About adding an option to disable full reload, I think it would be better to understand why your app is doing it instead of HMR kicking in. I don't think it is a good idea to leave the app in a corrupted state if we add this option. I think maybe there is a way to write a plugin to achieve the same out of core, and then we aren't responsible for users playing with fire.

@bluwy
Copy link
Member

bluwy commented Feb 16, 2022

@bluwy Thanks, I tried that, but I do not see anything striking on why does it do a full page reload:

https://gist.github.com/tomitrescak/0ffaddf56c858305c400020d09720459

Check out that log, is it possible to read anything from there?

@tomitrescak From the logs, looks like src/modules/ei/ei_layout.tsx triggers the full page reload. This can usually happen if the file is near the top of the HMR tree, since the entire components below it has to re-render. For example, the index.html imports main.js which imports that file. If not, maybe it's an issue with plugin-react?

I think this starts to deviate from the original issue though. Feel free to continue the discussion in GitHub discussions or discord. Or if you have a repro and think it's a bug, you can open an issue too.

@cdauth
Copy link
Author

cdauth commented Feb 16, 2022

As the original reporter of this issue, I want to emphasize that I generally do not want full reloads, regardless of whether they are justified or caused by a misconfiguration. In my case full reloads usually happen when I include an additional file from an external library or change a file in an external library (which happens for me quite frequently since my app is split into a React app and a common library), or when I kill the dev server because it has grown to several GB of memory consumption.

When I'm working on my code and have saved multiple files multiple times and then want to look up something in the API docs, I first have to click away 50 unload handler alert boxes that the full reload has raised in the meantime, which cause Chrome to not allow interacting with any other tab/window while they are open.

With Vite’s dev server mechanism of serving each file individually rather than bundling them, a full reload takes the browser 5–10 seconds to load the around 350 files that it has to load in my case (and my app isn't even that big yet). I quite often find myself in a situation where I'm editing a file and then quickly want to look at something in the app again, but because of the full reload I have to wait until the page has reloaded.

And even if all of this weren't the case, I would still not want a full reload, since it causes me to lose the app state when I was maybe in the middle of testing something out in the app. It is very intrusive and just doesn't give me any benefit. If I want to do a full page reload, I can easily do it myself, and do it in the moment when I feel like it should be done. Before I migrated to Vite I had neither hot module reloads nor full page reloads, and even though I really like HMR, I actually found the situation before more developer friendly over all because it was not so intrusive.

I would suggest to add an option to disable all full page reloads. If a full page reload is needed, a message can be printed on the terminal to let me know.

@bluwy
Copy link
Member

bluwy commented Feb 17, 2022

I still don't think it's a beneficial feature overall. Giving an option to disable full reloads allows users to shoot themselves in the foot, and that isn't really good DX IMO. Vite doesn't do full reload unnecessarily unless it absolutely needs to to ensure application consistency. You don't want to be working in an app that still contains your old code.

Re unload handlers, you could have a special case to prevent that in dev mode, if it's needed to reload anyways. Re 5-10 seconds load, you might want to lazy load some parts of the app to speed things up, which would be a good thing to do overall in prod. But 350 files doesn't sound like a lot and I'm not sure why that is slow on your end.

@cdauth
Copy link
Author

cdauth commented Feb 24, 2022

For my use case it is just absolutely unpractical.

I noticed another regression that this introduces. I'm developing Confluence apps which are running in an iframe. Sometimes there are multiple iframes of the same app open at the same time, for example when an iframe opens a dialog, which is opened in another iframe on top. With the full reload feature, when making a change, both iframes reload, which spams the browser console with messages from the underlying iframe that I'm not currently working on, and it also breaks the connection between the opener and the dialog.

I don't see how introducing an opt-in configuration option can worsen the DX. No one is forced to use it. In my use case the DX would be improved a lot by the option. I would be interested to work on a pull request to introduce such an option, but if you say that you wouldn't want that feature in the tool (not sure if you are a maintainer or who makes the decisions about that), I would rather switch to another build tool.

@patak-dev
Copy link
Member

Hey @cdauth, before adding this to core, I think we should explore if it is possible to achieve the same without an option. Here is another issue with a more general idea #5763, and maybe what you are looking for could be achieved by throwing an error from a vite:beforeFullReload listener. See https://vitejs.dev/guide/api-hmr.html#hot-on-event-cb.

I still don't understand how the internal state of the module graph will remain after a fullReload is ignored though. As an example, there are fullReloads that aren't due to HMR failing but because a missing dependency was found, triggering pre-bundling and invalidating node deps in your app (you could end up with two vue runtimes for example). A fullReload is needed here or nothing will work.

I added this issue to be discussed in a team meeting, but I can't promise when we will get to it. You are free to switch to another build tool if you think there are better tradeoffs in it.

@cdauth
Copy link
Author

cdauth commented Feb 24, 2022

Thank you for the tip, I will see if I can achieve anything with such an event handler.

I don't know enough about HMR to tell how it works technically. All I know is that I have been using the hot only option in a Vue.js app running on a webpack dev server for many years, and it behaves exactly in the way that I would expect. My impression is that it simply doesn't reload a component anymore if too much of its structure/dependencies changes. Which is perfect for my development workflow where I like to rely on HMR for small style, text or DOM changes, but for larger structural changes I like to do a manual reload, making sure I only reload only the iframe that I want to reload and that I can clear the browser console before.

@patak-dev patak-dev added the p2-to-be-discussed Enhancement under consideration (priority) label Feb 25, 2022
@cdauth
Copy link
Author

cdauth commented Mar 16, 2022

For now this workaround seems to work:

import 'vite/types/importMeta.d'; // Not needed when not using TypeScript

if (import.meta.hot) {
    import.meta.hot.on('vite:beforeFullReload', () => {
        throw '(skipping full reload)';
    });
}

It is not the perfect solution because it prints a (single-line) error on the browser console each time a full reload is attempted, but it successfully prevents the reload. It also seems that hot-module replacement still works properly after a prevented full reload.

@patak-dev
Copy link
Member

I think we should close this issue and focus the discussion on #5763, since with that feat you could implement the same in a more clean way.

@cdauth
Copy link
Author

cdauth commented Mar 16, 2022

Personally I would still prefer a configuration option, since I think it is common to want to disable full reloads, so it shouldn't involve having to add some internal API calls to the frontend code. But the suggested solution works for me, so at least my use case is solved.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: pending triage p2-to-be-discussed Enhancement under consideration (priority)
Projects
Archived in project
Development

No branches or pull requests

5 participants