feat(server): invalidate module when watch files change#4461
feat(server): invalidate module when watch files change#4461mattcompiles wants to merge 7 commits intovitejs:mainfrom
Conversation
|
I restarted the GH Actions, but the windows container failed again! |
Co-authored-by: patak <matias.capeletto@gmail.com>
|
I think this solution is a good way to implement this feature. But, at least to my interpretation of the module graph, this is changing the meaning of // a single file may corresponds to multiple modules with different queries
fileToModulesMap = new Map<string, Set<ModuleNode>>()IIUC, the In my opinion, this is ok. The alternative would be to add an extra |
Co-authored-by: Shinigami <chrissi92@hotmail.de>
|
@patak-js Thanks for the review. Understand your comments. I was just trying to add the feature while creating the least amount of new concepts. Happy for any feedback on approach or to rename |
|
Lets see what others think, or we'll discuss it in a few days with the team 👍🏼 |
| addWatchModuleToFile(file: string, id: string): void { | ||
| file = normalizePath(file) | ||
| let fileMappedModules = this.fileToModulesMap.get(file) | ||
| if (!fileMappedModules) { | ||
| fileMappedModules = new Set() | ||
| this.fileToModulesMap.set(file, fileMappedModules) | ||
| } | ||
|
|
||
| const moduleNode = this.idToModuleMap.get(id) | ||
| if (moduleNode) { | ||
| fileMappedModules.add(moduleNode) | ||
| } | ||
| } |
There was a problem hiding this comment.
@mattcompiles what do you think if replace addWatchModuleToFile with
ensureModulesByFile(file: string): void {
file = normalizePath(file)
let fileMappedModules = this.fileToModulesMap.get(file)
if (!fileMappedModules) {
fileMappedModules = new Set()
this.fileToModulesMap.set(file, fileMappedModules)
}
return fileMappedModules
}So we can then use it in transformRequest.ts.
transformResult.watchFiles.forEach((file) => {
moduleGraph.ensureModulesByFile(file).add(mod)
})If we end up going this way, I think it is better to avoid the ( mod -> mod.id -> mod ) currently present in the PR. ensureModulesByFile could also be used in another PR to refactor and simplify a few function implementations in the moduleGraph.
Something I forgot yesterday, last time that we discussed #3216 we saw that there was a lot of support (a lot of +1 reactions) but there wasn't much in concrete examples of where this feature would help. If you have a use case for this feature, it would be great if you could add a comment in the issue describing it so we leave it as reference.
There was a problem hiding this comment.
Happy for any suggested strategies by the core team. I'll wait for a final opinion before updating the code though if that's ok.
|
Closing in favour of db4ba56 |
|
I see that Evan added the watched files as extra imports at the end 👍🏼 @mattcompiles, the tests in this PR are still valuable. Would you like to reopen the PR and repurpose it for them? |
Description
As specified in the rollup
addWatchFiledocs.This change will invalidate the current module if any changes are detected from the files passed to
this.addWatchFileduring thetransformhook.Resolves #3216
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).