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.

Sign up for GitHub

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

Jump to bottom

Full app reload, always #55

Closed
jods4 opened this issue May 18, 2021 · 4 comments
Closed

Full app reload, always #55

jods4 opened this issue May 18, 2021 · 4 comments

Comments

@jods4
Copy link

jods4 commented May 18, 2021 •
edited by hannoeru
Loading

While testing 0.11.4, I noticed that no matter what I change in a page, a full page reload is triggered.
For example, if I change a button label -> full page reload.

I suspect it was introduced by the fix for #52.
Page change -> potential <route> block change -> regenerate routing (always?) -> HMR not possible for routes.js -> full reload (?)

If <route> didn't change, that process should stop early and not induce an HMR failure.
@jods4 jods4 mentioned this issue May 18, 2021
Closed
@hannoeru
Copy link
Owner

I can't repro it. Can you provide more context or reproduction?

@jods4
Copy link
Author

jods4 commented May 18, 2021

@hannoeru sorry about that.
It took me a bit of time to manage to build a repro because this bug is triggered by something very specific to my project.

Here's a repro: https://github.com/jods4/bug-vite-pages
Click on the counter button, then save index.vue (no change) and it triggers a full reload.

I found what specifically triggers the bug:
I have a extendRoute option that removes some properties that were created by <route> (in real life it does a little more, but for this repro, delete route.nav is enough.
https://github.com/jods4/bug-vite-pages/blob/master/vite.config.js#L14

Armed with this knowledge, I dug into the codebase and I think I found an explanation.
The plugin performs a full reload when isRouteBlockChanged:

vite-plugin-pages/src/generate.ts

Lines 145 to 158 in 6096a33

export function isRouteBlockChanged(filePath: string, routes: Route[], options: ResolvedOptions): boolean {
const routeBlock = getRouteBlock(filePath, options)
if (routeBlock) {
const route = findRouteByFilename(routes, filePath)
if (route) {
const before = Object.assign({}, route)
debug.sfc('route: %O', routeBlock)
Object.assign(route, routeBlock)
return !deepEqual(before, route)
}
}
return false
}

This function doesn't actually compare the <route> block text to see if it has changed.
Instead it compares the old route with the same route with the new block applied.
This doesn't account for the fact that extendRoute may have been used to modify properties that initially came from the route block!

I suppose there are several ways to fix this, e.g.:

  • Call extendRoute on the new object before doing the comparison;
  • Keep (and compare) the text of the route block.

@hannoeru
Copy link
Owner

Try v0.12.1

@jods4
Copy link
Author

jods4 commented May 19, 2021

That works!

Thank you for that plugin.
I had the same idea and was about to start writing a plugin when I found it. It's great! 😃

ckangwen pushed a commit to charrue/vite-plugin-pages that referenced this issue Jan 24, 2023
@hannoeru
fix: compare original route block to trigger hmr currently, close han…
ab2e93e 
…noeru#55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jods4 @hannoeru