Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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(() => {});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is not used anymore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to silence React using console.error when we throw an error during render 😒
https://stackoverflow.com/a/66329699

const Store1 = createStore({
name: 'one',
initialState: { todos: [], loading: false },
Expand All @@ -483,12 +481,10 @@ describe('Integration', () => {
});

const Subscriber = createSubscriber(Store1);
render(<Subscriber>{() => null}</Subscriber>);
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(<Subscriber>{() => null}</Subscriber>);
}).toThrow(/should be contained/);
errorSpy.mockRestore();
});
});
6 changes: 3 additions & 3 deletions src/components/__tests__/container.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -267,7 +267,7 @@ describe('Container', () => {
const children = <Subscriber>{renderPropChildren}</Subscriber>;
render(<Container defaultCount={5}>{children}</Container>);
const [, { increase }] = renderPropChildren.mock.calls[0];
increase();
act(() => increase());

expect(actionInner).toHaveBeenCalledWith(expect.any(Object), {
defaultCount: 5,
Expand All @@ -285,7 +285,7 @@ describe('Container', () => {
);
const [, { increase }] = renderPropChildren.mock.calls[0];
rerender(<Container defaultCount={6}>{children}</Container>);
increase();
act(() => increase());

expect(actionInner).toHaveBeenCalledWith(expect.any(Object), {
defaultCount: 6,
Expand Down
57 changes: 25 additions & 32 deletions src/components/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) => {
Expand All @@ -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 = {
Expand All @@ -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]
);
}
Expand All @@ -294,6 +286,7 @@ function createFunctionContainer({ displayName, override } = {}) {
scope,
registry,
restProps,
check,
override
);
const api = useApi(check, getContainedStore, ctx);
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/components/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { defaultRegistry } from './store';
export const Context = React.createContext(
{
globalRegistry: defaultRegistry,
getStore: defaultRegistry.getStore,
retrieveStore: (Store) => defaultRegistry.getStore(Store),
},
() => 0
);
25 changes: 20 additions & 5 deletions src/store/__tests__/bind-actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -16,7 +21,7 @@ describe('bindAction', () => {
storeStateMock,
action,
'myAction',
() => containerProps,
createConfigArg({ props: containerProps }),
actionsMock
);
result(1, '2', 3);
Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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),
Expand All @@ -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);
Expand Down
14 changes: 14 additions & 0 deletions src/store/__tests__/registry.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
39 changes: 14 additions & 25 deletions src/store/bind-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}, {});
Loading