Skip to content

Commit

Permalink
perform dynamic analysis to warn about cycles in Effects
Browse files Browse the repository at this point in the history
  • Loading branch information
bcherny committed Jun 1, 2018
1 parent 51000fd commit 961a187
Show file tree
Hide file tree
Showing 9 changed files with 255 additions and 52 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
14 changes: 11 additions & 3 deletions src/index.js.flow
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}>>>
66 changes: 47 additions & 19 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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> =
Expand Down
17 changes: 8 additions & 9 deletions src/react.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)[_])
}
Expand Down
163 changes: 163 additions & 0 deletions test/cyclical-dependencies.tsx
Original file line number Diff line number Diff line change
@@ -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')!)
})
})
4 changes: 4 additions & 0 deletions test/test.flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 961a187

Please sign in to comment.