fix: update package cache watcher #12772
Conversation
bluwy
added
the
p3-minor-bug
|
|
| packageCache: PackageCache, | ||
| pkgPath: string, | ||
| ): void { | ||
| packageCache.delete(pkgPath) |
There was a problem hiding this comment.
This isn't really working now because cache keys have rpd_ and fnpd_ prefixes. The below forEach loop should cover this instead.
| if (!isInNodeModules(pkg.dir) && !watchedDirs.has(pkg.dir)) { | ||
| watchedDirs.add(pkg.dir) | ||
| watchFile(path.join(pkg.dir, 'package.json')) |
There was a problem hiding this comment.
Make sure to only watch package.json in node_modules, also uses a watchedDirs set to cache as this .set call can be blasted with many same paths. In dev, watchFile would invoke a fs.existSync call otherwise.
| watchChange(id) { | ||
| if (id.endsWith('/package.json')) { | ||
| invalidatePackageData(packageCache, id) | ||
| invalidatePackageData(packageCache, path.normalize(id)) | ||
| } | ||
| }, | ||
| handleHotUpdate({ file }) { | ||
| if (file.endsWith('/package.json')) { | ||
| invalidatePackageData(packageCache, path.normalize(file)) | ||
| } |
There was a problem hiding this comment.
This plugin used to be build only, now it also handles dev by using handleHotUpdate
| const { packageCache } = config | ||
| const setPackageData = packageCache.set.bind(packageCache) | ||
| packageCache.set = (id, pkg) => { | ||
| if (id.endsWith('.json')) { | ||
| watcher.add(id) | ||
| } | ||
| return setPackageData(id, pkg) | ||
| } |
There was a problem hiding this comment.
Removed as handled in handleHotUpdate now.
| if (file.endsWith('/package.json')) { | ||
| return invalidatePackageData(packageCache, file) | ||
| } |
There was a problem hiding this comment.
This return call was blocking HMR updates for package.json, that also blocks handleHotUpdate, so removed here.
Description
The watcher is incorrect after the recent refactors, I've made some fixes below which should preserve the old behaviour, except that it would also only watch
package.jsonnot insidenode_modulesnow. Before that, we did and I think it's taxing to do so.Additional context
I'll leave comments below to explain the changes. I've also manually tested this locally.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).