diff --git a/src/components/__tests__/integration.test.js b/src/__tests__/integration.test.js similarity index 95% rename from src/components/__tests__/integration.test.js rename to src/__tests__/integration.test.js index b35b345..6014ee7 100644 --- a/src/components/__tests__/integration.test.js +++ b/src/__tests__/integration.test.js @@ -5,10 +5,10 @@ import React, { Fragment, memo, useEffect } from 'react'; import { render } from '@testing-library/react'; import { act } from 'react-dom/test-utils'; -import { createStore, defaultRegistry } from '../../store'; -import { createContainer } from '../container'; -import { createSubscriber } from '../subscriber'; -import { createHook } from '../hook'; +import { createStore, defaultRegistry } from '../store'; +import { createContainer } from '../components/container'; +import { createSubscriber } from '../components/subscriber'; +import { createHook } from '../components/hook'; const actTick = () => act(async () => await Promise.resolve()); @@ -471,10 +471,8 @@ describe('Integration', () => { expect(handlers2.onDestroy).toHaveBeenCalledTimes(1); }); - it('should throw an error if contained store is used without container', async () => { - const rejectSpy = jest - .spyOn(Promise, 'reject') - .mockImplementation(() => {}); + it('should throw an error if contained store is used without container', () => { + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); const Store1 = createStore({ name: 'one', initialState: { todos: [], loading: false }, @@ -483,12 +481,10 @@ describe('Integration', () => { }); const Subscriber = createSubscriber(Store1); - render({() => null}); - await actTick(); - expect(rejectSpy).toHaveBeenCalled(); - const [error] = rejectSpy.mock.calls[0]; - expect(error).toEqual(expect.any(Error)); - expect(error.message).toContain('should be contained'); + expect(() => { + render({() => null}); + }).toThrow(/should be contained/); + errorSpy.mockRestore(); }); }); diff --git a/src/components/__tests__/container.test.js b/src/components/__tests__/container.test.js index 3b3fc08..97ed7c0 100644 --- a/src/components/__tests__/container.test.js +++ b/src/components/__tests__/container.test.js @@ -1,7 +1,7 @@ /* eslint-env jest */ import React from 'react'; -import { render } from '@testing-library/react'; +import { render, act } from '@testing-library/react'; import { StoreMock, storeStateMock } from '../../__tests__/mocks'; import { defaultRegistry, StoreRegistry } from '../../store/registry'; @@ -267,7 +267,7 @@ describe('Container', () => { const children = {renderPropChildren}; render({children}); const [, { increase }] = renderPropChildren.mock.calls[0]; - increase(); + act(() => increase()); expect(actionInner).toHaveBeenCalledWith(expect.any(Object), { defaultCount: 5, @@ -285,7 +285,7 @@ describe('Container', () => { ); const [, { increase }] = renderPropChildren.mock.calls[0]; rerender({children}); - increase(); + act(() => increase()); expect(actionInner).toHaveBeenCalledWith(expect.any(Object), { defaultCount: 6, diff --git a/src/components/container.js b/src/components/container.js index a6394e5..ea0be24 100644 --- a/src/components/container.js +++ b/src/components/container.js @@ -49,16 +49,16 @@ export default class Container extends Component { // These fallbacks are needed only to make enzyme shallow work // as it does not fully support provider-less Context enzyme#1553 globalRegistry = defaultRegistry, - getStore = defaultRegistry.getStore, + retrieveStore = defaultRegistry.getStore, } = this.context; this.state = { api: { globalRegistry, - getStore: (Store, scope) => + retrieveStore: (Store, scope) => this.constructor.storeType === Store ? this.getScopedStore(scope) - : getStore(Store), + : retrieveStore(Store), }, // stored to make them available in getDerivedStateFromProps // as js context there is null https://github.com/facebook/react/issues/12612 @@ -103,20 +103,15 @@ export default class Container extends Component { const { storeType, hooks } = this.constructor; const { api } = this.state; // we explicitly pass scope as it might be changed - const { storeState } = api.getStore(storeType, scope); + const { storeState } = api.retrieveStore(storeType, scope); - const actions = bindActions( - storeType.actions, - storeState, - this.getContainerProps - ); + const config = { + props: () => this.actionProps, + contained: (s) => storeType === s, + }; - this.scopedHooks = bindActions( - hooks, - storeState, - this.getContainerProps, - actions - ); + const actions = bindActions(storeType.actions, storeState, config); + this.scopedHooks = bindActions(hooks, storeState, config, actions); // make sure we also reset actionProps this.actionProps = null; @@ -146,8 +141,6 @@ export default class Container extends Component { return restProps; }; - getContainerProps = () => this.actionProps; - getRegistry() { const isLocal = !this.props.scope && !this.props.isGlobal; return isLocal ? this.registry : this.state.api.globalRegistry; @@ -224,15 +217,15 @@ function useRegistry(scope, isGlobal, { globalRegistry }) { }, [scope, isGlobal, globalRegistry]); } -function useContainedStore(scope, registry, props, override) { +function useContainedStore(scope, registry, props, check, override) { // Store contained scopes in a map, but throwing it away on scope change // eslint-disable-next-line react-hooks/exhaustive-deps const containedStores = useMemo(() => new Map(), [scope]); // Store props in a ref to avoid re-binding actions when they change and re-rendering all // consumers unnecessarily. The update is handled by an effect on the component instead - const containerProps = useRef(); - containerProps.current = props; + const propsRef = useRef(); + propsRef.current = props; const getContainedStore = useCallback( (Store) => { @@ -241,13 +234,12 @@ function useContainedStore(scope, registry, props, override) { // so we can provide props to actions (only triggered by children) if (!containedStore) { const isExisting = registry.hasStore(Store, scope); - const { storeState } = registry.getStore(Store, scope, true); - const getProps = () => containerProps.current; - const actions = bindActions(Store.actions, storeState, getProps); + const config = { props: () => propsRef.current, contained: check }; + const { storeState, actions } = registry.getStore(Store, scope, config); const handlers = bindActions( { ...Store.handlers, ...override?.handlers }, storeState, - getProps, + config, actions ); containedStore = { @@ -263,20 +255,20 @@ function useContainedStore(scope, registry, props, override) { } return containedStore; }, - [containedStores, registry, scope, override] + [containedStores, registry, scope, check, override] ); return [containedStores, getContainedStore]; } -function useApi(check, getContainedStore, { globalRegistry, getStore }) { - const getStoreRef = useRef(); - getStoreRef.current = (Store) => - check(Store) ? getContainedStore(Store) : getStore(Store); +function useApi(check, getContainedStore, { globalRegistry, retrieveStore }) { + const retrieveRef = useRef(); + retrieveRef.current = (Store) => + check(Store) ? getContainedStore(Store) : retrieveStore(Store); // This api is "frozen", as changing it will trigger re-render across all consumers - // so we link getStore dynamically and manually call notify() on scope change + // so we link retrieveStore dynamically and manually call notify() on scope change return useMemo( - () => ({ globalRegistry, getStore: (s) => getStoreRef.current(s) }), + () => ({ globalRegistry, retrieveStore: (s) => retrieveRef.current(s) }), [globalRegistry] ); } @@ -294,6 +286,7 @@ function createFunctionContainer({ displayName, override } = {}) { scope, registry, restProps, + check, override ); const api = useApi(check, getContainedStore, ctx); @@ -329,7 +322,7 @@ function createFunctionContainer({ displayName, override } = {}) { !storeState.listeners().length && // ensure registry has not already created a new store with same scope storeState === - registry.getStore(Store, cachedScope, true).storeState + registry.getStore(Store, cachedScope, null)?.storeState ) { handlers.onDestroy?.(); registry.deleteStore(Store, cachedScope); diff --git a/src/components/hook.js b/src/components/hook.js index 4003de3..73fe51d 100644 --- a/src/components/hook.js +++ b/src/components/hook.js @@ -9,8 +9,8 @@ const DEFAULT_SELECTOR = (state) => state; export function createHook(Store, { selector } = {}) { return function useSweetState(propsArg) { - const { getStore } = useContext(Context); - const { storeState, actions } = getStore(Store); + const { retrieveStore } = useContext(Context); + const { storeState, actions } = retrieveStore(Store); const hasPropsArg = propsArg !== undefined; const propsArgRef = useRef(propsArg); @@ -27,9 +27,9 @@ export function createHook(Store, { selector } = {}) { ); const getSnapshot = useCallback(() => { - const state = getStore(Store).storeState.getState(); + const state = retrieveStore(Store).storeState.getState(); return stateSelector(state, propsArgRef.current); - }, [getStore, stateSelector]); + }, [retrieveStore, stateSelector]); const currentState = useSyncExternalStore( storeState.subscribe, diff --git a/src/context.js b/src/context.js index 25f0899..85fc707 100644 --- a/src/context.js +++ b/src/context.js @@ -7,7 +7,7 @@ import { defaultRegistry } from './store'; export const Context = React.createContext( { globalRegistry: defaultRegistry, - getStore: defaultRegistry.getStore, + retrieveStore: (Store) => defaultRegistry.getStore(Store), }, () => 0 ); diff --git a/src/store/__tests__/bind-actions.test.js b/src/store/__tests__/bind-actions.test.js index 8ce9aeb..3317913 100644 --- a/src/store/__tests__/bind-actions.test.js +++ b/src/store/__tests__/bind-actions.test.js @@ -6,6 +6,11 @@ import defaults from '../../defaults'; jest.mock('../../defaults'); +const createConfigArg = ({ props = {} } = {}) => ({ + props: () => props, + contained: () => false, +}); + describe('bindAction', () => { it('should bind and call providing mutator, getState and container props', () => { const actionInner = jest.fn(); @@ -16,7 +21,7 @@ describe('bindAction', () => { storeStateMock, action, 'myAction', - () => containerProps, + createConfigArg({ props: containerProps }), actionsMock ); result(1, '2', 3); @@ -44,7 +49,12 @@ describe('bindAction', () => { () => ({ setState }) => setState(); - const result = bindAction(storeStateMock, action, 'myAction'); + const result = bindAction( + storeStateMock, + action, + 'myAction', + createConfigArg() + ); result(); expect(storeStateMock.mutator.actionName).toEqual('myAction'); @@ -60,7 +70,12 @@ describe('bindAction', () => { () => ({ dispatch }) => dispatch(action2()); - const result = bindAction(storeStateMock, action, 'myAction2'); + const result = bindAction( + storeStateMock, + action, + 'myAction2', + createConfigArg() + ); result(); expect(storeStateMock.mutator.actionName).toEqual('myAction2.dispatch'); @@ -69,7 +84,7 @@ describe('bindAction', () => { describe('bindActions', () => { it('should return all actions bound', () => { - const result = bindActions(actionsMock, storeStateMock); + const result = bindActions(actionsMock, storeStateMock, createConfigArg()); expect(result).toEqual({ increase: expect.any(Function), decrease: expect.any(Function), @@ -83,7 +98,7 @@ describe('bindActions', () => { dispatch(actionsMock.decrease()) ); actionsMock.decrease.mockReturnValue(({ getState }) => getState()); - const result = bindActions(actionsMock, storeStateMock); + const result = bindActions(actionsMock, storeStateMock, createConfigArg()); const output = result.increase(); expect(output).toEqual(state); diff --git a/src/store/__tests__/registry.test.js b/src/store/__tests__/registry.test.js index fe9d802..e2b3fb0 100644 --- a/src/store/__tests__/registry.test.js +++ b/src/store/__tests__/registry.test.js @@ -20,6 +20,20 @@ describe('StoreRegistry', () => { expect(instance.storeState.getState()).toEqual({ count: 0 }); }); + it('should only return store if no config', () => { + const registry = new StoreRegistry(); + const instance = registry.getStore(StoreMock, 's1', null); + expect(instance).toEqual(null); + }); + + it('should error if not contained', () => { + const registry = new StoreRegistry(); + const ContainedStore = { ...StoreMock, containedBy: jest.fn() }; + expect(() => { + registry.getStore(ContainedStore, 's1'); + }).toThrow(/should be contained/); + }); + it('should say if store exists already', () => { const registry = new StoreRegistry(); expect(registry.hasStore(StoreMock, 's1')).toBe(false); diff --git a/src/store/bind-actions.js b/src/store/bind-actions.js index da49339..ace3654 100644 --- a/src/store/bind-actions.js +++ b/src/store/bind-actions.js @@ -13,50 +13,39 @@ export const bindAction = ( storeState, actionFn, actionKey, - getContainerProps = () => {}, - boundActions = {} + config, + actions ) => { - // Setting mutator name so we can log action name for better debuggability - const dispatch = (thunkFn, actionName = `${actionKey}.dispatch`) => + const callThunk = (instance, thunkFn, actionName) => thunkFn( { setState: defaults.devtools - ? namedMutator(storeState, actionName) - : storeState.mutator, - getState: storeState.getState, + ? namedMutator(instance.storeState, actionName) + : instance.storeState.mutator, + getState: instance.storeState.getState, get actions() { if (!warnings.has(actionFn)) { warnings.set( actionFn, console.warn( `react-sweet-state 'actions' property has been deprecated and will be removed in the next mayor. ` + - `Please check action '${actionName}' of Store '${storeState.key}' and use 'dispatch' instead` + `Please check action '${actionName}' of Store '${instance.storeState.key}' and use 'dispatch' instead` ) ); } - return boundActions; + return actions; }, - dispatch, + dispatch: (tFn) => callThunk(instance, tFn, `${actionName}.dispatch`), }, - getContainerProps() + config.props() ); - return (...args) => dispatch(actionFn(...args), actionKey); + return (...args) => + callThunk({ storeState, actions }, actionFn(...args), actionKey); }; -export const bindActions = ( - actions, - storeState, - getContainerProps = () => ({}), - boundActions = null -) => +export const bindActions = (actions, storeState, config, boundActions = null) => Object.keys(actions).reduce((acc, k) => { - acc[k] = bindAction( - storeState, - actions[k], - k, - getContainerProps, - boundActions || acc - ); + acc[k] = bindAction(storeState, actions[k], k, config, boundActions || acc); return acc; }, {}); diff --git a/src/store/registry.js b/src/store/registry.js index ac43e8f..1314901 100644 --- a/src/store/registry.js +++ b/src/store/registry.js @@ -1,3 +1,4 @@ +import supports from '../utils/supported-features'; import { bindActions } from './bind-actions'; import createStoreState from './create-state'; @@ -10,20 +11,20 @@ export class StoreRegistry { this.defaultScope = defaultScope; } - initStore = (key, Store, fromContainer) => { + initStore = (key, Store, config) => { const { initialState, actions } = Store; - if (Store.containedBy && !fromContainer) { - Promise.reject( - new Error( - `Store ${Store.key} should be contained by a container but it is used globally. ` + - `While it might still work, it will likely cause unexpected behaviours.` - ) + if (Store.containedBy && !config.contained(Store)) { + const err = new Error( + `Store ${Store.key} should be contained by a container but it is used globally. ` + + `While it might still work, it will likely cause unexpected behaviours.` ); + if (supports.scheduling()) Promise.reject(err); + else throw err; } const storeState = createStoreState(key, initialState); - const boundActions = bindActions(actions, storeState); + const boundActions = bindActions(actions, storeState, config); const store = { storeState, actions: boundActions }; this.stores.set(key, store); @@ -35,9 +36,15 @@ export class StoreRegistry { return this.stores.has(key); }; - getStore = (Store, scopeId = this.defaultScope, fromContainer = false) => { + getStore = ( + Store, + scopeId = this.defaultScope, + config = { props: () => ({}), contained: () => false } + ) => { const key = this.generateKey(Store, scopeId); - return this.stores.get(key) || this.initStore(key, Store, fromContainer); + return ( + this.stores.get(key) || (config && this.initStore(key, Store, config)) + ); }; deleteStore = (Store, scopeId = this.defaultScope) => {