I think this should be:

exports.BaseStore = BaseStore;
exports.MemoryStore = MemoryStore;

so that the BaseStore is exported and can be used.

@minrk, by doing this I also need to move BaseStore to another file. The change I made expects a class to be exported, not a module. I did it because we don't need to know the class name we are dynamically importing (RedisStore, PGStore etc). Take a look:

const loadStorage = (options) => {
  const BackendStorageClass = require(options.storageBackend || "./store.js");
  return new BackendStorageClass(options);
};

Also, I'm not sure it is a good an idea for a plugin to use BaseStore. We might have cyclic references. What do you think? IMHO, It's more like a duck-typing style than inheritance.

Duck typing is fine, but BaseStore exists for the purpose of being used in different implementations, potentially to reduce how much has to be duplicated. In particular, it implements some basic and second-order functionality that reduces what implementations must provide (e.g. providers don't strictly need to implement get, only getAll). It also gives us a place to add new second-order methods that providers would get for free.

If we aren't exporting BaseStore, there's no reason for BaseStore to exist and it can be subsumed into MemoryStore as a single class, rather than the sole subclass of an unusable base.

We can split the files if we really have to, but maybe it would be better to allow it to be an attribute rather than requiring all providers to use a dedicated module that only exports one class? Perhaps something like backend = './store.js:MemoryStore':

[modName, className] = backend.split(':');
backendMod = require(modName);
if (className) {
  backendClass = backendMod[className];
else {
  backendClass = backendMod;
}

@minrk since it is not a javascript pattern (I suppose), I don't think it is a good idea. Unless you want to extend your storage strategies inside the library, I vote for removing BaseStore. It's going to leave the code simpler.

I think the main issue here is that this is going to cause a major bump in semver when we could easily make this a minor bump adding the functionality. Keep the original keys on the exported interface and it will be alright.

@minrk @rgbkrk store.js exports only MemoryStore https://github.com/jupyterhub/configurable-http-proxy/blob/master/lib/store.js#L81, not BaseStore.

In order to not make a brake change, what I can do is: keep exports.MemoryStore and handle this import in loadStorage. It will keep the same contract but sounds weird, since plugins must export a class and our own module exports an object.

Ok, my only suggestion then is to keep the original MemoryStore export, we can defer the BaseStore to outside the scope of this PR to at least finish bringing in the great work you've already done.

I agree with you about the potential for cyclic issues and for why the base store would need to end up in a separate file. I also see the ease at which a default export would provide. For now, let's keep it backwards compatible while moving forward.

@rgbkrk @minrk I made the changes. Could you please check again?