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

Creating new nested key #262

Open
AshMartian opened this issue Jun 27, 2017 · 2 comments
Open

Creating new nested key #262

AshMartian opened this issue Jun 27, 2017 · 2 comments

Comments

@AshMartian
Copy link

I have a settings local-storage model with nested attributes. This is working great, but when trying to add more attributes I'm confused as to why the ember-local-storage plugin isn't simply creating the new key for the missing nested attribute. For example:

My settings when project was first launched in the browser:

lastWifi: false,
      lastConnected: null,
      settings: {
        language: 'en',
        about: {
          version: "0.5"
        }
}

This created a localStorage object, works like a charm. However in developing the app, I need to add more nested keys.

lastWifi: false,
      lastConnected: null,
      settings: {
        language: 'en',
        about: {
          version: "0.5"
        },
        bluetooth: {
            allow: true
        }

When trying to access the bluetooth and it's allow key, I'm getting this error:

ember.debug.js:19818 Property set failed: object in path "settings.bluetooth" could not be found or was destroyed.
Error
    at setPath (http://localhost:4200/assets/vendor.js:35584:15)
    at Object.set (http://localhost:4200/assets/vendor.js:35504:14)
    at Class.set (http://localhost:4200/assets/vendor.js:48371:26)
    at Class.saveIfChanged (http://localhost:4200/assets/vendor.js:74170:17)
    at Class.superWrapper [as set] (http://localhost:4200/assets/vendor.js:54095:22)
    at Class.toggleSetting (http://localhost:4200/assets/www.js:1207:30)
    at Backburner.join (http://localhost:4200/assets/vendor.js:13883:21)
    at Function.run.join (http://localhost:4200/assets/vendor.js:35754:28)
    at http://localhost:4200/assets/vendor.js:23468:37
    at Object.flaggedInstrument (http://localhost:4200/assets/vendor.js:31850:14)

I understand I could use the .isInitialContent() .clear() or .reset() functions, however I would then have to do this for every update I deploy or on every new key I add? Seems somewhat counter productive and could lead to a bad user experience.

I've now added my local-storage to a model, as recommended for nested attributes. However the behavior does not change.

@fsmanuel
Copy link
Member

fsmanuel commented Jun 29, 2017

@blandman Thanks for the feedback. I get your point. The problem is that the addon only sets the initial state if there is no data for that key. It seem that your problem is a common scenario for nested objects. I see different approaches how we can handle it.

First of all you should be able to solve it like that:

// Setup the new key
if (!this.get('settings.bluetooth')) {
  this.set('settings.bluetooth', {});
}

this.set('settings.bluetooth.allow', true);

We could now add that functionality in some way or another to the addon but I'm not sure yet how to not impact the performance for all storage objects. I think one way might be an option on the storageFor to tell it that there needs to be some migration work to be done (storageFor('settings', { migrate: true })). It should just check for new keys in initialState and add that defaults.

Another way would be to add a new method setWithDefault('settings.bluetooth.allow', true, {}) that would do the setup for the new key (like in the code example above).

What do you think is the best way? Do you have a better idea?

@AshMartian
Copy link
Author

Thank you so much for this! I could test performance, the app I'm working on has a lot of realtime data and ideally has many user configurable settings and running on mobile. Having migrate as a storageFor option is brilliant. The basic functionality would go a long way.

I could quickly test a few options 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants