fix: prevent unnecessary reloads
Description
I'm working on a Tailwind project, when I update an html file that is tracked by Tailwind, Vite full-reloads all pages.
If files are tracked by Tailwind they bypass this check: https://github.com/vitejs/vite/blob/a9ccedd18a20b2fb5e15df191f7d4f1ff464e04b/packages/vite/src/node/server/hmr.ts#L105
In my case, hmrContext.modules is:
hmrContext.modules
[
ModuleNode {
id: 'C:/Users/sib/dev/luidev/components/HTML/default.html',
file: 'C:/Users/sib/dev/luidev/components/HTML/default.html',
importers: Set(1) {
ModuleNode {
id: 'C:/Users/sib/dev/luidev/server/style.css',
file: 'C:/Users/sib/dev/luidev/server/style.css',
importers: [Set],
importedModules: [Set],
acceptedHmrDeps: Set(0) {},
transformResult: [Object],
ssrTransformResult: null,
ssrModule: null,
ssrError: null,
lastHMRTimestamp: 1653670227215,
lastInvalidationTimestamp: 0,
url: '/style.css',
type: 'js',
isSelfAccepting: true
}
},
importedModules: Set(0) {},
acceptedHmrDeps: Set(0) {},
transformResult: null,
ssrTransformResult: null,
ssrModule: null,
ssrError: null,
lastHMRTimestamp: 0,
lastInvalidationTimestamp: 1653670238242,
url: '/@fs/C:/Users/sib/dev/luidev/components/HTML/default.html',
type: 'js',
isSelfAccepting: false
}
]
When they bypass here, they fall into here: https://github.com/vitejs/vite/blob/a9ccedd18a20b2fb5e15df191f7d4f1ff464e04b/packages/vite/src/node/server/hmr.ts#L164-L171
Which is the source of problem. This part of code doesn't send path to full-reload events and this cause full reload on all pages:
https://github.com/vitejs/vite/blob/a9ccedd18a20b2fb5e15df191f7d4f1ff464e04b/packages/vite/src/client/client.ts#L116-L131
I assume this is what happened to @cdauth, as he described in #6695:
form state to be lost whenever I edit some unrelated file
Additional context
What is the purpose of this pull request?
- [x] Bug fix
- [ ] New Feature
- [ ] Documentation update
- [ ] Other
Before submitting the PR, please make sure you do the following
- [x] Read the Contributing Guidelines.
- [x] Read the Pull Request Guidelines and follow the Commit Convention.
- [x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
- [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g.
fixes #123). - [x] Ideally, include relevant tests that fail without this PR but pass with it.
I guess changing
if (!hmrContext.modules.length) {
// html file cannot be hot updated
if (file.endsWith('.html')) {
config.logger.info(colors.green(`page reload `) + colors.dim(shortFile), {
clear: true,
timestamp: true
})
ws.send({
type: 'full-reload',
path: config.server.middlewareMode
? '*'
: '/' + normalizePath(path.relative(config.root, file))
})
} else {
// loaded but not in the module graph, probably not js
debugHmr(`[no modules matched] ${colors.dim(shortFile)}`)
}
return
}
to
// html file cannot be hot updated
if (file.endsWith('.html')) {
config.logger.info(colors.green(`page reload `) + colors.dim(shortFile), {
clear: true,
timestamp: true
})
ws.send({
type: 'full-reload',
path: config.server.middlewareMode
? '*'
: '/' + normalizePath(path.relative(config.root, file))
})
return
}
if (!hmrContext.modules.length) {
// loaded but not in the module graph, probably not js
debugHmr(`[no modules matched] ${colors.dim(shortFile)}`)
return
}
is better. (I'm not sure if I'm correct.)
I'm running into a slightly different issue, but the fix is the same. Is there a good reason not to add the path that caused the full-reload to the 'full-reload' event payload? In my case, I'm experimenting with vite-node and would like to know the path that caused the full reload to construct a nice log message.
Regarding sapphi's comment, I suppose if the HTML was ever in the
hmrContext.modules, it may be safer to invalidate the modules anyway to prevent potential issues with tailwind.
With the updated understanding over the last two years, I agree with that. 😅
I think we should close this one as stale, @sibbng @philipp-spiess if this is still an issue in Tailwinds v4, would you create an issue so we can properly track this? And you can send a new PR to continue this discussion against it in that case.