devvmh/redux-crud-store

Cache period should be configurable

alexhung opened this issue · 9 comments

devvmh commented

Thanks, this is a great issue to have on the books.

A pull request would be welcome. In the meantime, I'll consider prioritizing this after i finish removing immutable js. Currently I'm trying to build a demo app that can fully test that big changes aren't breaking, in anticipation of merging the pr that removes immutable. After that I'll look at this. It may be a while, though, so please send a pr if you have ideas.

  • 👍1
devvmh commented

#49 is merged to master. I'll probably release it as 5.4.0 very soon and wait for bug reports; once I'm confident it's not breaking people I'll start working on this issue.

In the meantime, I'm going to take an initial look at this issue now.

  • 👍1
devvmh commented

Well, I have a start on a branch at https://github.com/devvmh/redux-crud-store/compare/feature/configurable-cache-period

I'm not sure the best way to configure this. I could use a setter, and replace cachePeriod and halfCachePeriod with functions rather than constants. I'm not sure if that's more or less elegant than using environment variables.

The setter is probably easier to use for people who don't know lots about environment variables.

Do you have suggestions/preferences?

alexhung commented

Ideally, I'd like it to be overwritable for action creators, with fallback to either constant or env var (I prefer env var). Having said that, exposing the caching period as env var will be very helpful already.

My project use Fb's create-react-app so if you decide to go down the env var, consideration needs to be taken for how create-react-app uses env var (i.e. all env var needs to be prefixed with REACT_APP_).

devvmh commented

OK, when I next get some time to work on this, I'll look into:

  1. Using process.env.CACHE_PERIOD and process.env.REACT_APP_CACHE_PERIOD to override the default cache period
  2. Passing the values through action creators -> actions -> reducers to override that way

I thought that list would be longer but it should be good.

Again, I won't merge/push this until I've released 5.4.0 with no immutable and given people a chance to try it out

julienR2 commented

hi @devvmh !
I'm back on my react native project with which I'll be using redux-crud-store too !

I'll hopefully be a bit more active and I'll definitely give a try to the no immutable version (also because I'm curious to know if it resolve a problem I had with redux-persist)

Cache period would be a nice add too 👍

devvmh commented

OK, so I've added support on the branch. Code review comments welcome.

The way things are currently architected, you can't use action creators to specify a cache period because the only actions that actually do anything with the cache period are GARBAGE_COLLECT actions, which are dispatched by a saga.

For now, I've added the ability to dispatch your own GARBAGE_COLLECT action if env vars aren't doing it for you. But realistically this doesn't seem like a common use case, or even that useful. If you really wanted to clean up the cache without env vars, I'd recommend just writing your own reducer that modifies the state.models key.

Sample GARBAGE_COLLECT action:

{
  type: GARBAGE_COLLECT,
  meta: {
    now: Date.now(),
    cachePeriod: 1 * 60 * 1000 // one minute
  }
}
devvmh commented

See #59 for PR

devvmh commented

in version 5.5.0