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

localStorage-based data property get/set issue when using a mixin #10881

Closed
185driver opened this issue Dec 1, 2019 · 4 comments
Closed

localStorage-based data property get/set issue when using a mixin #10881

185driver opened this issue Dec 1, 2019 · 4 comments

Comments

@185driver
Copy link

@185driver 185driver commented Dec 1, 2019

Version

2.6.10

Reproduction link

https://codepen.io/185driver/pen/xxbKRLg

Steps to reproduce

  1. Toggle the top button several times.
  2. Note that the "Actual localStorage value” line in both the top and bottom sections toggles in response to localStorage.is_dark switching between true and false. Alternatively, open the browser dev tools and directly observe localStorage.is_dark changing.
  3. Note that the “Vue isDark prop value” line also toggles, but this is simply the data option isDark property itself toggling.
  4. Toggle the bottom button several times.
  5. Note that the "Actual localStorage value” line in neither the top nor bottom sections toggles in response, though both should.
  6. Note that the “Vue isDarkMixin prop value” line does toggle, as this is simply the data option isDarkMixin property itself toggling.

What is expected?

Whether placed in a component, a mixin, or the root of the Vue instance, the functionality of the isDark and isDarkMixin properties should be the same in terms of toggling localStorage.is_dark, but it isn't. It works in a component or in the root, but doesn't work in a mixin.

What is actually happening?

Identical getter/setter data property code placed in a mixin does not have the same effect on a localStorage variable as it does elsewhere. It actually has no effect at all.


I can't help but wonder why mixin behavior in this use case differs from component behavior. Other than this, however, I'm impressed it is possible to simulate reactivity on a localStorage variable this way.

Other code I've seen doing the same basic thing:
https://stackoverflow.com/questions/42974170/is-there-any-way-to-watch-for-localstorage-in-vuejs/43474950#43474950

@jeremy21212121

This comment has been minimized.

Copy link

@jeremy21212121 jeremy21212121 commented Dec 1, 2019

There are a few problems with your code. Polling localStorage 10 times per second is not an ideal approach for performance or application architecture.

Instead of using localStorage as your source of truth, instead use data (or vuex) and sync those changes with localStorage.

For example, your click handler would set the isDark property on the data object and also store this value in localStorage. Use the mounted or created hooks to load this value from localStorage into component data. This will persist the value across reloads.

I made an example. I was having trouble getting localStorage to work with codepen so I hosted it here.

new Vue({
  el: '#app',
  data() {
	return {
	  isDark: false
	}
  },
  methods: {
	clickHandler() {
	  this.isDark = !this.isDark;
	  localStorage.setItem('isDark', this.isDark)
	}
  },
  created () {
	const lsDark = localStorage.getItem('isDark')
	if (lsDark) {
	  this.isDark = lsDark === 'true'
	}
  }
});

You should also consider using feature detection before calling localStorage methods. It isn't always available. See this SO for details.

Hopefully this helps.

@185driver

This comment has been minimized.

Copy link
Author

@185driver 185driver commented Dec 1, 2019

@jeremy21212121 Thank you. I appreciate your thoughts. A few things:

  1. I agree with you re: the setTimeout, but I'm only using it for display purposes in my issue repro. That code doesn't exist in my project code.
  2. I have a use case that requires the isDark property to populate from localStorage on app start as illustrated in my issue repro. My reasons are my own.
  3. You've helpfully suggested a different way to populate a localStorage variable, but that's not the issue I submitted. Rather, I would like to understand the expected vs actual behavior I've observed, given it is problematic when using a mixin. It may be a bug.
@jeremy21212121

This comment has been minimized.

Copy link

@jeremy21212121 jeremy21212121 commented Dec 1, 2019

Your mixin setter never runs. This is probably because Vue uses getters/setters under the hood, this is how reactivity works. So your code happens to work as a data property but not as a mixin.

Computed properties support a get/set syntax. You could give that try for your use case. Or you could follow the docs for client-side storage and not paddle upstream.

Either way, it's not a bug with Vue.

@posva

This comment has been minimized.

Copy link
Member

@posva posva commented Dec 1, 2019

Hi, thanks for your interest but Github issues are for bug reports and feature requests only. You can ask questions on the forum, the Discord server or StackOverflow.


Since Vue relies on getters and setters, it's not a good idea to define yours in data. In your case your data isn't reactive anyway, so you could define it externally as an object:

const mixin = {
  created () {
    Object.defineProperty(this, 'isDarkMixin', {
    get  () {
      console.log('read isDarkMixin')
      return localStorage.getItem('is_dark') === 'true' || false;
    },
    set  (newVal) {
            console.log('write isDarkMixin')
      return localStorage.setItem('is_dark', newVal);
    },
    })
  }
}
@posva posva closed this Dec 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.