cache.update & cache.forget do not remove deleted files from the cache.

[agarzola]

First off, this package has reduced our concatenated JS file build by an order of magnitude. Thank you for publishing it! I think I discovered an issue with it.

Our watch task, I added something like this (which I lifted almost verbatim from this archived blog post):

gulp.watch('src/**/*.js', scriptsFunction)
  .on('change', cache.update('js'));

This, however, did not work. When I deleted a file from the filesystem, it simply did not get removed from the cache. I could still use the features on my app that were powered by the code in the file I had deleted. I did a little digging and found a few of issues:

1. Chokidar (the library Gulp’s watch method uses under the hood), does not fire the change event when a file is deleted. Instead it fires the unlink event.
2. gulp-memory-cache’s update and forget methods invoke’s the cache’s remove method, which simply removes the entry for that file from its internal cache object, referencing it by the file path. However, the cache object uses an absolute path as the reference key for a given file, while the event.path supplied by Chokidar is relative to the CWD. In other words, simply supplying event.path doesn’t work because src/js/my-file.js doesn’t match /Users/username/path/to/project/src/js/my-file.js. Additionally, the callback for this event does not receive an Event object, but rather two parameters: filePath and stats.

Armed with this information, I was able to get files to be correctly deleted from the cache using the following:

// Earlier in the file, import the `path` module:
const path = require('path');

// Then, in our `watch` call:
gulp.watch('src/**/*.js', scriptsFunction)
  .on('unlink', (filePath) => {
    cache.forget('js', path.resolve(filePath));
  });

This reliably removes files from the cache and our concatenated JS no longer includes files that were deleted from the file system. 🎉

I propose the following updates to this library:

1. Either remove or modify the update method:

• Remove the update method, since its only purpose (as far as I can tell) is to remove files when a change event is fired.
• Modify the update method not to check event.type, but instead presume it is being passed as the callback to the unlink event and expect the parameters passed in by Chokidar for the unlink event (filePath and stats).

1. Update the forget method to resolve the supplied path into an absolute path, since we know that files stored in the cache are keyed by their absolute paths.
2. Update the documentation on how to effectively remove files from the cache on watch.

I’m happy to submit a PR to tackle these if contributions are welcome.

[agarzola]

@troch Bumping this issue.