-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Should the state be merged at the same level as actions and getters? #71
Comments
hey, thanks for opening the discussion. Initially I only had
Totally :)
I don't think it really matters as long as there is an explicit error for the user
I was thinking of making the whole store a Personally, I like the idea of grouping everything into the store without having state, getters or actions |
Would it make sense to use
It would also help clearing up confusing while working on components "When to use .value and when not to". Currently, the indicator to use
|
No, that's not desirable. Having not to use
To be fair it's the same as |
With
implemented, the difference between an action and a getter wouldn't be clear anymore:
Having them all scoped would solve this, and would also mean it "follows" the structure of the createStore config parameter:
When / if the getters-with-params functionality is implemented, would Pinia still have "getters without parameters" too? Eg would |
The getters with parameters is different from a function, it's still an idea and no proposal has been made. Otherwise yeah, it would just be an action |
Hi, this is a great project, I love it! Something I yearned for a long time now. Little backstory to understand where I'm coming from: What brought me to Vue was the sweet, simple style of handling state. <div id="app-5">
<p>{{ message }}</p>
<button v-on:click="reverseMessage">Reverse Message</button>
</div>
<script>
var app5 = new Vue({
el: '#app-5',
data: {
message: 'Hello Vue.js!'
},
methods: {
reverseMessage: function () {
this.message = this.message.split('').reverse().join('')
}
}
})
</script> Finally a framework using a immutable state as source of truth and then build something from that in a functional fashion with But then Vuex gets recommend a lot and vuex kind of gets in the way of things with their overly boilerplate With this in mind I petition to also changes Pinias' interface to have Furthermore maybe if the suggestion of getting rid of the I mean something like this sketch // ** are for empathizing
export const useMainStore = createStore({
// a function that returns a fresh state
state: () => ({
counter: 0,
**get double()** {
return doubleCount.value; // either this
return counter*2// or maybe don't even need computed
},
}),
// optional getters
getters: {
doubleCount: **computed**(() => state.counter * 2),
},
// optional actions
actions: {
setCounter(val:number) {
this.state.counter = val;
},
},
}) Now it should work without import { useMainStore } from '@/stores/main'
export default defineComponent({
setup() {
const {state:main} = useMainStore()
console.log(store.counter); // 0
main.counter = 2 // TS-Error , because its Readonly<T>
main.setCounter(2); // alrighty
console.log(main.counter) // 2
console.log(main.double) // 4, I guess 🤔
return {
// gives access to the `counter` and `double`
...toRefs(main) // not sure about `toRefs()`
// gives access to the whole store
store: useMainStore(),
}
},
}) Also I think there would be great advantages in picking up suggestions from #58 to use Typescripts Readonly utility type to indicate state shouldn't be modified from outside, as I tried to show here as well. Love to hear your thoughts about this! |
The overall direction was exposing everything at the same level. See #190 You won't be able to do |
I don't think that's a problem, as it's the same with Would that mean that—even though it might be a bit of a weird use case—you'd be able to do something like: const { counter } = toRefs(useMainStore()); ? |
Yes, since to unwrap we would internally use |
Awesome, very promising! // gives access only to the state
state: computed(() => main.state), Lovely 😍 I also think people will get used to Does this mean, after this changes, there won't be a |
@posva looks sweet, will have to check it out over the next few days! |
ah, but now you can change the state directly, without needing a action right? I feel like this is invalidating the use case for a store, namely the one-direction-flow of data. In my opinion if state can be changed directly, quickly there will be spaghetti code and a lot of coupling and major headaches when debugging. :( |
See #58 |
In the readme, you mention
I figured we should open a discussion around this 🙂
My 2 cents:
I think we should either scope all things into relevant properties, eg:
or scope none
Having them all merged in the "root" store might cause naming conflicts between getters and state. However, you could make the argument that having a getter with the exact same name as a property in the state is a fairly bad design decision to begin with. In the case of a naming conflict, the state one should probably overwrite the getter, as getters will often rely on the state value to produce their own value.
A big pro for having them all in the root is that you would be able to do things like
where you can destructure any of the 'items' in the store.
Is there any specific reason why you decided on putting actions / getters in the root currently @posva?
The text was updated successfully, but these errors were encountered: