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

Is this addon expected to work with Ember Octane? #325

Closed jking6884 closed 2 years ago

jking6884 commented 3 years ago

I am seeing an issue with getting this addon to work with Ember 3.26, but it was previously working in 3.11.

fsmanuel commented 3 years ago

@jking6884 It's supposed to work. I'm using it with 3.24.0. What are the issues?

jking6884 commented 3 years ago

There seems to be a couple of problems, I think.

image

This computed that is returned from storageFor never seems to fire the way it is set up. I was thinking this was potentially because of the fact that the get/set functionality is not the same in the native classes in Octane. At any rate, it doesn't seem to fire ever, so the storage create method isn't called either. However, I also found that if I tweaked the way that the logic worked (I create an in-repo-addon with ember-local-storge and changed things to test), the create method was also not getting the proper value of this. So I had to pass in the context from the controller when calling storageFor.

The computed property also doesn't seem to be firing when setting values.

image

image

fsmanuel commented 3 years ago

You can use it like that:

import { storageFor } from "ember-local-storage";

export default class extends Controller {
  @storageFor('role') roleStorage;

  doSomething() {
    this.roleStorage.get('someKey');
  }
}
jking6884 commented 3 years ago

@fsmanuel I have confirmed that it is indeed working when using it as a decorator. Thanks for that clarification.

I think it would be a great idea to document this in the readme as it is currently not very clear how this addon should be used in the context of native classes with Octane.

fsmanuel commented 3 years ago

@jking6884 good point. Will add documentation for native classes in the next days.