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

Fix null and non submodules objects properties in state #40

Merged
merged 1 commit into from
Sep 8, 2019
Merged

Fix null and non submodules objects properties in state #40

merged 1 commit into from
Sep 8, 2019

Conversation

aymasse
Copy link
Contributor

@aymasse aymasse commented Sep 6, 2019

Hello,

I'm writing this PR to fix two issues I encountered with the current version (2.0.2) of this library.

The first is that the function isFieldASubModule in submodule.ts was trying to access a property from state properties initialized to null because in JS typeof null is strangely equal to 'object'.

The second one was a bit more tricky, as all object based state properties were all set to {}, even if they should have been Set or Map.

Feel free to tell me if this PR needs more work.

@tinou98
Copy link

tinou98 commented Sep 7, 2019

I faced the same second problem, but in the end I'm not sure that Set or Map are allowed in a Vuex state :

From https://vuex.vuejs.org/guide/state.html

The data you store in Vuex follows the same rules as the data in a Vue instance, ie the state object must be plain. See also: Vue#data.

From Vue#data

The data object for the Vue instance. Vue will recursively convert its properties into getter/setters to make it “reactive”. The object must be plain: native objects such as browser API objects and prototype properties are ignored. A rule of thumb is that data should just be data - it is not recommended to observe objects with their own stateful behavior.

The only case where this fix is required is for Array (for which I made a pull request #39)

@aymasse
Copy link
Contributor Author

aymasse commented Sep 7, 2019

Oh I did not know about this rule, although it makes sense now that I see it. I'll let @michaelolof decide if this rule should be strictly enforced or not.

@danielroe
Copy link

I think null is a legitimate value for a store state.

@michaelolof michaelolof merged commit 26c266e into michaelolof:master Sep 8, 2019
Glandos added a commit to Glandos/vuex-class-component that referenced this pull request Jun 29, 2020
This partly reverts michaelolof#40, and add a test for null object that can be set.
Now nested object can be set without explicit mutation, even in submodule.
Glandos added a commit to Glandos/vuex-class-component that referenced this pull request Mar 17, 2021
This partly reverts michaelolof#40, and add a test for null object that can be set.
Now nested object can be set without explicit mutation, even in submodule.
Glandos added a commit to Glandos/vuex-class-component that referenced this pull request Mar 18, 2021
This partly reverts michaelolof#40, and add a test for null object that can be set.
Now nested object can be set without explicit mutation, even in submodule.
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.

4 participants