Skip to content

Enable "state" as a valid key in getters #452

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

Merged
merged 2 commits into from
Nov 10, 2016
Merged

Conversation

xiaoyanhao
Copy link
Contributor

Issue

When I declare a state key in getters as follow, there is an error:

const store = new Vuex.Store({
  state: {
    count: 0
  },
  getters: {
    state: state => state.count > 0 ? 'hasAny' : 'none'
  }
})

Uncaught RangeError: Maximum call stack size exceeded(…)

Check out the simple demo and the error will be logged in the console.

Debug

Finally I found there will be a circular call if declaring state key in getters:

  1. Getters is treated as computed properties and state as data properties (under a state key) in Vuex.Store._vm. However, in vue, if there is a same key both in computed and data, we will always get the computed property using proxied properties of the vue instance.

    let vm = new Vue({
      data: {
        count: 0,
        state: 'I will be overwritten'
      },
      computed: {
        state: function () {
          return this.count > 0 ? 'hasAny' : 'none'
        }
      }
    })
    
    vm.state // 'none'
  2. When getting store state, the state in getters is returned instead of the single state tree based on step 1.

    class Store {
      get state () {
        return this._vm.state
      }
    }
  3. Invoke the state getter function which will access store.state at first. So it jumps to step 2 and leads to a endless call stack.

    store._wrappedGetters[getterKey] = function wrappedGetter (store) {
      return rawGetter(
        getNestedState(store.state, modulePath), // local state
        store.getters, // getters
        store.state // root state
      )
    }

Fix

Access the store state through instance properties (prefixed with $) instead of proxied properties in order to avoid overlapping between data and computed properties in vue instance.

class Store {
  get state () {
    // return this._vm.state
    return this._vm.$data.state // also reactive
  }
}

Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

LGTM

@yyx990803 yyx990803 merged commit 7141fb0 into vuejs:dev Nov 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants