Vuex: No warning/error is given when setting state of a non-existent state property name

[SergioCrisostomo]

Version

3.0.1

Reproduction link

https://jsfiddle.net/Sergio_fiddle/m99jmqsa/

Steps to reproduce

Open the jsFiddle

What is expected?

Two possible cases:

a) Vuex would create a property in state and the getter would be loaded with the data

b) a warning/error would be thrown stating "you are setting data to a non-existent property"

What is actually happening?

Vuex silently fails

[LinusBorg]

Technically, there's no way to implement such a warning. We can't listen to arbitrary getters of unknown properties, so we can't detect that operation and warn about it.

[SergioCrisostomo]

We could use Proxies for that. I think that Vuex not complaining is a unexpected behavior, and a warning/error is expected/useful for the developer.

Something like (jsFiddle):

const state = {
  foo: 10
};

const STATE = new Proxy(state, {
  set(target, key, value) {
      if (!target.hasOwnProperty(key)) {
        console.error(`Please declare the "${key}" property name before using it`);
      }
      target[key] = value;
  },
  get(target, key) {
      return target[key];
  }
});

STATE['non_existing_property'] = 'bar'; // this throws a error
STATE.foo = 34

console.log(STATE['non_existing_property'], STATE.foo);

[LinusBorg]

Yes, Proxies can solve that, but Proxies can't be completely polyfilled for older browsers, so that's out of the question for the 2.x branch of Vue, which promises compatibility down to IE9.

We are planning for a 2.x-next which will switch the complete reactivity-system to proxies and will be developed in parallel to 2.x, so for that branch, such warnings will be possible.

You can use a Proxy for that in Vuex today, if you don't care about IE support.

[SergioCrisostomo]

Sounds good with a 2.x-next branch. If possible to add this would be great. Just "silently failing" feels like Vuex has loose control of the state object, and took us a long time to track down. And if the extra check is "too expensive" maybe that logic could be behind a debug flag.

[ktsn]

As this is only needed in development phase we can use Proxy if it is supported in the browser and I think we can ship it in the current branch. The Vue core is actually using it for renderProxy. https://github.com/vuejs/vue/blob/dev/src/core/instance/proxy.js#L73

[shinrox]

This would be really helpful in preventing unexpected behavior like this

it could be added only when strict mode is true

[kiaking]

I think this is possible by proxy, though I'm nosier we want to do this. We had some proxy comparing to raw object problem in Vue 3, and I don't think we want extra false negative on Vuex. It's going to be worth because this time it have different behavior in Dev and Prod build.

I'm keen to hear more feedback on how Proxy might affect any edge cases.

[kiaking]

Closing due to inactivity for now.

[SergioCrisostomo]

Was it from me you wanted more feedback? If so I misunderstood.
I still think this is important, the concept of control over the store values gets lost when you can "sneak in" properties by accident or intentionally and they behave in other ways that declared properties. The solution I added in #1605 is only for Dev, so I think its a "safe" adition.

When you mention _"We had some proxy comparing to raw object problem in Vue 3"_ what problems where those?

[kiaking]

When you mention "We had some proxy comparing to raw object problem in Vue 3" what problems where those?

There was an issue related to proxy not being the same object as the original object. Comparing both would fail and such.

That might not be the case for this implementation. However, having different objects being handled depending on the environment is something we should avoid as much as possible.

Sorry for closing this sudden, let's see if can get a bit more latest feedback on this.

@ktsn any thoughts?

[SergioCrisostomo]

No problem. In the solution I suggested $$state does get replaced by a Proxy in dev build (ie process.env.NODE_ENV !== 'production').
Would be nice to see the test that tracks that specific problem you mentioned, I could test my solution on it and make another suggestion if needed.

[ktsn]

I'm against to proxy the state object as there is the problem @kiaking states. Also there is a valid case to add a new property to state.
Sorry I thought this issue was for getters object to warn for non-existing getters.