rfink / sequelize-redis-cache

Small fluent interface for caching sequelize database query results in redis more easily
MIT License
174 stars 47 forks source link

New instance of cacher overrides other #28

Closed Naoto-Ida closed 7 years ago

Naoto-Ida commented 7 years ago

Thanks for the awesome lib. We're using it for our GraphQL API with Sequelize and Redis.

I have a problem when using more than one instance of the cacher like this:

import initCacher from 'sequelize-redis-cache'
import redisClient from './somewhere'

const cacherForDb1 = initCacher(sequelizeDb1, redisClient)
const cacherForDb2 = initCacher(sequelizeDb2, redisClient)
const cacheObjForDb1 = cacherForDb1('foo').ttl(300)
const cacheObjForDb2 = cacherForDb2('bar').ttl(300)

Then I get the error:

Error: foo has not been defined from sequelize/lib/sequelize.js:643:11.

I'm guessing that it is because this is a shared instance? How can I work around this?

rfink commented 7 years ago

Hello and thanks for using my lib! This was the original design of the project, to have a single global instance. However, my plan has been to update that, since turning this into a singleton (if desired) should be the choice and responsibility of the end user. I'm going to add this into a branch, and I'd be happy to update this thread when I have something implemented and tested.

Naoto-Ida commented 7 years ago

Thank you so much. I can also take a look at it too If you want. I'm guessing this would be a breaking change?

rfink commented 7 years ago

Yes, I believe it would be breaking. I have another branch where I'm making some updates to the API to accommodate for some new features (such as caching raw queries), maybe it makes sense to go ahead and merge that change. The branch is "feature/new_api", if you are interested in taking a look.

Naoto-Ida commented 7 years ago

I checked it out for a few minutes. Seems very good. I was waiting for the raw query too, in addition to non-singleton. When do you think this can be merged?

rfink commented 7 years ago

I don't think it would be a problem to have them both in this week.

Naoto-Ida commented 7 years ago

Great, looking forward to it 😃

rfink commented 7 years ago

Okay, all of my proposed changes are in the feature/new_api branch. Tests are passing, module looks like it's working as expected. Want to give it a peek before I tag and release?

Naoto-Ida commented 7 years ago

Sure, let me check it out.

Naoto-Ida commented 7 years ago

Seems great!

rfink commented 7 years ago

Great. This is tagged and released as 2.0.0 (and it's published to npm as such). Thanks very much for your feedback.