Skip to content

Commit

Permalink
(persistReducer): timeout (#713)
Browse files Browse the repository at this point in the history
* (persistReducer): timeout

* cleanup

* fix equality?

* smaller

* more
  • Loading branch information
rt2zz authored Feb 10, 2018
1 parent b11bff8 commit 4360500
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 9 deletions.
36 changes: 33 additions & 3 deletions src/persistReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import defaultGetStoredState from './getStoredState'
import purgeStoredState from './purgeStoredState'

type PersistPartial = { _persist: PersistState }
const DEFAULT_TIMEOUT = 5000
/*
@TODO add validation / handling for:
- persisting a reducer which has nested _persist
Expand All @@ -47,6 +48,8 @@ export default function persistReducer<State: Object, Action: Object>(
? autoMergeLevel1
: config.stateReconciler
const getStoredState = config.getStoredState || defaultGetStoredState
const timeout =
config.timeout !== undefined ? config.timeout : DEFAULT_TIMEOUT
let _persistoid = null
let _purge = false
let _paused = true
Expand All @@ -64,6 +67,33 @@ export default function persistReducer<State: Object, Action: Object>(
let restState: State = rest

if (action.type === PERSIST) {
let _sealed = false
let _rehydrate = (payload, err) => {
// only rehydrate if we are not already sealed
!_sealed && action.rehydrate(config.key, payload, err)
if (process.env.NODE_ENV !== 'production' && _sealed)
console.error(
`redux-persist: rehydrate for "${
config.key
}" called after timeout.`,
payload,
err
)
}
timeout &&
setTimeout(() => {
!_sealed &&
_rehydrate(
undefined,
new Error(
`redux-persist: persist timed out for persist key "${
config.key
}"`
)
)
_sealed = true
}, timeout)

// @NOTE PERSIST resumes if paused.
_paused = false

Expand All @@ -87,17 +117,17 @@ export default function persistReducer<State: Object, Action: Object>(
const migrate = config.migrate || ((s, v) => Promise.resolve(s))
migrate(restoredState, version).then(
migratedState => {
action.rehydrate(config.key, migratedState)
_rehydrate(migratedState)
},
migrateErr => {
if (process.env.NODE_ENV !== 'production' && migrateErr)
console.error('redux-persist: migration error', migrateErr)
action.rehydrate(config.key, undefined, migrateErr)
_rehydrate(undefined, migrateErr)
}
)
},
err => {
action.rehydrate(config.key, undefined, err)
_rehydrate(undefined, err)
}
)

Expand Down
13 changes: 8 additions & 5 deletions src/persistStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,12 @@ const persistorReducer = (state = initialState, action) => {

export default function persistStore(
store: Object,
persistorOptions?: PersistorOptions,
options?: ?PersistorOptions,
cb?: BoostrappedCb
): Persistor {
let options: Object = persistorOptions || {}

// help catch incorrect usage of passing PersistConfig in as PersistorOptions
if (process.env.NODE_ENV !== 'production') {
let optionsToTest: Object = options || {}
let bannedKeys = [
'blacklist',
'whitelist',
Expand All @@ -56,15 +55,19 @@ export default function persistStore(
'migrate',
]
bannedKeys.forEach(k => {
if (!!options[k])
if (!!optionsToTest[k])
console.error(
`redux-persist: invalid option passed to persistStore: "${k}". You may be incorrectly passing persistConfig into persistStore, whereas it should be passed into persistReducer.`
)
})
}
let boostrappedCb = cb || false

let _pStore = createStore(persistorReducer, initialState, options.enhancer)
let _pStore = createStore(
persistorReducer,
initialState,
options ? options.enhancer : undefined
)
let register = (key: string) => {
_pStore.dispatch({
type: REGISTER,
Expand Down
1 change: 1 addition & 0 deletions src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type PersistConfig = {
getStoredState?: PersistConfig => Promise<PersistedState>, // used for migrations
debug?: boolean,
serialize?: boolean,
timeout?: number,
}

export type PersistorOptions = {
Expand Down
36 changes: 35 additions & 1 deletion tests/complete.spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

// @flow

import test from 'ava'
Expand All @@ -10,6 +11,7 @@ import { combineReducers, createStore } from 'redux'
import persistReducer from '../src/persistReducer'
import persistStore from '../src/persistStore'
import { createMemoryStorage } from 'storage-memory'
import brokenStorage from './utils/brokenStorage'
import { PERSIST, REHYDRATE } from '../src/constants'
import sleep from './utils/sleep'

Expand All @@ -19,6 +21,7 @@ const config = {
version: 1,
storage: createMemoryStorage(),
debug: true,
timeout: 5,
}

test('multiple persistReducers work together', t => {
Expand All @@ -28,9 +31,40 @@ test('multiple persistReducers work together', t => {
const rootReducer = combineReducers({ r1, r2 })
const store = createStore(rootReducer)
const persistor = persistStore(store, {}, () => {
console.log(store.getState(), persistor.getState())
t.is(persistor.getState().bootstrapped, true)
resolve()
})
})
})

test('persistStore timeout 0 never bootstraps', t => {
return new Promise((resolve, reject) => {
let r1 = persistReducer({...config, storage: brokenStorage, timeout: 0}, reducer)
const rootReducer = combineReducers({ r1 })
const store = createStore(rootReducer)
const persistor = persistStore(store, null, () => {
console.log('resolve')
reject()
})
setTimeout(() => {
t.is(persistor.getState().bootstrapped, false)
resolve()
}, 10)
})
})


test('persistStore timeout forces bootstrap', t => {
return new Promise((resolve, reject) => {
let r1 = persistReducer({...config, storage: brokenStorage}, reducer)
const rootReducer = combineReducers({ r1 })
const store = createStore(rootReducer)
const persistor = persistStore(store, null, () => {
t.is(persistor.getState().bootstrapped, true)
resolve()
})
setTimeout(() => {
reject()
}, 10)
})
})
14 changes: 14 additions & 0 deletions tests/utils/brokenStorage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// @flow

export default {
getItem(): Promise<void> {
return new Promise((resolve: Function, reject: Function) => {})
},
setItem(): Promise<void> {
return new Promise((resolve: Function, reject: Function) => {})
},
removeItem(): Promise<void> {
return new Promise((resolve: Function, reject: Function) => {})
}
}

4 comments on commit 4360500

@rosskhanas
Copy link

Choose a reason for hiding this comment

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

@rt2zz it seems like a new code always throws an exception. A _sealed property is set to true only after the error is thrown and the store is rehydrated again with an error, which is probably not ok. Could you please check this?

@rt2zz
Copy link
Owner Author

@rt2zz rt2zz commented on 4360500 Feb 12, 2018

Choose a reason for hiding this comment

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

my mistake, tests do not cover this :/

This should be resolved now in v5.7.1

@rosskhanas
Copy link

Choose a reason for hiding this comment

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

Thank you!

@sibelius
Copy link

Choose a reason for hiding this comment

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

5.7.1 is broke as well

Please sign in to comment.