Local Forage support - Githubissues

funkensturm / ember-local-storage

The addon provides a storageFor computed property that returns a proxy and persists the changes to localStorage or sessionStorage. It ships with an ember-data adapter.

https://www.funkensturm.com/ember-local-storage/

MIT License

218 stars 76 forks source link

Local Forage support #278

Open Mogball opened 6 years ago

Mogball commented 6 years ago

https://github.com/funkensturm/ember-local-storage/pull/155 https://github.com/funkensturm/ember-local-storage/issues/153

This is my attempt to add localforage into ember-local-storage.

A few things to note:

localforage is an async-only API, which means that the API for ember-local-storage has to be async (otherwise one risks dealing with race conditions)
localforage lacks a storage event as mentioned in #153; there is a localForage-observable package that functions across tabs and essentially does the same as the storage event, but this package is not available on bower because one of its dependencies is not

Those things considered, it might do better as a fork. On the other hand, commit https://github.com/funkensturm/ember-local-storage/commit/548b36146225091a0914290e492f313c6ff0b683 seems to function okay without the async API (testing in an actual ember app) but a few test cases weren't dealing with race conditions well.

fsmanuel commented 6 years ago

@Mogball wow. Thanks. I'll review as soon as possible!

akshaisarma commented 6 years ago

@fsmanuel, I would love to have this change merged in. @Mogball actually contributed this code for my project's use case mentioned here. Right now we're using @Mogball's fork directly but would prefer to use the official ember-local-storage plugin. If there's anything I can do to help in this review, please let me know.

fsmanuel commented 6 years ago

@Mogball @akshaisarma I didn't had time to review yet. The main problem is that it changes the api to async. That would break every app that uses the addon. At least that would require a manjor release. My goal is to have localForage as an option that can be configured. Something like:

// config/environment.js
module.exports = function() {
  var ENV = {
    'ember-local-storage': {
      useLocalForage: true
    }
  }
};

That way we can add a section to the readme that explains how the async api works if localForage is enabled.

To make that possible we need some wrapper fuctions that abstracts the sync ans async behavior. At a first glance @Mogball implementation looks good and we can use all that code for the async/localForage use.

I'll ping you as soon as I have done the review.