From 7bb7733ff85c6a908c9090da2762186d7afefac5 Mon Sep 17 00:00:00 2001
From: Eduardo San Martin Morote <posva@users.noreply.github.com>
Date: Tue, 22 Sep 2020 10:08:12 +0200
Subject: [PATCH] feat: access the state and getters through `this` (#190)

BREAKING CHANGE: there is no longer a `state` property on the store, you need to directly access it. `getters` no longer receive parameters, directly call `this.myState` to read state and other getters
---
 README.md                     | 63 ++++++++++++++++++++------------
 __tests__/actions.spec.ts     | 15 ++++++--
 __tests__/getters.spec.ts     | 43 +++++++++++++---------
 __tests__/rootState.spec.ts   |  2 +-
 __tests__/state.spec.ts       | 31 ++++++++++++++++
 __tests__/tds/store.test-d.ts |  6 +++-
 src/index.ts                  |  2 +-
 src/ssrPlugin.ts              |  6 ++--
 src/store.ts                  | 68 +++++++++++++++++++++--------------
 src/types.ts                  | 58 +++++++++++++++++-------------
 10 files changed, 196 insertions(+), 98 deletions(-)
 create mode 100644 __tests__/state.spec.ts

diff --git a/README.md b/README.md
index 8f397a2ebc..6b4bf882ec 100644
--- a/README.md
+++ b/README.md
@@ -18,7 +18,8 @@ There are the core principles that I try to achieve with this experiment:
 
 - Flat modular structure 🍍 No nesting, only stores, compose them as needed
 - Light layer on top of Vue 💨 keep it very lightweight
-- Only `state`, `getters` 👐 `patch` is the new _mutation_
+- Only `state`, `getters`
+- No more verbose mutations, 👐 `patch` is _the mutation_
 - Actions are like _methods_ ⚗️ Group your business there
 - Import what you need, let webpack code split 📦 No need for dynamically registered modules
 - SSR support ⚙️
@@ -101,15 +102,19 @@ export const useMainStore = createStore({
   }),
   // optional getters
   getters: {
-    doubleCount: (state, getters) => state.counter * 2,
+    doubleCount() {
+      return this.counter * 2,
+    },
     // use getters in other getters
-    doubleCountPlusOne: (state, { doubleCount }) => doubleCount.value * 2,
+    doubleCountPlusOne() {
+      return this.doubleCount * 2
+    }
   },
   // optional actions
   actions: {
     reset() {
       // `this` is the store instance
-      this.state.counter = 0
+      this.counter = 0
     },
   },
 })
@@ -127,10 +132,10 @@ export default defineComponent({
     return {
       // gives access to the whole store
       main,
-      // gives access to the state
-      state: main.state,
-      // gives access to specific getter; like `computed` properties, do not include `.value`
-      doubleCount: main.doubleCount,
+      // gives access only to specific state
+      state: computed(() => main.counter),
+      // gives access to specific getter; like `computed` properties
+      doubleCount: computed(() => main.doubleCount),
     }
   },
 })
@@ -193,20 +198,31 @@ router.beforeEach((to, from, next) => {
 
 ⚠️: Note that if you are developing an SSR application, [you will need to do a bit more](#ssr).
 
-Once you have access to the store, you can access the `state` through `store.state` and any getter directly on the `store` itself as a _computed_ property (from `@vue/composition-api`) (meaning you need to use `.value` to read the actual value on the JavaScript but not in the template):
+You can access any property defined in `state` and `getters` directly on the store, similar to `data` and `computed` properties in a Vue component.
 
 ```ts
 export default defineComponent({
   setup() {
     const main = useMainStore()
-    const text = main.state.name
-    const doubleCount = main.doubleCount.value // notice the `.value` at the end
+    const text = main.name
+    const doubleCount = main.doubleCount
     return {}
   },
 })
 ```
 
-`state` is the result of a `ref` while every getter is the result of a `computed`. Both from `@vue/composition-api`.
+The `main` store in an object wrapped with `reactive`, meaning there is no need to write `.value` after getters but, like `props` in `setup`, we cannot destructure it:
+
+```ts
+export default defineComponent({
+  setup() {
+    // ❌ This won't work because it breaks reactivity
+    // it's the same as destructuring from `props`
+    const { name, doubleCount } = useMainStore()
+    return { name, doubleCount }
+  },
+})
+```
 
 Actions are invoked like methods:
 
@@ -227,7 +243,7 @@ export default defineComponent({
 To mutate the state you can either directly change something:
 
 ```ts
-main.state.counter++
+main.counter++
 ```
 
 or call the method `patch` that allows you apply multiple changes at the same time with a partial `state` object:
@@ -291,7 +307,7 @@ export default {
 }
 ```
 
-Note: **This is necessary in middlewares and other asyncronous methods**
+Note: **This is necessary in middlewares and other asynchronous methods**.
 
 It may look like things are working even if you don't pass `req` to `useStore` **but multiple concurrent requests to the server could end up sharing state between different users**.
 
@@ -344,18 +360,18 @@ createStore({
   id: 'cart',
   state: () => ({ items: [] }),
   getters: {
-    message: state => {
+    message() {
       const user = useUserStore()
-      return `Hi ${user.state.name}, you have ${items.length} items in the cart`
+      return `Hi ${user.name}, you have ${this.items.length} items in the cart`
     },
   },
   actions: {
     async purchase() {
       const user = useUserStore()
 
-      await apiBuy(user.state.token, this.state.items)
+      await apiBuy(user.token, this.items)
 
-      this.state.items = []
+      this.items = []
     },
   },
 })
@@ -386,7 +402,7 @@ export const useSharedStore = createStore({
       const user = useUserStore()
       const cart = useCartStore()
 
-      return `Hi ${user.state.name}, you have ${cart.state.list.length} items in your cart. It costs ${cart.price}.`
+      return `Hi ${user.name}, you have ${cart.list.length} items in your cart. It costs ${cart.price}.`
     },
   },
 })
@@ -410,7 +426,7 @@ export const useSharedStore = createStore({
       const cart = useCartStore()
 
       try {
-        await apiOrderCart(user.state.token, cart.state.items)
+        await apiOrderCart(user.token, cart.items)
         cart.emptyCart()
       } catch (err) {
         displayError(err)
@@ -438,13 +454,14 @@ export const useCartUserStore = pinia(
   },
   {
     getters: {
-      combinedGetter: ({ user, cart }) =>
-        `Hi ${user.state.name}, you have ${cart.state.list.length} items in your cart. It costs ${cart.price}.`,
+      combinedGetter () {
+        return `Hi ${this.user.name}, you have ${this.cart.list.length} items in your cart. It costs ${this.cart.price}.`,
+      }
     },
     actions: {
       async orderCart() {
         try {
-          await apiOrderCart(this.user.state.token, this.cart.state.items)
+          await apiOrderCart(this.user.token, this.cart.items)
           this.cart.emptyCart()
         } catch (err) {
           displayError(err)
diff --git a/__tests__/actions.spec.ts b/__tests__/actions.spec.ts
index c2fd6c511d..fb7aee8168 100644
--- a/__tests__/actions.spec.ts
+++ b/__tests__/actions.spec.ts
@@ -1,6 +1,6 @@
 import { createStore, setActiveReq } from '../src'
 
-describe('Store', () => {
+describe('Actions', () => {
   const useStore = () => {
     // create a new store
     setActiveReq({})
@@ -13,9 +13,20 @@ describe('Store', () => {
           a: { b: 'string' },
         },
       }),
+      getters: {
+        nonA(): boolean {
+          return !this.a
+        },
+        otherComputed() {
+          return this.nonA
+        },
+      },
       actions: {
+        async getNonA() {
+          return this.nonA
+        },
         toggle() {
-          this.state.a = !this.state.a
+          return (this.a = !this.a)
         },
 
         setFoo(foo: string) {
diff --git a/__tests__/getters.spec.ts b/__tests__/getters.spec.ts
index 7380b474b5..3ef37374c8 100644
--- a/__tests__/getters.spec.ts
+++ b/__tests__/getters.spec.ts
@@ -1,6 +1,6 @@
 import { createStore, setActiveReq } from '../src'
 
-describe('Store', () => {
+describe('Getters', () => {
   const useStore = () => {
     // create a new store
     setActiveReq({})
@@ -10,9 +10,18 @@ describe('Store', () => {
         name: 'Eduardo',
       }),
       getters: {
-        upperCaseName: ({ name }) => name.toUpperCase(),
-        composed: (state, { upperCaseName }) =>
-          (upperCaseName.value as string) + ': ok',
+        upperCaseName() {
+          return this.name.toUpperCase()
+        },
+        doubleName() {
+          return this.upperCaseName
+        },
+        composed() {
+          return this.upperCaseName + ': ok'
+        },
+        // TODO: I can't figure out how to pass `this` as an argument. Not sure
+        // it is possible in this specific scenario
+        // upperCaseNameArrow: store => store.name,
       },
     })()
   }
@@ -26,24 +35,24 @@ describe('Store', () => {
     id: 'A',
     state: () => ({ a: 'a' }),
     getters: {
-      fromB(state) {
+      fromB() {
         const bStore = useB()
-        return state.a + ' ' + bStore.state.b
+        return this.a + ' ' + bStore.b
       },
     },
   })
 
   it('adds getters to the store', () => {
     const store = useStore()
-    expect(store.upperCaseName.value).toBe('EDUARDO')
-    store.state.name = 'Ed'
-    expect(store.upperCaseName.value).toBe('ED')
+    expect(store.upperCaseName).toBe('EDUARDO')
+    store.name = 'Ed'
+    expect(store.upperCaseName).toBe('ED')
   })
 
   it('updates the value', () => {
     const store = useStore()
-    store.state.name = 'Ed'
-    expect(store.upperCaseName.value).toBe('ED')
+    store.name = 'Ed'
+    expect(store.upperCaseName).toBe('ED')
   })
 
   it('supports changing between requests', () => {
@@ -55,16 +64,16 @@ describe('Store', () => {
     // simulate a different request
     setActiveReq(req2)
     const bStore = useB()
-    bStore.state.b = 'c'
+    bStore.b = 'c'
 
-    aStore.state.a = 'b'
-    expect(aStore.fromB.value).toBe('b b')
+    aStore.a = 'b'
+    expect(aStore.fromB).toBe('b b')
   })
 
   it('can use other getters', () => {
     const store = useStore()
-    expect(store.composed.value).toBe('EDUARDO: ok')
-    store.state.name = 'Ed'
-    expect(store.composed.value).toBe('ED: ok')
+    expect(store.composed).toBe('EDUARDO: ok')
+    store.name = 'Ed'
+    expect(store.composed).toBe('ED: ok')
   })
 })
diff --git a/__tests__/rootState.spec.ts b/__tests__/rootState.spec.ts
index ce3881dc52..d3ee12f79a 100644
--- a/__tests__/rootState.spec.ts
+++ b/__tests__/rootState.spec.ts
@@ -1,6 +1,6 @@
 import { createStore, getRootState } from '../src'
 
-describe('Store', () => {
+describe('Root State', () => {
   const useA = createStore({
     id: 'a',
     state: () => ({ a: 'a' }),
diff --git a/__tests__/state.spec.ts b/__tests__/state.spec.ts
new file mode 100644
index 0000000000..0737ac96f3
--- /dev/null
+++ b/__tests__/state.spec.ts
@@ -0,0 +1,31 @@
+import { createStore, setActiveReq } from '../src'
+import { computed } from '@vue/composition-api'
+
+describe('State', () => {
+  const useStore = () => {
+    // create a new store
+    setActiveReq({})
+    return createStore({
+      id: 'main',
+      state: () => ({
+        name: 'Eduardo',
+        counter: 0,
+      }),
+    })()
+  }
+
+  it('can directly access state at the store level', () => {
+    const store = useStore()
+    expect(store.name).toBe('Eduardo')
+    store.name = 'Ed'
+    expect(store.name).toBe('Ed')
+  })
+
+  it('state is reactive', () => {
+    const store = useStore()
+    const upperCased = computed(() => store.name.toUpperCase())
+    expect(upperCased.value).toBe('EDUARDO')
+    store.name = 'Ed'
+    expect(upperCased.value).toBe('ED')
+  })
+})
diff --git a/__tests__/tds/store.test-d.ts b/__tests__/tds/store.test-d.ts
index d41b6875c1..a6d032314b 100644
--- a/__tests__/tds/store.test-d.ts
+++ b/__tests__/tds/store.test-d.ts
@@ -5,7 +5,9 @@ const useStore = createStore({
   id: 'name',
   state: () => ({ a: 'on' as 'on' | 'off' }),
   getters: {
-    upper: state => state.a.toUpperCase(),
+    upper() {
+      return this.a.toUpperCase()
+    },
   },
 })
 
@@ -13,4 +15,6 @@ const store = useStore()
 
 expectType<{ a: 'on' | 'off' }>(store.state)
 
+expectType<{ upper: string }>(store)
+
 expectError(() => store.nonExistant)
diff --git a/src/index.ts b/src/index.ts
index 0c34756552..6b5245048a 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1,4 +1,4 @@
 export { createStore } from './store'
 export { setActiveReq, setStateProvider, getRootState } from './rootStore'
-export { StateTree, StoreGetter, Store } from './types'
+export { StateTree, Store } from './types'
 export { PiniaSsr } from './ssrPlugin'
diff --git a/src/ssrPlugin.ts b/src/ssrPlugin.ts
index 7962caad2d..873632662d 100644
--- a/src/ssrPlugin.ts
+++ b/src/ssrPlugin.ts
@@ -2,7 +2,7 @@ import { VueConstructor } from 'vue/types'
 import { setActiveReq } from './rootStore'
 import { SetupContext } from '@vue/composition-api'
 
-export const PiniaSsr = (vue: VueConstructor) => {
+export const PiniaSsr = (_Vue: VueConstructor) => {
   const isServer = typeof window === 'undefined'
 
   if (!isServer) {
@@ -12,14 +12,14 @@ export const PiniaSsr = (vue: VueConstructor) => {
     return
   }
 
-  vue.mixin({
+  _Vue.mixin({
     beforeCreate() {
       // @ts-ignore
       const { setup, serverPrefetch } = this.$options
       if (setup) {
         // @ts-ignore
         this.$options.setup = (props: any, context: SetupContext) => {
-          // @ts-ignore
+          // @ts-ignore TODO: fix usage with nuxt-composition-api https://github.com/posva/pinia/issues/179
           if (context.ssrContext) setActiveReq(context.ssrContext.req)
           return setup(props, context)
         }
diff --git a/src/store.ts b/src/store.ts
index 83c86d8ecf..397ff5499f 100644
--- a/src/store.ts
+++ b/src/store.ts
@@ -6,10 +6,9 @@ import {
   DeepPartial,
   isPlainObject,
   StoreWithGetters,
-  StoreGetter,
-  StoreAction,
   Store,
   StoreWithActions,
+  Method,
 } from './types'
 import { useStoreDevtools } from './devtools'
 import {
@@ -40,6 +39,22 @@ function innerPatch<T extends StateTree>(
   return target
 }
 
+function toComputed<T>(refObject: Ref<T>) {
+  // let asComputed = computed<T>()
+  const reactiveObject = {} as {
+    [k in keyof T]: Ref<T[k]>
+  }
+  for (const key in refObject.value) {
+    // @ts-ignore: the key matches
+    reactiveObject[key] = computed({
+      get: () => refObject.value[key as keyof T],
+      set: value => (refObject.value[key as keyof T] = value),
+    })
+  }
+
+  return reactiveObject
+}
+
 /**
  * Creates a store instance
  * @param id unique identifier of the store, like a name. eg: main, cart, user
@@ -48,8 +63,8 @@ function innerPatch<T extends StateTree>(
 export function buildStore<
   Id extends string,
   S extends StateTree,
-  G extends Record<string, StoreGetter<S>>,
-  A extends Record<string, StoreAction>
+  G extends Record<string, Method>,
+  A extends Record<string, Method>
 >(
   id: Id,
   buildState = () => ({} as S),
@@ -82,6 +97,7 @@ export function buildStore<
     isListening = false
     innerPatch(state.value, partialState)
     isListening = true
+    // because we paused the watcher, we need to manually call the subscriptions
     subscriptions.forEach(callback => {
       callback(
         { storeName: id, type: '⤵️ patch', payload: partialState },
@@ -108,21 +124,28 @@ export function buildStore<
   const storeWithState: StoreWithState<Id, S> = {
     id,
     _r,
-    // it is replaced below by a getter
-    state: state.value,
+    // @ts-ignore, `reactive` unwraps this making it of type S
+    state: computed<S>({
+      get: () => state.value,
+      set: newState => {
+        isListening = false
+        state.value = newState
+        isListening = true
+      },
+    }),
 
     patch,
     subscribe,
     reset,
   }
 
-  const computedGetters: StoreWithGetters<S, G> = {} as StoreWithGetters<S, G>
+  const computedGetters: StoreWithGetters<G> = {} as StoreWithGetters<G>
   for (const getterName in getters) {
     computedGetters[getterName] = computed(() => {
       setActiveReq(_r)
       // eslint-disable-next-line @typescript-eslint/no-use-before-define
-      return getters[getterName](state.value, computedGetters)
-    }) as StoreWithGetters<S, G>[typeof getterName]
+      return getters[getterName].call(store, store)
+    }) as StoreWithGetters<G>[typeof getterName]
   }
 
   // const reactiveGetters = reactive(computedGetters)
@@ -132,25 +155,17 @@ export function buildStore<
     wrappedActions[actionName] = function() {
       setActiveReq(_r)
       // eslint-disable-next-line
-      return actions[actionName].apply(store, arguments as unknown as any[])
+      return actions[actionName].apply(store, (arguments as unknown) as any[])
     } as StoreWithActions<A>[typeof actionName]
   }
 
-  const store: Store<Id, S, G, A> = {
+  const store: Store<Id, S, G, A> = reactive({
     ...storeWithState,
+    // using this means no new properties can be added as state
+    ...toComputed(state),
     ...computedGetters,
     ...wrappedActions,
-  }
-
-  // make state access invisible
-  Object.defineProperty(store, 'state', {
-    get: () => state.value,
-    set: (newState: S) => {
-      isListening = false
-      state.value = newState
-      isListening = true
-    },
-  })
+  }) as Store<Id, S, G, A>
 
   return store
 }
@@ -162,14 +177,14 @@ export function buildStore<
 export function createStore<
   Id extends string,
   S extends StateTree,
-  G extends Record<string, StoreGetter<S>>,
-  A extends Record<string, StoreAction>
+  G /* extends Record<string, StoreGetterThis> */,
+  A /* extends Record<string, StoreAction> */
 >(options: {
   id: Id
   state?: () => S
-  getters?: G
+  getters?: G & ThisType<S & StoreWithGetters<G>>
   // allow actions use other actions
-  actions?: A & ThisType<A & StoreWithState<Id, S> & StoreWithGetters<S, G>>
+  actions?: A & ThisType<A & S & StoreWithState<Id, S> & StoreWithGetters<G>>
 }) {
   const { id, state, getters, actions } = options
 
@@ -183,6 +198,7 @@ export function createStore<
     if (!store) {
       stores.set(
         id,
+        // @ts-ignore
         (store = buildStore(id, state, getters, actions, getInitialState(id)))
       )
 
diff --git a/src/types.ts b/src/types.ts
index 725704387a..ede3d79a00 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -16,11 +16,6 @@ export function isPlainObject(
 
 export type NonNullObject = Record<any, any>
 
-export interface StoreGetter<S extends StateTree, T = any> {
-  // TODO: would be nice to be able to define the getters here
-  (state: S, getters: Record<string, Ref<any>>): T
-}
-
 type TODO = any
 // type StoreMethod = TODO
 export type DeepPartial<T> = { [K in keyof T]?: DeepPartial<T[K]> }
@@ -31,13 +26,6 @@ export type SubscriptionCallback<S> = (
   state: S
 ) => void
 
-export type StoreWithGetters<
-  S extends StateTree,
-  G extends Record<string, StoreGetter<S>>
-> = {
-  [k in keyof G]: G[k] extends StoreGetter<S, infer V> ? Ref<V> : never
-}
-
 export interface StoreWithState<Id extends string, S extends StateTree> {
   /**
    * Unique identifier of the store
@@ -45,7 +33,7 @@ export interface StoreWithState<Id extends string, S extends StateTree> {
   id: Id
 
   /**
-   * State of the Store
+   * State of the Store. Setting it will replace the whole state.
    */
   state: S
 
@@ -74,30 +62,52 @@ export interface StoreWithState<Id extends string, S extends StateTree> {
   subscribe(callback: SubscriptionCallback<S>): () => void
 }
 
-export interface StoreAction {
-  (...args: any[]): any
-}
+export type Method = (...args: any[]) => any
+
+// export type StoreAction<P extends any[], R> = (...args: P) => R
+// export interface StoreAction<P, R> {
+//   (...args: P[]): R
+// }
 
 // in this type we forget about this because otherwise the type is recursive
-export type StoreWithActions<A extends Record<string, StoreAction>> = {
-  [k in keyof A]: A[k] extends (this: infer This, ...args: infer P) => infer R
-    ? (this: This, ...args: P) => R
+export type StoreWithActions<A> = {
+  [k in keyof A]: A[k] extends (...args: infer P) => infer R
+    ? (...args: P) => R
+    : never
+}
+
+// export interface StoreGetter<S extends StateTree, T = any> {
+//   // TODO: would be nice to be able to define the getters here
+//   (state: S, getters: Record<string, Ref<any>>): T
+// }
+
+export type StoreWithGetters<G> = {
+  [k in keyof G]: G[k] extends (this: infer This, store?: any) => infer R
+    ? R
     : never
 }
 
+// // in this type we forget about this because otherwise the type is recursive
+// export type StoreWithThisGetters<G> = {
+//   // TODO: does the infer this as the second argument work?
+//   [k in keyof G]: G[k] extends (this: infer This, store?: any) => infer R
+//     ? (this: This, store?: This) => R
+//     : never
+// }
+
 // has the actions without the context (this) for typings
 export type Store<
   Id extends string,
   S extends StateTree,
-  G extends Record<string, StoreGetter<S>>,
-  A extends Record<string, StoreAction>
-> = StoreWithState<Id, S> & StoreWithGetters<S, G> & StoreWithActions<A>
+  G,
+  A
+> = StoreWithState<Id, S> & S & StoreWithGetters<G> & StoreWithActions<A>
 
 export type GenericStore = Store<
   string,
   StateTree,
-  Record<string, StoreGetter<StateTree>>,
-  Record<string, StoreAction>
+  Record<string, Method>,
+  Record<string, Method>
 >
 
 export interface DevtoolHook {