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

HMR improvements #503

Closed
jods4 opened this issue Sep 6, 2024 · 18 comments
Closed

HMR improvements #503

jods4 opened this issue Sep 6, 2024 · 18 comments
Labels
⚡️ enhancement improvement over an existing feature

Comments

@jods4
Copy link
Contributor

jods4 commented Sep 6, 2024

I would like to suggest a few improvements to HMR.

I observe updateRoutes is only invoked when adding or removing page.

In updatePage there's a comment:

// no need to manually trigger the update of vue-router/auto-routes because
// the change of the vue file will trigger HMR

That's not accurate.
Vite will notice the module has changed and invalidate it, but it will not actively push it through HMR to clients.
As auto-routes is typically not referenced by pages or components, only by the module that create routes, it might not be reloaded for a long time, if ever.
It also looks that this "late update" is not considered HMR and just creates duplicate modules in the front-end, which led me to all sorts of confusing issues.
I would suggest to trigger HMR when the routes are modified

Is this really the right place to do it though?
Consider:

  1. extendRoutes and beforeWriteFiles can do a lot of shenanigans that may have different dependencies and are basically impossible to predict.
  2. Updates trigger basically on every page edit. That's often , and most of the time no route-impacting change is actually made.

For these reasons I would suggest a different approach altogether: after the auto-routes virtual file has been rebuilt, compute a hash. If the hash is different from previous generation, send HMR. This covers all possible changes (add/remove/update/extensibility changes) and never sends unnecessary HMR to front-end.

Tip

Some tips for people googling how to update more than the router itself:
auto-route exports handleHotUpdate(router), which you can use to register your app router. With this, your router will automatically be updated whenever auto-routes is HMR. Very cool!
If you want to update more than the router, for example if your navigation menu is built off of your route table... it's possible but not obvious.
Because auto-routes self-accepts HMR, it won't propagate further: don't try to accept it with hot.accept("/__vue-router/auto-route").
The way to go is to subscribe to notifications with import.meta.hot.on('vite:afterUpdate', args =>...) and filter args.updates.some(u => u.acceptedPath === '/__vue-router/auto-routes')
Note that you will not find the new module in args and the self-accepting HMR does not update the existing exports, only the router.
So to get the updated routes, your only way is to get your router and call router.getRoutes().

@posva Maybe this process is worth simplifying?
The obvious idea (to me) is a custom message hot.on('auto-routes:afterUpdate', routes => ..) but the bummer is that these messages can only be sent by server.
Alternatively auto-routes could export a onRouteUpdated(routes => ..) function to register listeners that are called by accept?

@posva posva added the ⚡️ enhancement improvement over an existing feature label Sep 7, 2024
@posva
Copy link
Owner

posva commented Sep 7, 2024

The process is definitely worth simplifying, maybe even fixed. Without the current implementation, there was no invalidation at all and things were even worse...
The info you provided here is really helpful, thank you. I will definitely take a look

@mrhammadasif
Copy link

mrhammadasif commented Dec 30, 2024

Implementing the HMR using the instructions from the HMR page reloads the full page, but ignores the layout file. After HMR, the page just shows the content of the pages/page.vue

import { handleHotUpdate } from 'vue-router/auto-routes'
// TODO this is not working
if (import.meta.hot) {
  handleHotUpdate(router)
}

@cucuzi
Copy link

cucuzi commented Jan 10, 2025

Implementing the HMR using the instructions from the HMR page reloads the full page, but ignores the layout file. After HMR, the page just shows the content of the pages/page.vue

import { handleHotUpdate } from 'vue-router/auto-routes'
// TODO this is not working
if (import.meta.hot) {
  handleHotUpdate(router)
}

Same problem. Add the following code.

router.beforeEach((to, from, next) => {
  console.log(to)
  next();
})

First visit:
image
Upadate file:
image
The route lose a matched.

@posva
Copy link
Owner

posva commented Jan 21, 2025

@jods4 Do you have small examples I can debug in the playground regarding this:

That's not accurate.
Vite will notice the module has changed and invalidate it, but it will not actively push it through HMR to clients.
As auto-routes is typically not referenced by pages or components, only by the module that create routes, it might not be reloaded for a long time, if ever.
It also looks that this "late update" is not considered HMR and just creates duplicate modules in the front-end, which led me to all sorts of confusing issues.
I would suggest to trigger HMR when the routes are modified


Regarding the suggestions, are you suggesting both things?

  • Adding a hook for dev only to fix some cases (routes.getRoutes(), importing from auto-routes)
  • Calling server.updateRoutes() (triggers HMR) each time the virtual auto-routes file changes?

@posva
Copy link
Owner

posva commented Jan 21, 2025

@cucuzi Can you try cloning the repository and reproducing the HMR error within the playground? (pnpm run play and check playground folder)

@jods4
Copy link
Contributor Author

jods4 commented Jan 21, 2025

Hi @posva !

I will try to reformulate the suggestions. There was basically 3 points in my issue.

  1. You should call updateRoutes when the route table is updated to push HMR update to client.

Note

Explanation/reprodution of one potential issue (it was a long time ago, keep in mind my memory could be fuzzy):

If the Vue file you've modified is not loaded yet in client, then no HMR will take place.
Yet auto-routes has actually changed on server and will be served with a different timestamp from now on!
When the client then loads a module that (directly or indirectly) imports auto-routes, it has different URL, so it will be seen as a different module by browser.
Now you end up with 2 different auto-routes modules loaded on the client. As well as any transitive module that could be on the loading path of your component.
This easily leads to very mysterious bugs.

  1. Calling updateRoutes right when a .vue file is added/updated/removed can lead to a lot of unnecessary HMR.
    Not all edits impact the route table (actually: most don't). And extendRoutes and beforeWriteFiles are impossible to account for.
    Instead I would suggest to compute a hash of auto-routes after it's generated and call updateRoutes when that hash changes.

  2. It would be nice to have an API on the client to be notified of the new routes after HMR.
    Something like onRouteUpdated(routes => ..) or similar.
    It's useful (e.g. to update a navigation menu) and it's not easily doable today.

@jods4
Copy link
Contributor Author

jods4 commented Jan 21, 2025

For what it's worth, in my plugin config I added this piece of code in beforeWriteFiles and it really helped with 1.

const module = server?.moduleGraph.getModuleById("/__vue-router/auto-routes");
if (module) server.reloadModule(module);

It's a hack and triggers too much, but it helps until this issue gets fixed.

@posva
Copy link
Owner

posva commented Jan 24, 2025

Thanks for taking the time! I'm struggling to reproduce the duplication of auto-routes but I think I've seen it

Instead I would suggest to compute a hash of auto-routes after it's generated and call updateRoutes when that hash changes.

Internally we do not rewrite the dts file but we do need to call the hooks in order to let users apply changes. So we can control that internally and I will play around with it to push changes forward. If that work, we shouldn't need an API like onrouteUpdated right? Ideally, things should just work (e.g. import routes from vue-router/auto-routes will still reload the component, this is what I tested locally)

@jods4
Copy link
Contributor Author

jods4 commented Jan 24, 2025

If that work, we shouldn't need an API like onrouteUpdated right? Ideally, things should just work (e.g. import routes from vue-router/auto-routes will still reload the component, this is what I tested locally)

Here's a scenario from the applications I work on:

  • At startup the routing table is processed to produce a (static) navigation table that is bound to the menus.
  • Amongst other things, this transformation creates a hierarchical structure from the flat routing, localises page names, remove inaccesible routes (access rights), and more.

It would be nice to have an event to let us know the routes have changed and rebuild that table.

Could we work around this without an event?
In limited scenarios, yes. For example if the navigation table was only used by the menu component, it could import the routing table and perform the transformation when it's loaded.
When the routes are HMR, the component would be reloaded with an opportunity to rebuild the navigation.

But if you share that navigation across multiple components, it won't work.
You could build the navigation in a root component (app?) and then inject it, that would work I guess, although now routes HMR has the drawback of being a full app reload, slightly defeating the purpose?

What do you think?

EDIT: Honestly, if the route reloaded event is too annoying to add, don't bother. There are hacks around it and it has a limited (advanced) use.
What we do currently is detect HMR and get the new routes from the router instance. I assume we could also use a dynamic import.

@posva
Copy link
Owner

posva commented Jan 24, 2025

I'm considering adding an optional callback to handleHotUpdate that runs after replacing the routes. This allows for runtime-based added routes like redirects. Does that also work for your use case

@jods4
Copy link
Contributor Author

jods4 commented Jan 24, 2025

I think it does. The routes will be available to callback?

@posva
Copy link
Owner

posva commented Jan 24, 2025

Good idea! They can be made available, they should also be accessible with router.getRoutes() since it happens after but maybe it makes sense to allow running it before replacing the routes

@jods4
Copy link
Contributor Author

jods4 commented Jan 24, 2025

yes: router.getRoutes() is how I got the routes back.
It wasn't convenient as the router wasn't readily available to the HMR handler and I had to hack a bit to expose it.
Direct access to the export of auto-routes would be nice.

@posva
Copy link
Owner

posva commented Jan 26, 2025

So something like this:

import { createRouter, createWebHistory } from 'vue-router'
import { routes, handleHotUpdate } from 'vue-router/auto-routes'

export const router = createRouter({
  history: createWebHistory(),
  routes,
})

function addRedirects() {
  router.addRoute({
    path: '/new-about',
    redirect: '/about?from=/new-about',
  })
}

if (import.meta.hot) {
  handleHotUpdate(router, (routes) => {
    addRedirects()
  })
} else {
  // production
  addRedirects()
}

The callback would trigger after replacing routes by default. If needed, another syntax can be added handleHotUpdate(router, { before, after })

@jods4
Copy link
Contributor Author

jods4 commented Jan 26, 2025

@posva That would work for me

@posva posva closed this as completed in 170df11 Jan 26, 2025
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in unplugin-vue-router Jan 26, 2025
@posva
Copy link
Owner

posva commented Jan 26, 2025

I tested HMR more this time and things seem to be working well now!

@cucuzi
Copy link

cucuzi commented Jan 27, 2025

@cucuzi Can you try cloning the repository and reproducing the HMR error within the playground? (pnpm run play and check playground folder)

I think it's caused by import the 'vite-plugin-vue-layouts'.
I generated the router based on the code in the document. Extending routes at runtime

posva added a commit that referenced this issue Jan 27, 2025
@posva
Copy link
Owner

posva commented Jan 27, 2025

I updated the docs for runtime routes + HMR. Feel free to send a PR to improve!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡️ enhancement improvement over an existing feature
Projects
Status: Done
Development

No branches or pull requests

4 participants