Skip to content
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

internal: Code cleanup and readability #9

Merged
merged 2 commits into from
Aug 1, 2020
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
8 changes: 8 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ module.exports = {
'react/jsx-uses-react': 'error',
'react/jsx-uses-vars': 'error',
},
overrides: [
{
files: ['**/*.test.*', '**/tests/*.*'],
env: {
jest: true,
},
},
],

ignorePatterns: ['demos/', 'dist/', 'coverage/', 'node_modules/'],
};
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ module.exports = {
},

collectCoverage: true,
coveragePathIgnorePatterns: ['<rootDir>/node_modules/', '<rootDir>/tests/'],
coverageReporters: ['json', 'html'],
};
10 changes: 3 additions & 7 deletions src/SyncReduxToRecoil.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import React from 'react';
import { Store } from 'redux';
import { useSelector, useStore } from 'react-redux';
import { useRecoilState } from 'recoil';

import { ReduxState, internalStateAtom } from './internals';
import { ReduxState, reduxStateAtom, reduxStoreRef } from './internals';

const selectEntireState = (state: ReduxState) => state;
let store: Store | null = null;
const getStore = (): Store | null => store;

export interface SyncReduxToRecoilProps {
enabled?: boolean;
Expand All @@ -16,8 +13,8 @@ export interface SyncReduxToRecoilProps {
const SyncReduxToRecoil: React.FC<SyncReduxToRecoilProps> = (props) => {
const { children, enabled } = props;

store = useStore();
const [lastReduxState, setReduxState] = useRecoilState(internalStateAtom);
reduxStoreRef.c = useStore();
const [lastReduxState, setReduxState] = useRecoilState(reduxStateAtom);

const currentReduxState = useSelector(selectEntireState);
if (enabled && currentReduxState !== lastReduxState) {
Expand All @@ -38,4 +35,3 @@ SyncReduxToRecoil.defaultProps = {
};

export default SyncReduxToRecoil;
export { getStore };
25 changes: 12 additions & 13 deletions src/atomFromRedux.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,40 @@
import { get as getPath } from 'immutable-path';
import { RecoilState, selectorFamily } from 'recoil';

import { DefaultReturnType, internalStateAtom } from './internals';
import {
applyChangesToState,
ChangeEntry,
DefaultReturnType,
applyChangesToObject,
reduxStateAtom,
reduxStoreRef,
syncChangesFromRecoilAction,
} from './syncChangesFromRecoil';
import { getStore } from './SyncReduxToRecoil';
} from './internals';

const atomSelectorCache = Object.create(null);
const { hasOwnProperty } = Object.prototype;

const atomSelectorFamily = selectorFamily({
key: 'redux-to-recoil:atom',
get: (realPath: string) => ({ get }) => {
const reduxState = get(internalStateAtom);
const reduxState = get(reduxStateAtom);
if (realPath) {
return getPath(reduxState, realPath);
}
return reduxState;
},
set: (realPath: string) => ({ get, set }, newValue: unknown) => {
const reduxState = get(internalStateAtom);
const reduxState = get(reduxStateAtom);
const thisChange: ChangeEntry = [realPath, newValue];
// @TODO: Batching support
const allChanges = [thisChange];
const newState = applyChangesToState(reduxState, allChanges);
const newState = applyChangesToObject(reduxState, allChanges);

set(internalStateAtom, newState);
const reduxStore = getStore();
set(reduxStateAtom, newState);
const reduxStore = reduxStoreRef.c;
if (reduxStore) {
reduxStore.dispatch(syncChangesFromRecoilAction(allChanges));
} else if (__DEV__) {
console.error('Cannot dispatch to Redux store because it is not synced');
throw new Error('Cannot dispatch to Redux store because <SyncReduxToRecoil> is not mounted');
}
},
});
Expand All @@ -46,9 +47,7 @@ const atomFromRedux = <ReturnType = DefaultReturnType>(path: string): RecoilStat

if (!hasOwnProperty.call(atomSelectorCache, realPath)) {
// Although named "atomFromRedux", each instance is actually just a selector. They all pull from a single atom.
const selectorForPath = atomSelectorFamily(realPath);

atomSelectorCache[realPath] = selectorForPath;
atomSelectorCache[realPath] = atomSelectorFamily(realPath);
}

return atomSelectorCache[realPath];
Expand Down
19 changes: 19 additions & 0 deletions src/internals/applyChangesToObject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { set as setPath } from 'immutable-path/dist/immutable-object-path';

import { ChangeEntry, ReduxState } from './types';

const applyChangesToObject = (state: ReduxState, changes: Array<ChangeEntry>): ReduxState => {
let newState = state;
for (let i = 0; i < changes.length; i++) {
const [path, value] = changes[i];
if (path) {
newState = setPath(newState, path, value);
} else {
newState = value;
}
}

return newState;
};

export default applyChangesToObject;
13 changes: 11 additions & 2 deletions src/internals/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
export { default as internalStateAtom } from './internalStateAtom';
export * from './internalStateAtom';
export { default as applyChangesToObject } from './applyChangesToObject';
export * from './applyChangesToObject';

export { default as reduxStateAtom } from './reduxStateAtom';
export * from './reduxStateAtom';

export { default as reduxStoreRef } from './reduxStoreRef';
export * from './reduxStoreRef';

export { default as syncChangesFromRecoilAction } from './syncChangesFromRecoilAction';
export * from './syncChangesFromRecoilAction';

export * from './types';
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { atom } from 'recoil';

import { ReduxState } from './types';

const internalStateAtom = atom<ReduxState>({
const reduxStateAtom = atom<ReduxState>({
key: 'redux-to-recoil:state',
default: null,
});

export default internalStateAtom;
export default reduxStateAtom;
8 changes: 8 additions & 0 deletions src/internals/reduxStoreRef.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Store } from 'redux';

// This acts like a React ref, but it's not
const reduxStoreRef: { c: Store | null } = {
c: null,
};

export default reduxStoreRef;
16 changes: 16 additions & 0 deletions src/internals/syncChangesFromRecoilAction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { ChangeEntry } from './types';

const SYNC_CHANGES_FROM_RECOIL = 'SYNC_CHANGES_FROM_RECOIL';

export interface SyncFromRecoilAction {
type: typeof SYNC_CHANGES_FROM_RECOIL;
payload: Array<ChangeEntry>;
}

const syncChangesFromRecoilAction = (changes: Array<ChangeEntry>): SyncFromRecoilAction => ({
type: SYNC_CHANGES_FROM_RECOIL,
payload: changes,
});

export default syncChangesFromRecoilAction;
export { SYNC_CHANGES_FROM_RECOIL };
7 changes: 0 additions & 7 deletions src/internals/typeDeclarations.d.ts

This file was deleted.

9 changes: 8 additions & 1 deletion src/internals/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
// These renames are for readability, and to avoid having to repeatedly disable no-explicit-any when used.
declare global {
// Microbundle handles this
const __DEV__: boolean;
}

export type ChangeEntry = [string, ReduxState];

// These renames are for readability, and to avoid having to repeatedly disable no-explicit-any for each usage.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type DefaultReturnType = any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
14 changes: 6 additions & 8 deletions src/selectorFromReselect.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
import { RecoilValueReadOnly, selector } from 'recoil';

import { internalStateAtom } from './internals';
import { DefaultReturnType, ReduxState } from './internals/types';
import { DefaultReturnType, ReduxState, reduxStateAtom } from './internals';

let count = 0;
let selectorCount = 0;

const selectorFromReselect = <ReturnType = DefaultReturnType>(
selectorFn: (reduxState: ReduxState) => ReturnType,
): RecoilValueReadOnly<ReturnType> => {
count++;
selectorCount++;

const wrappedSelector = selector<ReturnType>({
key: `redux-to-recoil:selector:${count}${selectorFn.name}`,
key: `redux-to-recoil:selector:${selectorCount}${selectorFn.name}`,
get: ({ get }) => {
const reduxState = get(internalStateAtom);
const value = selectorFn(reduxState);
return value;
const reduxState = get(reduxStateAtom);
return selectorFn(reduxState);
},
});

Expand Down
34 changes: 2 additions & 32 deletions src/syncChangesFromRecoil.ts
Original file line number Diff line number Diff line change
@@ -1,45 +1,15 @@
import { set as setPath } from 'immutable-path';
import { Reducer } from 'redux';

import { ReduxState } from './internals/types';

const SYNC_CHANGES_FROM_RECOIL = 'SYNC_CHANGES_FROM_RECOIL';

export type ChangeEntry = [string, ReduxState];

export interface SyncFromRecoilAction {
type: typeof SYNC_CHANGES_FROM_RECOIL;
payload: Array<ChangeEntry>;
}

const syncChangesFromRecoilAction = (changes: Array<ChangeEntry>): SyncFromRecoilAction => ({
type: SYNC_CHANGES_FROM_RECOIL,
payload: changes,
});

const applyChangesToState = (state: ReduxState, changes: Array<ChangeEntry>): ReduxState => {
let newState = state;
for (let i = 0; i < changes.length; i++) {
const [path, value] = changes[i];
if (path) {
newState = setPath(newState, path, value);
} else {
newState = value;
}
}

return newState;
};
import { SYNC_CHANGES_FROM_RECOIL, applyChangesToObject } from './internals';

const syncChangesFromRecoil = (rootReducer: Reducer): Reducer => {
return (state, action) => {
if (action.type === SYNC_CHANGES_FROM_RECOIL) {
return applyChangesToState(state, action.payload);
return applyChangesToObject(state, action.payload);
} else {
return rootReducer(state, action);
}
};
};

export default syncChangesFromRecoil;
export { applyChangesToState, syncChangesFromRecoilAction };
7 changes: 4 additions & 3 deletions tests/helpers.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { ReduxState } from '../src/internals';
import { AnyAction, Store, createStore } from 'redux';
import React from 'react';
import syncChangesFromRecoil from '../src/syncChangesFromRecoil';
import { AnyAction, Store, createStore } from 'redux';
import { Provider } from 'react-redux';

import { ReduxState } from '../src/internals';
import syncChangesFromRecoil from '../src/syncChangesFromRecoil';
import SyncReduxToRecoil from '../src/SyncReduxToRecoil';

const VALUE1_DEFAULT = 100;
Expand Down
10 changes: 9 additions & 1 deletion tests/readState.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-env jest */
import React from 'react';
import { Store } from 'redux';
import { RecoilState, useRecoilValue } from 'recoil';
Expand Down Expand Up @@ -65,6 +64,15 @@ describe('read Redux state through Recoil', () => {
expect(atomWithDot).toStrictEqual(atomWithoutDot);
});

it('caches atomFromRedux instances', () => {
const value1Atom: RecoilState<number> = atomFromRedux<number>('.value1');
const value2Atom: RecoilState<number> = atomFromRedux<number>('.value2');
const value1Atom2: RecoilState<number> = atomFromRedux<number>('.value1');

expect(value1Atom).not.toEqual(value2Atom);
expect(value1Atom).toStrictEqual(value1Atom2);
});

it('always sees the current Redux values', () => {
const value2Atom: RecoilState<number> = atomFromRedux<number>('value2');
const value2AtomHook = () => useRecoilValue(value2Atom);
Expand Down
1 change: 0 additions & 1 deletion tests/selectors.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-env jest */
import React from 'react';
import { Store } from 'redux';
import { Selector } from 'react-redux';
Expand Down
1 change: 0 additions & 1 deletion tests/writeState.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-env jest */
import React from 'react';
import { Store } from 'redux';
import { RecoilState, useRecoilState } from 'recoil';
Expand Down