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

Add async storage for saving/getting large data [DO NOT MERGE] #4418

Conversation

Semigradsky
Copy link

Fix #3239

var isLocked = lock.locked();

if (isLocked) {
return Promise.resolve(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure native Promises don't work in IE11.. Can you use localForage's callback API instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my fault. Rewrited to callback API.

@Semigradsky
Copy link
Author

@bhousel ping

@@ -431,7 +455,7 @@ export function coreContext() {

// Debounce save, since it's a synchronous localStorage write,
// and history changes can happen frequently (e.g. when dragging).
context.debouncedSave = _debounce(context.save, 350);
context.debouncedSave = _debounce(context.save, 100);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you changed this? (If anything I think we could slow down the saves, not speed them up)

@bhousel
Copy link
Member

bhousel commented Oct 30, 2017

Thanks for being patient @Semigradsky - I think I'm going to move this new code into its own module, and also try to update the other places in iD that use storage to use this new store, and add some code to grab the existing localStorage values if they exist. Hope to merge it today 👍

@bhousel
Copy link
Member

bhousel commented Oct 31, 2017

Hey @Semigradsky - I did some work on this yesterday but here's where I ended up:

  • The switch from sync to async storage is a pretty big deal. We can do it, (and there are real benefits) but it changes the flow of when stored variables can be available to the iD code
  • IndexedDB is actually kind of slow. I'm reading that get times around like 100-300ms are normal for largish objects.
  • Some components in iD need access to the stored variables at startup (think like the splash screen and the history restore) - however we don't have an ability to delay startup until the stored variables are ready - this would be an API breaking change..
  • So considering the current code and that it would be a breaking change, I want to put the pause on this PR until we start work on iD v3.

I don't have a timeline on when it would happen, but it is something we want eventually..

@bhousel bhousel added the wip Work in progress label Oct 31, 2017
@bhousel bhousel changed the title Add async storage for saving/getting large data Add async storage for saving/getting large data [DO NOT MERGE] Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wip Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants