search

SaneMethod / jquery-ajax-localstorage-cache

Ajax Cache backed by localStorage for jQuery
Other
379 stars 79 forks source link

set timestamp safely on response #11

Closed treasonx closed 8 years ago

treasonx commented 12 years ago

www.gruntjs.com is running into a problem right now in chrome where when trying to set the TTL its throwing an exception due to localStorage being full. It looks like the timestamp should be set in the try catch when the response comes back.

paulirish commented 12 years ago

Hmmm... i can understand try/catching it but dropping the conditional around that seems like an odd choice.

treasonx commented 12 years ago

The conditional for setting the ttl seemed a bit odd to me given what the script is trying to accomplish.

1) Why do we set the ttl outside the success callback? Shouldn't the ttl be set when the data is stored on success? What if the request fails? Do we really want to set a ttl in a failure case? If we did there would be a ttl with no associated payload, which wastes space.

2) The only way we get into the block where the conditional was, is if we have no data and we need to retrieve it. The conditional was redundant. The only way we would get into the block where the conditional was is it we had no data. The only way we had no data was if the ttl was null or we had just expired it.