expose `caches` to module user

[ahaurw01]

expose `caches` to module user

[ahaurw01]

Nice find!

Not knowing much about your workflow, I'm curious how it would get into a state where it captures a delete event on a file without having remembered that file. Is it possible the file object in the cache has a different processed path than the one on disk?

In any case this looks like a good opportunity for a patch. I'm not a big fan of exposing the implementation details of the caching mechanism, but I'm fine with the .forget call being a no-op (or even better logging a warning) when you try to forget a file that was never known.

Do you have time to submit a PR with this change and a test? If not i can probably do it over the thanksgiving break. Will a simple try-catch over your erroring call suffice until then?

[jcppman]

Cool, I can do the PR. And for now a try-catch is good enough as my temp workaround.

Which way do you prefer?

  // Add another if-else
  if (caches[cacheName] === undefined) {
    util.log('Warn: cacheName not found.');
  } else {
      delete caches[cacheName][path];
  }

  // Try-catch
  try {
    delete caches[cacheName][path];
  } catch (err) {
    if (err instanceof TypeError) {
      util.log('Warn: cacheName not found');
    } else {
      throw err;
    }
  }

FYI:

the reason why I might pass some 'not-exists' cacheName to remember.forget is that I have several gulp watchers, they watch different globs and trigger different tasks, some of them use cache with remember and some of them only use cache. So when a change event is emitted, sometimes the associated task use remember, sometimes not.

[ahaurw01]

Gotcha, thanks for the back story on your workflow. I think this is a nice opportunity to warn users who might be trying to debug incorrect or unexpected usages of .forget.

I prefer the first option. Also it's probably a good idea to prefix the warning with the name of this plugin per the gulp plugin guidelines.

Don't forget to write a test. Thanks!

[David-Hari]

I'm basically in the same boat.

Some of my tasks just use cache without a corresponding remember (because it's just a simple copy that does not require re-processing all previous files). I've written generic watch code similar to jcppman, and I would like to know whether or not a file in in the cache before trying to delete it.

I see the latest version does indeed log a warning when a file cannot be found in the cache. But I would prefer an API function to allow me to check before trying to remove it. Getting a warning when there is no problem would only cause to confuse people.

[David-Hari]

Even a simple deleteAll API function would be helpful, if I just decided to take the heavy-handed approach and blow away the whole cache whenever a file was deleted.

[ahaurw01]

@David-Hari I've added a couple PRs referenced above to help bridge the gap here. One for adding the forgetAll feature you alluded to (which I think might be helpful for other use cases as well) and one for getting a raw file cache by name.

I'm reluctant to make available the whole caching mechanism (the hash of hashes) as it would encourage misuse and expose implementation details.

Does this address your use case? I'd like to keep the API as slim as possible, so let me know if this fits the bill, or feel free to send another PR with a different approach.

@jcppman, please weigh in here if you're still interested in this topic.

[David-Hari]

I would also like a function to check if a file is in the cache, something like isInCache(name), but forgetAll will do just fine.

[ahaurw01]

@David-Hari You can achieve that by way of:

watcher.on('change', function (e) {
  if (e.type === 'deleted') {
    var cache = remember.cacheFor(cacheName);
    if (cache[e.path]) {
      remember.forget(cacheName, e.path);
    }
  }
}

While it's true I'm not a big fan of exposing the caches at all, this at least solves any other use case people might have that requires some knowledge of the contents of the cache. Only exposing remember.isInCache(name) might lead to a proliferation of a helper API down the road.

[jcppman]

@ahaurw01 I vote for the cacheFor method, although I'm not a big fan of exposing implementation details too. As you said, there are many different usages and gulp-remember can't implement all required caches manipulation methods. A cacheFor will efficiently solve this in a relative dirty way, and if users choose to use the caches directly, they should use it at their own risk.

BTW, how about merge the delete and deleteAll and have a new delete method that accepts stuff like node glob expression?

[ahaurw01]

@jcppman That's an interesting thought about merging the forget methods. I'll opt for keeping it simple now with the existing forget and the new forgetAll, but I'll keep this in mind for the future.

[jcppman]

Oops, forget, not delete. Nice fuzzyfind :)

[ahaurw01]

No worries 😸