Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cached responses [do not merge] #2465

Merged
merged 9 commits into from
Feb 25, 2015
Merged

Cached responses [do not merge] #2465

merged 9 commits into from
Feb 25, 2015

Conversation

javisantana
Copy link
Contributor

in order to improve the dashboard speed I added this cached sync. What it does is to store the data in local storage using it the next time you fetch it. It always fetch it but you get results faster.

I also prepared it to invalidate the cache when needed based on the namespace you give when create the sync method. It's the same system that subrogate keys use

In this case I'm using it i the visualization list in order to improve the dashboard and the table list speed

cc @CartoDB/frontend

This PR is to discuss this and find problems, before reviewing it, please read https://github.com/teambox/backbone.memoized_sync/blob/master/backbone.memoized_sync.js

@javisantana javisantana changed the title Cached responses Cached responses [do not merge] Feb 24, 2015
@viddo
Copy link
Contributor

viddo commented Feb 24, 2015

sounds good to me

The only things I can think of is that @xavijam added a wrapper for localStorage for the new dashboard (due to some browser issues I think?), perhaps we need to use the same here?

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

javisantana added a commit that referenced this pull request Feb 25, 2015
@javisantana javisantana merged commit 65af391 into master Feb 25, 2015
@javisantana javisantana deleted the cached_responses branch February 25, 2015 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants