From 751f2867b97f210488eb82bad1ec05af6ab6e72c Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Fri, 9 Oct 2020 12:42:23 +0200 Subject: [PATCH] refactor: prefix store properties with $ (#254) BREAKING CHANGE: all store properties (`id`, `state`, `patch`, `subscribe`, and `reset`) are now prefixed with `$` to allow properties defined with the same type and avoid types breaking. Tip: you can refactor your whole codebase with F2 (or right-click + Refactor) on each of the store's properties --- README.md | 6 +-- __tests__/actions.spec.ts | 38 +++++++++--------- __tests__/pinia/stores/cart.ts | 15 +++---- __tests__/pinia/stores/user.ts | 6 +-- __tests__/ssr.spec.ts | 2 +- __tests__/store.patch.spec.ts | 18 ++++----- __tests__/store.spec.ts | 70 +++++++++++++++++++++++---------- __tests__/subscriptions.spec.ts | 14 +++---- src/devtools.ts | 20 +++++----- src/rootStore.ts | 27 +++++++++---- src/store.ts | 23 +++++------ src/types.ts | 18 +++++---- test-dts/deprecated.test-d.ts | 2 +- test-dts/store.test-d.ts | 3 +- 14 files changed, 154 insertions(+), 108 deletions(-) diff --git a/README.md b/README.md index 1ea5ac0e..2bc16422 100644 --- a/README.md +++ b/README.md @@ -237,16 +237,16 @@ To mutate the state you can either directly change something: main.counter++ ``` -or call the method `patch` that allows you apply multiple changes at the same time with a partial `state` object: +or call the method `$patch` that allows you apply multiple changes at the same time with a partial `state` object: ```ts -main.patch({ +main.$patch({ counter: -1, name: 'Abalam', }) ``` -The main difference here is that `patch` allows you to group multiple changes into one single entry in the devtools. +The main difference here is that `$patch` allows you to group multiple changes into one single entry in the devtools. ### Replacing the `state` diff --git a/__tests__/actions.spec.ts b/__tests__/actions.spec.ts index a97a7227..f1fdd324 100644 --- a/__tests__/actions.spec.ts +++ b/__tests__/actions.spec.ts @@ -30,7 +30,7 @@ describe('Actions', () => { }, setFoo(foo: string) { - this.patch({ nested: { foo } }) + this.$patch({ nested: { foo } }) }, combined() { @@ -52,34 +52,34 @@ describe('Actions', () => { actions: { swap() { const bStore = useB() - const b = bStore.state.b - bStore.state.b = this.state.a - this.state.a = b + const b = bStore.$state.b + bStore.$state.b = this.$state.a + this.$state.a = b }, }, }) it('can use the store as this', () => { const store = useStore() - expect(store.state.a).toBe(true) + expect(store.$state.a).toBe(true) store.toggle() - expect(store.state.a).toBe(false) + expect(store.$state.a).toBe(false) }) it('store is forced as the context', () => { const store = useStore() - expect(store.state.a).toBe(true) + expect(store.$state.a).toBe(true) store.toggle.call(null) - expect(store.state.a).toBe(false) + expect(store.$state.a).toBe(false) }) it('can call other actions', () => { const store = useStore() - expect(store.state.a).toBe(true) - expect(store.state.nested.foo).toBe('foo') + expect(store.$state.a).toBe(true) + expect(store.$state.nested.foo).toBe('foo') store.combined() - expect(store.state.a).toBe(false) - expect(store.state.nested.foo).toBe('bar') + expect(store.$state.a).toBe(false) + expect(store.$state.nested.foo).toBe('bar') }) it('supports being called between requests', () => { @@ -91,12 +91,12 @@ describe('Actions', () => { // simulate a different request setActiveReq(req2) const bStore = useB() - bStore.state.b = 'c' + bStore.$state.b = 'c' aStore.swap() - expect(aStore.state.a).toBe('b') + expect(aStore.$state.a).toBe('b') // a different instance of b store was used - expect(bStore.state.b).toBe('c') + expect(bStore.$state.b).toBe('c') }) it('can force the req', () => { @@ -105,13 +105,13 @@ describe('Actions', () => { const aStore = useA(req1) let bStore = useB(req2) - bStore.state.b = 'c' + bStore.$state.b = 'c' aStore.swap() - expect(aStore.state.a).toBe('b') + expect(aStore.$state.a).toBe('b') // a different instance of b store was used - expect(bStore.state.b).toBe('c') + expect(bStore.$state.b).toBe('c') bStore = useB(req1) - expect(bStore.state.b).toBe('a') + expect(bStore.$state.b).toBe('a') }) }) diff --git a/__tests__/pinia/stores/cart.ts b/__tests__/pinia/stores/cart.ts index f7bd76c1..13aadf39 100644 --- a/__tests__/pinia/stores/cart.ts +++ b/__tests__/pinia/stores/cart.ts @@ -4,6 +4,7 @@ import { useUserStore } from './user' export const useCartStore = defineStore({ id: 'cart', state: () => ({ + id: 2, rawItems: [] as string[], }), getters: { @@ -28,7 +29,7 @@ export const useCartStore = defineStore({ removeItem(name: string) { const i = this.rawItems.indexOf(name) - if (i > -1) this.rawItems.splice(i, 1) + if (i > -1) this.$state.rawItems.splice(i, 1) }, // TODO: use multiple stores @@ -39,7 +40,7 @@ export const useCartStore = defineStore({ // console.log('Purchasing', this.items) const n = this.items.length - this.state.rawItems = [] + this.rawItems = [] return { amount: n, user: user.name } }, @@ -50,23 +51,23 @@ export type CartStore = ReturnType export function addItem(name: string) { const store = useCartStore() - store.state.rawItems.push(name) + store.rawItems.push(name) } export function removeItem(name: string) { const store = useCartStore() - const i = store.state.rawItems.indexOf(name) - if (i > -1) store.state.rawItems.splice(i, 1) + const i = store.rawItems.indexOf(name) + if (i > -1) store.rawItems.splice(i, 1) } export async function purchaseItems() { const cart = useCartStore() const user = useUserStore() - if (!user.state.name) return + if (!user.name) return console.log('Purchasing', cart.items) const n = cart.items.length - cart.state.rawItems = [] + cart.rawItems = [] return n } diff --git a/__tests__/pinia/stores/user.ts b/__tests__/pinia/stores/user.ts index 79c07d00..956fb1d4 100644 --- a/__tests__/pinia/stores/user.ts +++ b/__tests__/pinia/stores/user.ts @@ -15,7 +15,7 @@ export const useUserStore = defineStore({ async login(user: string, password: string) { const userData = await apiLogin(user, password) - this.patch({ + this.$patch({ name: user, ...userData, }) @@ -24,7 +24,7 @@ export const useUserStore = defineStore({ logout() { this.login('a', 'b').then(() => {}) - this.patch({ + this.$patch({ name: '', isAdmin: false, }) @@ -46,7 +46,7 @@ export function logout() { store.login('e', 'e').then(() => {}) - store.patch({ + store.$patch({ name: '', isAdmin: false, }) diff --git a/__tests__/ssr.spec.ts b/__tests__/ssr.spec.ts index 3a7e017c..9860ec60 100644 --- a/__tests__/ssr.spec.ts +++ b/__tests__/ssr.spec.ts @@ -2,10 +2,10 @@ * @jest-environment node */ import { createPinia } from '../src' -import { useCartStore } from './pinia/stores/cart' import { createSSRApp, inject } from 'vue' import { renderToString, ssrInterpolate } from '@vue/server-renderer' import { useUserStore } from './pinia/stores/user' +import { useCartStore } from './pinia/stores/cart' describe('SSR', () => { const App = { diff --git a/__tests__/store.patch.spec.ts b/__tests__/store.patch.spec.ts index a8927f38..b3a1abdb 100644 --- a/__tests__/store.patch.spec.ts +++ b/__tests__/store.patch.spec.ts @@ -1,6 +1,6 @@ import { defineStore, setActiveReq } from '../src' -describe('store.patch', () => { +describe('store.$patch', () => { const useStore = () => { // create a new store setActiveReq({}) @@ -18,8 +18,8 @@ describe('store.patch', () => { it('patches a property without touching the rest', () => { const store = useStore() - store.patch({ a: false }) - expect(store.state).toEqual({ + store.$patch({ a: false }) + expect(store.$state).toEqual({ a: false, nested: { foo: 'foo', @@ -30,16 +30,16 @@ describe('store.patch', () => { it('patches a nested property without touching the rest', () => { const store = useStore() - store.patch({ nested: { foo: 'bar' } }) - expect(store.state).toEqual({ + store.$patch({ nested: { foo: 'bar' } }) + expect(store.$state).toEqual({ a: true, nested: { foo: 'bar', a: { b: 'string' }, }, }) - store.patch({ nested: { a: { b: 'hello' } } }) - expect(store.state).toEqual({ + store.$patch({ nested: { a: { b: 'hello' } } }) + expect(store.$state).toEqual({ a: true, nested: { foo: 'bar', @@ -50,8 +50,8 @@ describe('store.patch', () => { it('patches multiple properties at the same time', () => { const store = useStore() - store.patch({ a: false, nested: { foo: 'hello' } }) - expect(store.state).toEqual({ + store.$patch({ a: false, nested: { foo: 'hello' } }) + expect(store.$state).toEqual({ a: false, nested: { foo: 'hello', diff --git a/__tests__/store.spec.ts b/__tests__/store.spec.ts index 6f3be1f3..dde45999 100644 --- a/__tests__/store.spec.ts +++ b/__tests__/store.spec.ts @@ -5,6 +5,7 @@ import { setStateProvider, } from '../src' import { mount } from '@vue/test-utils' +import { getCurrentInstance } from 'vue' describe('Store', () => { let req: object @@ -26,7 +27,7 @@ describe('Store', () => { it('sets the initial state', () => { const store = useStore() - expect(store.state).toEqual({ + expect(store.$state).toEqual({ a: true, nested: { foo: 'foo', @@ -37,13 +38,13 @@ describe('Store', () => { it('can be reset', () => { const store = useStore() - store.state.a = false + store.$state.a = false const spy = jest.fn() - store.subscribe(spy) - store.reset() - store.state.nested.foo = 'bar' + store.$subscribe(spy) + store.$reset() + store.$state.nested.foo = 'bar' expect(spy).not.toHaveBeenCalled() - expect(store.state).toEqual({ + expect(store.$state).toEqual({ a: true, nested: { foo: 'bar', @@ -55,7 +56,7 @@ describe('Store', () => { it('can create an empty state if no state option is provided', () => { const store = defineStore({ id: 'some' })() - expect(store.state).toEqual({}) + expect(store.$state).toEqual({}) }) it('can hydrate the state', () => { @@ -83,7 +84,7 @@ describe('Store', () => { const store = useStore() - expect(store.state).toEqual({ + expect(store.$state).toEqual({ a: false, nested: { foo: 'bar', @@ -94,7 +95,7 @@ describe('Store', () => { it('can replace its state', () => { const store = useStore() - store.state = { + store.$state = { a: false, nested: { foo: 'bar', @@ -103,7 +104,7 @@ describe('Store', () => { }, }, } - expect(store.state).toEqual({ + expect(store.$state).toEqual({ a: false, nested: { foo: 'bar', @@ -115,17 +116,17 @@ describe('Store', () => { it('do not share the state between same id store', () => { const store = useStore() const store2 = useStore() - expect(store.state).not.toBe(store2.state) - store.state.nested.a.b = 'hey' - expect(store2.state.nested.a.b).toBe('string') + expect(store.$state).not.toBe(store2.$state) + store.$state.nested.a.b = 'hey' + expect(store2.$state.nested.a.b).toBe('string') }) it('subscribe to changes', () => { const store = useStore() const spy = jest.fn() - store.subscribe(spy) + store.$subscribe(spy) - store.state.a = false + store.$state.a = false expect(spy).toHaveBeenCalledWith( { @@ -133,17 +134,17 @@ describe('Store', () => { storeName: 'main', type: expect.stringContaining('in place'), }, - store.state + store.$state ) }) it('subscribe to changes done via patch', () => { const store = useStore() const spy = jest.fn() - store.subscribe(spy) + store.$subscribe(spy) const patch = { a: false } - store.patch(patch) + store.$patch(patch) expect(spy).toHaveBeenCalledWith( { @@ -151,7 +152,7 @@ describe('Store', () => { storeName: 'main', type: expect.stringContaining('patch'), }, - store.state + store.$state ) }) @@ -180,7 +181,7 @@ describe('Store', () => { if (!store) throw new Error('no store') const spy = jest.fn() - store.subscribe(spy) + store.$subscribe(spy) expect(spy).toHaveBeenCalledTimes(0) store.a = !store.a @@ -190,4 +191,33 @@ describe('Store', () => { store.a = !store.a expect(spy).toHaveBeenCalledTimes(2) }) + + it.skip('should not break getCurrentInstance', () => { + let store: ReturnType | undefined + + let i1: any = {} + let i2: any = {} + const wrapper = mount( + { + setup() { + i1 = getCurrentInstance() + store = useStore() + i2 = getCurrentInstance() + + return { store } + }, + + template: `a: {{ store.a }}`, + }, + { + global: { + plugins: [createPinia()], + }, + } + ) + + expect(i1 === i2).toBe(true) + + wrapper.unmount() + }) }) diff --git a/__tests__/subscriptions.spec.ts b/__tests__/subscriptions.spec.ts index 3b426c69..859f3cc2 100644 --- a/__tests__/subscriptions.spec.ts +++ b/__tests__/subscriptions.spec.ts @@ -19,27 +19,27 @@ describe('Subscriptions', () => { it('fires callback when patch is applied', () => { const spy = jest.fn() - store.subscribe(spy) - store.state.name = 'Cleiton' + store.$subscribe(spy) + store.$state.name = 'Cleiton' expect(spy).toHaveBeenCalledTimes(1) }) it('unsubscribes callback when unsubscribe is called', () => { const spy = jest.fn() - const unsubscribe = store.subscribe(spy) + const unsubscribe = store.$subscribe(spy) unsubscribe() - store.state.name = 'Cleiton' + store.$state.name = 'Cleiton' expect(spy).not.toHaveBeenCalled() }) it('listeners are not affected when unsubscribe is called multiple times', () => { const func1 = jest.fn() const func2 = jest.fn() - const unsubscribe1 = store.subscribe(func1) - store.subscribe(func2) + const unsubscribe1 = store.$subscribe(func1) + store.$subscribe(func2) unsubscribe1() unsubscribe1() - store.state.name = 'Cleiton' + store.$state.name = 'Cleiton' expect(func1).not.toHaveBeenCalled() expect(func2).toHaveBeenCalledTimes(1) }) diff --git a/src/devtools.ts b/src/devtools.ts index aeb897cd..29f86c9b 100644 --- a/src/devtools.ts +++ b/src/devtools.ts @@ -29,10 +29,10 @@ export function addDevtools(app: App, store: GenericStore, req: NonNullObject) { api.on.inspectComponent((payload, ctx) => { if (payload.instanceData) { payload.instanceData.state.push({ - type: '🍍 ' + store.id, + type: '🍍 ' + store.$id, key: 'state', editable: false, - value: store.state, + value: store.$state, }) } }) @@ -67,7 +67,7 @@ export function addDevtools(app: App, store: GenericStore, req: NonNullObject) { api.sendInspectorState(piniaInspectorId) } - store.subscribe((mutation, state) => { + store.$subscribe((mutation, state) => { // rootStore.state[store.id] = state const data: Record = { store: formatDisplay(mutation.storeName), @@ -99,7 +99,7 @@ export function addDevtools(app: App, store: GenericStore, req: NonNullObject) { payload.rootNodes = (payload.filter ? stores.filter((store) => - store.id.toLowerCase().includes(payload.filter.toLowerCase()) + store.$id.toLowerCase().includes(payload.filter.toLowerCase()) ) : stores ).map(formatStoreForInspectorTree) @@ -109,7 +109,7 @@ export function addDevtools(app: App, store: GenericStore, req: NonNullObject) { api.on.getInspectorState((payload) => { if (payload.app === app && payload.inspectorId === piniaInspectorId) { const stores = Array.from(getRegisteredStores()) - const store = stores.find((store) => store.id === payload.nodeId) + const store = stores.find((store) => store.$id === payload.nodeId) if (store) { payload.state = { @@ -127,15 +127,15 @@ export function addDevtools(app: App, store: GenericStore, req: NonNullObject) { // trigger an update so it can display new registered stores // @ts-ignore api.notifyComponentUpdate() - __VUE_DEVTOOLS_TOAST__(`🍍 "${store.id}" store installed`) + __VUE_DEVTOOLS_TOAST__(`🍍 "${store.$id}" store installed`) } ) } function formatStoreForInspectorTree(store: GenericStore): CustomInspectorNode { return { - id: store.id, - label: store.id, + id: store.$id, + label: store.$id, tags: [], } } @@ -144,8 +144,8 @@ function formatStoreForInspectorState( store: GenericStore ): CustomInspectorState[string] { const fields: CustomInspectorState[string] = [ - { editable: false, key: 'id', value: formatDisplay(store.id) }, - { editable: true, key: 'state', value: store.state }, + { editable: false, key: 'id', value: formatDisplay(store.$id) }, + { editable: true, key: 'state', value: store.$state }, ] return fields diff --git a/src/rootStore.ts b/src/rootStore.ts index ca87c566..f99d17b2 100644 --- a/src/rootStore.ts +++ b/src/rootStore.ts @@ -1,4 +1,5 @@ import { App, InjectionKey, Plugin } from 'vue' +import { IS_CLIENT } from './env' import { NonNullObject, StateTree, GenericStore } from './types' /** @@ -51,7 +52,7 @@ export function getRootState(req: NonNullObject): Record { // forEach is the only one that also works on IE11 stores.forEach((store) => { - rootState[store.id] = store.state + rootState[store.$id] = store.$state }) return rootState @@ -66,9 +67,16 @@ export const getClientApp = () => clientApp export interface Pinia { install: Exclude - store GenericStore>( - useStore: F - ): ReturnType + store any>(useStore: F): ReturnType +} + +declare module '@vue/runtime-core' { + export interface ComponentCustomProperties { + /** + * Instantiate a store anywhere + */ + $pinia: Pinia['store'] + } } export const piniaSymbol = (__DEV__ @@ -79,14 +87,17 @@ export function createPinia(): Pinia { const pinia: Pinia = { install(app: App) { app.provide(piniaSymbol, pinia) - // TODO: strip out if no need for - setClientApp(app) + app.config.globalProperties.$pinia = pinia.store + // TODO: write test + // only set the app on client + if (__BROWSER__ && IS_CLIENT) { + setClientApp(app) + } }, store GenericStore>( useStore: F ): ReturnType { - const store = useStore(pinia) as ReturnType - return store + return useStore(pinia) as ReturnType }, } diff --git a/src/store.ts b/src/store.ts index 83a537c8..2cfbb43d 100644 --- a/src/store.ts +++ b/src/store.ts @@ -76,7 +76,7 @@ export function buildStore< G extends Record, A extends Record >( - id: Id, + $id: Id, buildState: () => S = () => ({} as S), getters: G = {} as G, actions: A = {} as A, @@ -94,7 +94,7 @@ export function buildStore< (state) => { if (isListening) { subscriptions.forEach((callback) => { - callback({ storeName: id, type: '🧩 in place', payload: {} }, state) + callback({ storeName: $id, type: '🧩 in place', payload: {} }, state) }) } }, @@ -104,20 +104,20 @@ export function buildStore< } ) - function patch(partialState: DeepPartial): void { + function $patch(partialState: DeepPartial): void { 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 }, + { storeName: $id, type: '⤵️ patch', payload: partialState }, state.value ) }) } - function subscribe(callback: SubscriptionCallback) { + function $subscribe(callback: SubscriptionCallback) { subscriptions.push(callback) return () => { const idx = subscriptions.indexOf(callback) @@ -127,16 +127,16 @@ export function buildStore< } } - function reset() { + function $reset() { subscriptions = [] state.value = buildState() } const storeWithState: StoreWithState = { - id, + $id, _r, // @ts-ignore, `reactive` unwraps this making it of type S - state: computed({ + $state: computed({ get: () => state.value, set: (newState) => { isListening = false @@ -145,9 +145,9 @@ export function buildStore< }, }), - patch, - subscribe, - reset, + $patch, + $subscribe, + $reset, } const computedGetters: StoreWithGetters = {} as StoreWithGetters @@ -206,6 +206,7 @@ export function defineStore< // avoid injecting if `useStore` when not possible reqKey = reqKey || (getCurrentInstance() && inject(piniaSymbol)) if (reqKey) setActiveReq(reqKey) + // TODO: worth warning on server if no reqKey as it can leak data const req = getActiveReq() let stores = storesMap.get(req) if (!stores) storesMap.set(req, (stores = new Map())) diff --git a/src/types.ts b/src/types.ts index 2ffa164f..cb5e70fc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -32,36 +32,40 @@ export interface StoreWithState { /** * Unique identifier of the store */ - id: Id + $id: Id /** * State of the Store. Setting it will replace the whole state. */ - state: S + $state: S /** * Private property defining the request key for this store + * + * @internal */ _r: NonNullObject /** * Applies a state patch to current state. Allows passing nested values + * * @param partialState - patch to apply to the state */ - patch(partialState: DeepPartial): void + $patch(partialState: DeepPartial): void /** * Resets the store to its initial state by removing all subscriptions and * building a new state object */ - reset(): void + $reset(): void /** * Setups a callback to be called whenever the state changes. - * @param callback - callback that is called whenever the state - * @returns function that removes callback from subscriptions + * + * @param callback - callback passed to the watcher + * @returns function that removes the watcher */ - subscribe(callback: SubscriptionCallback): () => void + $subscribe(callback: SubscriptionCallback): () => void } export type Method = (...args: any[]) => any diff --git a/test-dts/deprecated.test-d.ts b/test-dts/deprecated.test-d.ts index d9ee9d15..5acd8258 100644 --- a/test-dts/deprecated.test-d.ts +++ b/test-dts/deprecated.test-d.ts @@ -12,7 +12,7 @@ const useDeprecated = createStore({ const deprecatedStore = useDeprecated() -expectType<{ a: 'on' | 'off' }>(deprecatedStore.state) +expectType<{ a: 'on' | 'off' }>(deprecatedStore.$state) expectType(deprecatedStore.nested.counter) expectType<'on' | 'off'>(deprecatedStore.a) diff --git a/test-dts/store.test-d.ts b/test-dts/store.test-d.ts index 1858f57f..8105e2f7 100644 --- a/test-dts/store.test-d.ts +++ b/test-dts/store.test-d.ts @@ -12,8 +12,7 @@ const useStore = defineStore({ let store = useStore() -// FIXME: this should not be there anymore -expectType<{ a: 'on' | 'off' }>(store.state) +expectType<{ a: 'on' | 'off' }>(store.$state) expectType(store.nested.counter) expectType<'on' | 'off'>(store.a) -- 2.47.2