perform dynamic analysis to warn about cycles in Effects

Author: bcherny

package.json

@@ -25,7 +25,7 @@
     "test": "ava && flow focus-check ./test/test.flow.js && ./test/bad-cases/run.sh"
   },
   "dependencies": {
-    "typed-rx-emitter": "^1.1.1"
+    "typed-rx-emitter": "^1.1.2"
   },
   "devDependencies": {
     "@types/jsdom": "^11.0.4",

src/index.js.flow

@@ -12,6 +12,10 @@ type Lifted<State, T> = {
   value: T
 }
 
+export type Options = {
+  isDevMode: boolean
+}
+
 export interface Store<State: Object> {
   get<K: $Keys<State>>(key: K): $ElementType<State, K>;
   set<K: $Keys<State>>(key: K): (value: $ElementType<State, K>) => void;
@@ -33,17 +37,21 @@ declare export class StoreDefinition<State: Object> implements Store<State> {
   set<K: $Keys<State>>(key: K): (value: $ElementType<State, K>) => void;
   on<K: $Keys<State>>(key: K): Observable<$ElementType<State, K>>;
   onAll(): Observable<$Values<Undux<State>>>;
+  getCurrentSnapshot(): StoreSnapshot<State>;
   getState(): $ReadOnly<State>;
 }
 
-declare export function createStore<State: Object>(initialState: State): StoreDefinition<State>
+declare export function createStore<State: Object>(
+  initialState: State,
+  options?: Options
+): StoreDefinition<State>
 export type Plugin<State: Object> = (store: StoreDefinition<State>) => StoreDefinition<State>
 declare export var withLogger: Plugin<Object>
 declare export var withReduxDevtools: Plugin<Object>
 
 declare export function connect<State: Object>(
   store: StoreDefinition<State>
-): <Props: {store: Store<State>}>(
+): <S: Store<State>, Props: {store: S}>(
   Component: React.ComponentType<Props>
 ) =>
-  Class<React.Component<$Diff<Props, {store: Store<State>}>>>
+  Class<React.Component<$Diff<Props, {store: S}>>>

src/index.ts

@@ -2,6 +2,11 @@ import * as RxJS from 'rxjs'
 import { Emitter } from 'typed-rx-emitter'
 import { mapValues } from './utils'
 
+const CYCLE_ERROR_MESSAGE = '[undux] Error: Cyclical dependency detected. '
+  + 'This may cause a stack overflow unless you fix it. \n'
+  + 'The culprit is the following sequence of .set calls, '
+  + 'called from one or more of your Undux Effects: '
+
 export type Undux<State extends object> = {
   [K in keyof State]: {
     key: K
@@ -14,50 +19,69 @@ export interface Store<State extends object> {
   get<K extends keyof State>(key: K): State[K]
   set<K extends keyof State>(key: K): (value: State[K]) => void
   on<K extends keyof State>(key: K): RxJS.Observable<State[K]>
-  onAll<K extends keyof State>(): RxJS.Observable<Undux<State>[keyof State]>
+  onAll(): RxJS.Observable<Undux<State>[keyof State]>
   getState(): Readonly<State>
 }
 
 export class StoreSnapshot<State extends object> implements Store<State> {
   constructor(
     private state: State,
-    private store: StoreDefinition<State>
+    private storeDefinition: StoreDefinition<State>
   ) { }
   get<K extends keyof State>(key: K) {
     return this.state[key]
   }
   set<K extends keyof State>(key: K) {
-    return this.store.set(key)
+    return this.storeDefinition.set(key)
   }
   on<K extends keyof State>(key: K) {
-    return this.store.on(key)
+    return this.storeDefinition.on(key)
   }
-  onAll<K extends keyof State>() {
-    return this.store.onAll()
+  onAll() {
+    return this.storeDefinition.onAll()
   }
   getState() {
     return Object.freeze(this.state)
   }
 }
 
+export type Options = {
+  isDevMode: boolean
+}
+
+let DEFAULT_OPTIONS: Readonly<Options> = {
+  isDevMode: false
+}
+
 export class StoreDefinition<State extends object> implements Store<State> {
-  private store: StoreSnapshot<State>
-  private alls: Emitter<Undux<State>> = new Emitter
-  private emitter: Emitter<State> = new Emitter
+  private storeSnapshot: StoreSnapshot<State>
+  private alls: Emitter<Undux<State>>
+  private emitter: Emitter<State>
   private setters: {
     readonly [K in keyof State]: (value: State[K]) => void
   }
-  constructor(state: State) {
+  constructor(state: State, options: Options) {
+
+    let emitterOptions = {
+      isDevMode: options.isDevMode,
+      onCycle(chain: (string | number | symbol)[]) {
+        console.error(CYCLE_ERROR_MESSAGE + chain.join(' -> '))
+      }
+    }
+
+    // Initialize emitters
+    this.alls = new Emitter(emitterOptions)
+    this.emitter = new Emitter(emitterOptions)
 
     // Set initial state
-    this.store = new StoreSnapshot(state, this)
+    this.storeSnapshot = new StoreSnapshot(state, this)
 
     // Cache setters
     this.setters = mapValues(state, (v, key) =>
       (value: typeof v) => {
-        let previousValue = this.store.get(key)
-        this.store = new StoreSnapshot(
-          Object.assign({}, this.store.getState(), { [key]: value }),
+        let previousValue = this.storeSnapshot.get(key)
+        this.storeSnapshot = new StoreSnapshot(
+          Object.assign({}, this.storeSnapshot.getState(), { [key]: value }),
           this
         )
         this.emitter.emit(key, value)
@@ -68,24 +92,28 @@ export class StoreDefinition<State extends object> implements Store<State> {
   on<K extends keyof State>(key: K): RxJS.Observable<State[K]> {
     return this.emitter.on(key)
   }
-  onAll<K extends keyof State>(): RxJS.Observable<Undux<State>[keyof State]> {
+  onAll(): RxJS.Observable<Undux<State>[keyof State]> {
     return this.alls.all()
   }
   get<K extends keyof State>(key: K) {
-    return this.store.get(key)
+    return this.storeSnapshot.get(key)
   }
   set<K extends keyof State>(key: K) {
     return this.setters[key]
   }
+  getCurrentSnapshot() {
+    return this.storeSnapshot
+  }
   getState() {
-    return this.store.getState()
+    return this.storeSnapshot.getState()
   }
 }
 
 export function createStore<State extends object>(
-  initialState: State
+  initialState: State,
+  options: Options = DEFAULT_OPTIONS
 ): StoreDefinition<State> {
-  return new StoreDefinition<State>(initialState)
+  return new StoreDefinition<State>(initialState, options)
 }
 
 export type Plugin<State extends object> =

src/react.tsx

@@ -4,37 +4,36 @@ import { Subscription } from 'rxjs'
 import { Store, StoreDefinition, StoreSnapshot } from './'
 import { equals, getDisplayName } from './utils'
 
-export type Diff<T extends string, U extends string> = ({ [P in T]: P } & { [P in U]: never } & { [x: string]: never })[T]
-export type Omit<T, K extends keyof T> = { [P in Diff<keyof T, K>]: T[P] }
+export type Diff<T, U> = Pick<T, Exclude<keyof T, keyof U>>
 
 export function connect<StoreState extends object>(store: StoreDefinition<StoreState>) {
   return function <
     Props,
     PropsWithStore extends { store: Store<StoreState> } & Props = { store: Store<StoreState> } & Props
-    >(
-      Component: React.ComponentType<PropsWithStore>
-    ): React.ComponentClass<Omit<PropsWithStore, 'store'>> {
+  >(
+    Component: React.ComponentType<PropsWithStore>
+  ): React.ComponentClass<Diff<PropsWithStore, { store: Store<StoreState> }>> {
 
     type State = {
       store: StoreSnapshot<StoreState>
       subscription: Subscription
     }
 
-    return class extends React.Component<Omit<PropsWithStore, 'store'>, State> {
+    return class extends React.Component<Diff<PropsWithStore, { store: Store<StoreState> }>, State> {
       static displayName = `withStore(${getDisplayName(Component)})`
       state = {
-        store: store['store'],
+        store: store.getCurrentSnapshot(),
         subscription: store.onAll().subscribe(({ key, previousValue, value }) => {
           if (equals(previousValue, value)) {
             return false
           }
-          this.setState({ store: store['store'] })
+          this.setState({ store: store.getCurrentSnapshot() })
         })
       }
       componentWillUnmount() {
         this.state.subscription.unsubscribe()
       }
-      shouldComponentUpdate(props: Omit<PropsWithStore, 'store'>, state: State) {
+      shouldComponentUpdate(props: Readonly<Diff<PropsWithStore, { store: Store<StoreState> }>>, state: State) {
         return state.store !== this.state.store
           || Object.keys(props).some(_ => (props as any)[_] !== (this.props as any)[_])
       }

test/cyclical-dependencies.tsx

@@ -0,0 +1,163 @@
+import { test } from 'ava'
+import * as React from 'react'
+import { Simulate } from 'react-dom/test-utils'
+import { connect, createStore, Store } from '../src'
+import { withElement } from './testUtils'
+
+test('[cyclical dependencies] it should show a console error when there is a cycle (1)', t => {
+  t.plan(1)
+  console.error = (e: string) =>
+    t.regex(e, /Cyclical dependency detected/)
+  let store = createStore({ a: 1 }, { isDevMode: true })
+  store.on('a').subscribe(a =>
+    store.set('a')(a)
+  )
+  store.set('a')(2)
+})
+
+test('[cyclical dependencies] it should show a console error when there is a cycle (2)', t => {
+  t.plan(1)
+  console.error = (e: string) =>
+    t.regex(e, /Cyclical dependency detected/)
+  let store = createStore({ a: 1, b: 2 }, { isDevMode: true })
+  store.on('a').subscribe(a =>
+    store.set('b')(a)
+  )
+  store.on('b').subscribe(a =>
+    store.set('a')(a)
+  )
+  store.set('a')(2)
+})
+
+test('[cyclical dependencies] it should show a console error when there is a cycle (3)', t => {
+  t.plan(1)
+  console.error = (e: string) =>
+    t.regex(e, /Cyclical dependency detected/)
+  let store = createStore({ a: 1, b: 2 }, { isDevMode: true })
+  store.on('a').subscribe(a =>
+    store.set('b')(a)
+  )
+  store.on('b').subscribe(a =>
+    store.set('a')(a)
+  )
+  store.set('b')(3)
+})
+
+test('[cyclical dependencies] it should show a console error when there is a cycle (4)', t => {
+
+  t.plan(1)
+  console.error = (e: string) =>
+    t.regex(e, /Cyclical dependency detected/)
+
+  let store = createStore({ a: 1 }, { isDevMode: true })
+  let withStore = connect(store)
+
+  type State = {
+    a: number
+  }
+
+  type Props = {
+    store: Store<State>
+  }
+
+  store.on('a').subscribe(a =>
+    store.set('a')(2)
+  )
+
+  let A = withStore(({ store }: Props) =>
+    <button onClick={() => store.set('a')(2)} />
+  )
+
+  withElement(A, _ => {
+    Simulate.click(_.querySelector('button')!)
+  })
+})
+
+test('[cyclical dependencies] it should show a console error when there is a cycle (5)', t => {
+
+  t.plan(1)
+  console.error = (e: string) =>
+    t.regex(e, /Cyclical dependency detected/)
+
+  let storeA = createStore({ a: 1 }, { isDevMode: true })
+  let storeB = createStore({ b: 2 }, { isDevMode: true })
+  let withStoreA = connect(storeA)
+  let withStoreB = connect(storeB)
+
+  storeA.on('a').subscribe(a =>
+    storeB.set('b')(a)
+  )
+  storeB.on('b').subscribe(b =>
+    storeA.set('a')(b)
+  )
+
+  type StateA = {
+    a: number
+  }
+  type StateB = {
+    b: number
+  }
+
+  type PropsA = {
+    store: Store<StateA>
+  }
+  type PropsB = {
+    store: Store<StateB>
+  }
+
+  let A = withStoreA(() =>
+    <B />
+  )
+  let B = withStoreB(({ store: storeB }: PropsB) =>
+    <button onClick={() => storeB.set('b')(1)} />
+  )
+
+  withElement(A, _ => {
+    Simulate.click(_.querySelector('button')!)
+  })
+})
+
+test('[cyclical dependencies] it should show a console error when there is a cycle (6)', t => {
+
+  t.plan(1)
+  console.error = (e: string) =>
+    t.regex(e, /Cyclical dependency detected/)
+
+  let storeA = createStore({ a: 1 }, { isDevMode: true })
+  let storeB = createStore({ b: 2 }, { isDevMode: true })
+  let withStoreA = connect(storeA)
+  let withStoreB = connect(storeB)
+
+  storeA.on('a').subscribe(a =>
+    storeB.set('b')(a)
+  )
+  storeB.on('b').subscribe(b =>
+    storeA.set('a')(b)
+  )
+
+  type StateA = {
+    a: number
+  }
+  type StateB = {
+    b: number
+  }
+
+  type PropsA = {
+    store: Store<StateA>
+  }
+  type PropsB = {
+    store: Store<StateB>
+    storeA: Store<StateA>
+  }
+
+  let A = withStoreA(({ store }: PropsA) =>
+    <B storeA={store} />
+  )
+  let B = withStoreB(({ storeA }: PropsB) =>
+    <button onClick={() => storeA.set('a')(1)} />
+  )
+
+  withElement(A, _ => {
+    Simulate.click(_.querySelector('button')!)
+  })
+})

test/test.flow.js

@@ -30,6 +30,10 @@ let withEffects: Plugin<State> = store => {
 }
 
 let store = withEffects(withReduxDevtools(withLogger(createStore(initialState))))
+let debugStore = createStore(initialState, { isDevMode: true })
+
+let state = store.getState().users.concat(4)
+let snapshot = store.getCurrentSnapshot().get('users').concat(4)
 
 type Props = {|
   foo: number,