Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Oct 20 2017 13:29
    appleton removed as member
  • Oct 11 2017 12:25
    SMJ93 closed #131
  • Oct 03 2017 21:05
    limaneto commented #143
  • Sep 20 2017 13:31
    giorgiobeggiora commented #140
  • Mar 28 2017 03:52
    giorgiobeggiora opened #144
  • Mar 11 2017 02:34
    blainsmith closed #106
  • Feb 24 2017 02:51
    hirasso commented #143
  • Feb 22 2017 21:40
    hirasso edited #143
  • Feb 22 2017 21:39
    hirasso opened #143
  • Sep 24 2016 02:39
    aliencyl7 closed #139
  • Jul 08 2016 21:21
    jclif commented #64
  • May 20 2016 14:27
    MaffooBristol commented #142
  • May 20 2016 14:22
    MaffooBristol opened #142
  • Apr 01 2016 02:37
    YaaMe opened #141
  • Feb 26 2016 12:55
    MaffooBristol commented #140
  • Feb 25 2016 20:52
    MaffooBristol commented #140
  • Feb 25 2016 20:33
    dyurkavets commented #140
  • Feb 25 2016 19:37
    MaffooBristol commented #140
  • Feb 25 2016 19:23
    dyurkavets commented #140
  • Feb 25 2016 18:18
    MaffooBristol opened #140
Beard of War
@blainsmith
regarding mixin issues: getCacheKey checks if the instance has its own version of getCacheKey then it calls that instead, but since the mixin is adding a ‘getCacheKey’ function the it ALWAYS has it on it so it runs into a loop
MixinModel = Backbone.Model.extend(
  _.extend({}, Backbone.fetchCache.mixins, {
    url: '/my-mixin-model'
  })
);
Backbone.fetchCache.mixins = {
    setCache: Backbone.fetchCache.setCache,
    clearItem: Backbone.fetchCache.clearItem,
    getCacheKey: function(opts) {
      return Backbone.fetchCache.getCacheKey(this, opts);
    },
    getLastSync: function(opts) {
      return Backbone.fetchCache.getLastSync(this, opts);
    }
  };
so with this the MixinModel will have a function getCacheKey set right on it
function getCacheKey(instance, opts) {
    var url;

    // If the model has its own, custom, cache key function, use it.
    if (_.isFunction(instance.getCacheKey)) {
      return instance.getCacheKey(opts);
    }
when it calls getCacheKey above it first checks to see if the instance has its own function, which it does from the mixin so it just calls itself…again…and again
if we change the mixin function name to something other than getCacheKey it works fine since the name is different
Beard of War
@blainsmith
maybe we standardize on a namespace convention for these mixin function like setFCItem, clearFCItem, getFCKey, etc.
and leave the functionality in place to define your own getCacheKey method as it already is…this keeps the names different and the mixins working properly
Kris Walker
@kixxauth
@blainsmith the other thing I was thinking of is namespacing them the way you originally had them. So, the mixin would only add a fetchCache property to each instance, with the above methods attached to it.
Also, the instance.getCacheKey method is not out in the wild yet, so you could still change the way that gets attached.
Beard of War
@blainsmith
cool, yeah i’ll have to play with this some more, running into a context issue after nesting them in fetchCache
Kris Walker
@kixxauth
Yeah, we might be pushing a rock up a hill here :cry: I mean, when you think about it, the model instances should be associated with a key, but not really the cache functionality, since there could be many instances associated with the same key.
Beard of War
@blainsmith
thats true, so maybe what we have now just passing instances to the cache functions is good enough
Beard of War
@blainsmith
i like that you can address issues right here like #110 for that PR
and that hover over! so nice
Lior Chen
@liorch88

I still think the mixin is a good idea. To avoid the context issue, maybe just avoid nesting it. Maybe the API for instance methods should be different then the public fetchCache API anyway - I mean, it does offer different functionality (always passing the instance).
It should be documented that using the mixins means that the methods will only work for the model/collection's instance, and they will assume the key is either the default or under a model's getCacheKey method. If they want something else - use the global methods, not the mixin's.
The API I am thinking of is something like this: prefixing all the methods by "cache" and only including some of the functionality - the functionality I think is needed for models.

Backbone.fetchCache.mixins = {
    cacheSet: function (opts) {
        return Backbone.fetchCache.setCache(this, opts);
    },
    cacheGet: function (opts) {
        return Backbone.fetchCache.getCache(this, opts);
    },
    cacheClear: function (opts) {
        return Backbone.fetchCache.clearItem(this, opts);
    },
    cacheLastSync: function (opts) {
        return Backbone.fetchCache.getLastSync(this, opts);
    }
};

Notice that I didn't include getCacheKey, as I think it will only confuse, and not needed in model context.
What do you think?

Beard of War
@blainsmith
yeah that made me re-read @kixxauth’s response about multiple instances associated to the same key…even if that is the case it shouldnt matter since it was up the user to define a unique key name
Beard of War
@blainsmith
@liorch88 @kixxauth ive updated #106
Lior Chen
@liorch88
Cool
I see that in the updated PR you've added a call to getCacheKey before each method call to create the key. But why is this needed? I mean, all the methods call this method internally anyway. Am I missing something?
Beard of War
@blainsmith
only on a few methods that require the key to do its job
the internal get and clear depend on a key, not an instance
Lior Chen
@liorch88

Yeah, but to generate the key I see you pass the getCacheKey method with the this - which is precisely what they do internally, if you pass them an object:

function getCache(key, opts) {
    if (_.isFunction(key)) {
      key = key();
    } else if (key && _.isObject(key)) {
      key = Backbone.fetchCache.getCacheKey(key, opts);
    }
...
  }

  function clearItem(key, opts) {
    if (_.isFunction(key)) {
      key = key();
    } else if (key && _.isObject(key)) {
      key = Backbone.fetchCache.getCacheKey(key, opts);
    }
...
  }

Same goes for getLastSync, as it calls getCache internatlly.

Jaume Tarradas Llort
@bichotll
Just a comment...I think it would be better if it does not rewrite the class. I d prefer to extend a new class like Backbone.CacheCollection.
Great job any way!
Jared Clifton
@jclif
Saw that the changelog hadn't been updated for the newer versions, and was wondering whats been going on up to 1.5.5