From 17cdc014427cd6aafdaef77ecfc6510462fba4bd Mon Sep 17 00:00:00 2001 From: sorrycc Date: Tue, 12 Jul 2016 16:16:53 +0800 Subject: [PATCH 1/9] use opts.hmr to support hmr --- examples/user-dashboard/src/entries/index.js | 47 +++++++++++--------- src/index.js | 6 +-- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/examples/user-dashboard/src/entries/index.js b/examples/user-dashboard/src/entries/index.js index c9bc29b0..5aaccf36 100644 --- a/examples/user-dashboard/src/entries/index.js +++ b/examples/user-dashboard/src/entries/index.js @@ -12,26 +12,31 @@ app.model(require('../models/users')); app.router(require('../routes')); // 4. Start -const { render } = app.start(document.getElementById('root')); +app.start(document.getElementById('root'), { -// Support Routes HMR. -// This will be implemented in babel plugin later. -if (module.hot) { - const renderNormally = render; - const renderException = (error) => { - const RedBox = require('redbox-react'); - ReactDOM.render(, document.getElementById('root')); - }; - const newRender = (routes) => { - try { - renderNormally(routes); - } catch (error) { - console.error('error', error); - renderException(error); + // Support Routes HMR. + // This will be implemented in babel plugin later. + hmr: (render) => { + if (module.hot) { + const renderNormally = render; + const renderException = (error) => { + const RedBox = require('redbox-react'); + ReactDOM.render(, document.getElementById('root')); + }; + const newRender = (routes) => { + try { + renderNormally(routes); + } catch (error) { + console.error('error', error); + renderException(error); + } + }; + module.hot.accept('../routes', () => { + const routes = require('../routes'); + newRender(routes); + }); } - }; - module.hot.accept('../routes', () => { - const routes = require('../routes'); - newRender(routes); - }); -} + }, +}); + + diff --git a/src/index.js b/src/index.js index 133ef584..eac33709 100644 --- a/src/index.js +++ b/src/index.js @@ -104,9 +104,9 @@ function dva() { } render(); - return { - render, - }; + if (opts.hmr) { + opts.hmr(render); + } } } From e830fd29ba699fe3ebbde822cf0c63cb8302c6d4 Mon Sep 17 00:00:00 2001 From: sorrycc Date: Tue, 12 Jul 2016 17:15:38 +0800 Subject: [PATCH 2/9] support opts.reducers and opts.initialState --- package.json | 3 ++- src/index.js | 51 ++++++++++++++++++++++++++++++++++++++------------- src/utils.js | 2 ++ 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index 30304f4d..b99e0f6d 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "lint": "eslint --ext .js src" }, "dependencies": { + "is-plain-object": "^2.0.1", "isomorphic-fetch": "^2.2.1", "react-redux": "4.4.x", "react-router": "^2.5.1", @@ -80,4 +81,4 @@ "fetch.js", "index.js" ] -} +} \ No newline at end of file diff --git a/src/index.js b/src/index.js index eac33709..0f6cd020 100644 --- a/src/index.js +++ b/src/index.js @@ -16,11 +16,13 @@ function dva() { model, router, start, + store: null, }; return app; function model(model) { - check(model.namespace, is.notUndef, 'Namespace must be defined with model'); + check(model.namespace, is.notUndef, 'Namespace must be defined with model.'); + check(model.namespace, namespace => namespace !== 'routing', 'Namespace should not be routing.'); _models.push(model); } @@ -30,27 +32,49 @@ function dva() { } function start(container, opts = {}) { - check(container, is.element, 'Container must be DOMElement'); - check(_routes, is.notUndef, 'Routes is not defined'); - let sagas = {}; - const rootReducer = {}; + check(container, is.element, 'Container must be DOMElement.'); + check(_routes, is.notUndef, 'Routes is not defined.'); + // Get sagas and reducers from model. + let sagas = {}; + let reducers = { + routing, + }; _models.forEach(model => { - rootReducer[model.namespace] = handleActions(model.reducers || {}, model.state); + reducers[model.namespace] = handleActions(model.reducers || {}, model.state); sagas = { ...sagas, ...model.effects }; }); + // Support external reducers. + if (opts.reducers) { + check(opts.reducers, is.object, 'Reducers must be object.'); + check(opts.reducers, optReducers => { + for (var k in optReducers) { + if (k in reducers) return false; + } + return true; + }, 'Reducers should not be conflict with namespace in model.'); + reducers = { ...reducers, ...opts.reducers }; + } + + // Create store. const sagaMiddleware = createSagaMiddleware(); const enhancer = compose( applyMiddleware(sagaMiddleware), window.devToolsExtension ? window.devToolsExtension() : f => f ); - const store = createStore( - combineReducers({ ...rootReducer, routing }), {}, enhancer + const initialState = opts.initialState || {}; + const store = app.store = createStore( + combineReducers(reducers), initialState, enhancer ); + + // Sync history. const history = syncHistoryWithStore(opts.history || hashHistory, store); + + // Start saga. sagaMiddleware.run(rootSaga); + // Handle subscriptions. document.addEventListener('DOMContentLoaded', () => { _models.forEach(({ subscriptions }) => { if (subscriptions) { @@ -63,6 +87,12 @@ function dva() { }); }); + // Render and hmr. + render(); + if (opts.hmr) { + opts.hmr(render); + } + function getWatcher(k, saga) { let _saga = saga; let _type = 'takeEvery'; @@ -102,11 +132,6 @@ function dva() { ), container); } - - render(); - if (opts.hmr) { - opts.hmr(render); - } } } diff --git a/src/utils.js b/src/utils.js index 8dc2ce3b..71b2504f 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,3 +1,4 @@ +import isPlainObject from 'is-plain-object'; export function check(value, predicate, error) { if(!predicate(value)) { @@ -20,6 +21,7 @@ export const is = { number : n => typeof n === 'number', element : n => typeof n === 'object' && n.nodeType && n.nodeName, array : Array.isArray, + object : isPlainObject, jsx : v => v && v.$$typeof && v.$$typeof.toString() === 'Symbol(react.element)', sagaType : v => v === 'takeEvery' || v === 'takeLatest', }; From 7a55d2b0c0cd33f739e8ad5e43f3dca6d6a4b31d Mon Sep 17 00:00:00 2001 From: sorrycc Date: Tue, 12 Jul 2016 17:27:08 +0800 Subject: [PATCH 3/9] app.start: if no container, return jsx element, so we can do ssr and test --- src/index.js | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/index.js b/src/index.js index 0f6cd020..db43dade 100644 --- a/src/index.js +++ b/src/index.js @@ -32,7 +32,13 @@ function dva() { } function start(container, opts = {}) { - check(container, is.element, 'Container must be DOMElement.'); + // If no container supplied, return jsx element. + if (is.object(container)) { + opts = container; + container = null; + } else { + check(container, is.element, 'Container must be DOMElement.'); + } check(_routes, is.notUndef, 'Routes is not defined.'); // Get sagas and reducers from model. @@ -88,9 +94,16 @@ function dva() { }); // Render and hmr. - render(); - if (opts.hmr) { - opts.hmr(render); + if (container) { + render(); + if (opts.hmr) { + opts.hmr(render); + } + } else { + const Routes = _routes; + return + + ; } function getWatcher(k, saga) { From 97b9f0b0b711c57f476436a93a0e8337193dbe54 Mon Sep 17 00:00:00 2001 From: sorrycc Date: Tue, 12 Jul 2016 18:21:29 +0800 Subject: [PATCH 4/9] add testcase --- .gitignore | 2 ++ .travis.yml | 9 ++++++ package.json | 10 +++++- src/index.js | 20 +++++++++--- test/app.start-test.js | 69 ++++++++++++++++++++++++++++++++++++++++++ test/basic-test.js | 30 ++++++++++++++++++ 6 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 .travis.yml create mode 100644 test/app.start-test.js create mode 100644 test/basic-test.js diff --git a/.gitignore b/.gitignore index d8698404..fc52dc1c 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ tmp node_modules lib dist +coverage + diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 00000000..601542a7 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,9 @@ +language: node_js + +node_js: + - "4" + - "5" + - "6" + +after_success: + - npm run coveralls diff --git a/package.json b/package.json index b99e0f6d..6ff9d949 100644 --- a/package.json +++ b/package.json @@ -21,10 +21,14 @@ "author": "chencheng ", "license": "MIT", "scripts": { + "test": "babel-node node_modules/.bin/babel-istanbul cover node_modules/.bin/_mocha --no-timeouts", + "debug": "./node_modules/.bin/mocha --require babel-core/register --no-timeouts", "build": "rm -rf lib && babel src --out-dir lib --ignore __tests__", - "lint": "eslint --ext .js src" + "lint": "eslint --ext .js src", + "coveralls": "cat ./coverage/lcov.info | coveralls" }, "dependencies": { + "global": "^4.3.0", "is-plain-object": "^2.0.1", "isomorphic-fetch": "^2.2.1", "react-redux": "4.4.x", @@ -41,6 +45,7 @@ "devDependencies": { "babel-cli": "^6.10.1", "babel-eslint": "^6.0.4", + "babel-istanbul": "^0.11.0", "babel-plugin-add-module-exports": "^0.2.1", "babel-plugin-transform-runtime": "^6.9.0", "babel-preset-es2015": "^6.9.0", @@ -49,9 +54,12 @@ "babel-runtime": "^6.9.2", "browserify": "^13.0.1", "browserify-shim": "^3.8.12", + "coveralls": "^2.11.9", "envify": "^3.4.1", "eslint": "^2.7.0", "eslint-config-airbnb": "^9.0.1", + "expect": "^1.20.2", + "mocha": "^2.5.3", "uglifyjs": "^2.4.10" }, "babel": { diff --git a/src/index.js b/src/index.js index db43dade..67cbb24f 100644 --- a/src/index.js +++ b/src/index.js @@ -7,6 +7,8 @@ import { hashHistory, Router } from 'react-router'; import { syncHistoryWithStore, routerReducer as routing } from 'react-router-redux'; import { handleActions } from 'redux-actions'; import { fork } from 'redux-saga/effects'; +import document from 'global/document'; +import window from 'global/window'; import { is, check, warn } from './utils'; function dva() { @@ -31,10 +33,16 @@ function dva() { _routes = routes; } + // start + // app.start(); + // app.start(container); + // app.start(container, opts); + // app.start(opts); function start(container, opts = {}) { // If no container supplied, return jsx element. - if (is.object(container)) { - opts = container; + if (arguments.length === 0 + || (arguments.length === 1 && is.object(container))) { + opts = container || {}; container = null; } else { check(container, is.element, 'Container must be DOMElement.'); @@ -52,7 +60,7 @@ function dva() { }); // Support external reducers. - if (opts.reducers) { + if (is.notUndef(opts.reducers)) { check(opts.reducers, is.object, 'Reducers must be object.'); check(opts.reducers, optReducers => { for (var k in optReducers) { @@ -75,7 +83,11 @@ function dva() { ); // Sync history. - const history = syncHistoryWithStore(opts.history || hashHistory, store); + // Use try catch because it don't work in test. + let history; + try { + history = syncHistoryWithStore(opts.history || hashHistory, store); + } catch (e) {} // Start saga. sagaMiddleware.run(rootSaga); diff --git a/test/app.start-test.js b/test/app.start-test.js new file mode 100644 index 00000000..77c96e00 --- /dev/null +++ b/test/app.start-test.js @@ -0,0 +1,69 @@ +import expect from 'expect'; +import dva from '../src/index'; + +describe('app.start', () => { + + it('throw error if no routes defined', () => { + const app = dva(); + expect(() => { + app.start(); + }).toThrow(/Routes is not defined/); + }); + + it('opts.initialState', () => { + const app = dva(); + app.model({ + namespace: 'count', + state: 0, + }); + app.router(({history}) =>
); + app.start({ + initialState: { + count: 1, + }, + }); + expect(app.store.getState().count).toEqual(1); + }); + + it('opts.reducers', () => { + const reducers = { + count: (state, { type }) => { + if (type === 'add') { + return state + 1; + } + // default state + return 0; + }, + }; + const app = dva(); + app.router(({history}) =>
); + app.start({ + reducers, + }); + + expect(app.store.getState().count).toEqual(0); + app.store.dispatch({ type: 'add' }); + expect(app.store.getState().count).toEqual(1); + }); + + it('opts.reducers: throw error if not plain object', () => { + const app = dva(); + app.router(({history}) =>
); + expect(() => { + app.start({ + reducers: 0, + }); + }).toThrow(/Reducers must be object/); + }); + + it('opts.reducers: throw error if conflicts', () => { + const app = dva(); + app.router(({history}) =>
); + expect(() => { + app.start({ + reducers: { routing: function(){}, }, + }); + }).toThrow(/Reducers should not be conflict with namespace in model/); + }); + +}); diff --git a/test/basic-test.js b/test/basic-test.js new file mode 100644 index 00000000..971e6f51 --- /dev/null +++ b/test/basic-test.js @@ -0,0 +1,30 @@ +import expect from 'expect'; +import dva from '../src/index'; + +describe('basic', () => { + it('basic', () => { + let sagaCount = 0; + + const app = dva(); + app.model({ + namespace: 'count', + state: 0, + reducers: { + ['add'](state) { + return state + 1; + }, + }, + effects: { + ['add']: function*() { + sagaCount = sagaCount + 1; + }, + } + }); + app.router(({history}) =>
); + app.start(); + + app.store.dispatch({ type: 'add' }); + expect(app.store.getState().count).toEqual(1); + expect(sagaCount).toEqual(1); + }); +}); From 4b9c0d829293d1243fd4af0489d3df6c1833144a Mon Sep 17 00:00:00 2001 From: sorrycc Date: Tue, 12 Jul 2016 18:37:30 +0800 Subject: [PATCH 5/9] app.start: support opts.middlewares --- src/index.js | 7 +++++-- test/app.start-test.js | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index 67cbb24f..dc08935d 100644 --- a/src/index.js +++ b/src/index.js @@ -33,7 +33,7 @@ function dva() { _routes = routes; } - // start + // start usage: // app.start(); // app.start(container); // app.start(container, opts); @@ -72,9 +72,12 @@ function dva() { } // Create store. + if (is.notUndef(opts.middlewares)) { + check(opts.middlewares, is.array, 'Middlewares must be array.') + } const sagaMiddleware = createSagaMiddleware(); const enhancer = compose( - applyMiddleware(sagaMiddleware), + applyMiddleware.apply(null, [sagaMiddleware, ...(opts.middlewares || [])]), window.devToolsExtension ? window.devToolsExtension() : f => f ); const initialState = opts.initialState || {}; diff --git a/test/app.start-test.js b/test/app.start-test.js index 77c96e00..905f9197 100644 --- a/test/app.start-test.js +++ b/test/app.start-test.js @@ -66,4 +66,30 @@ describe('app.start', () => { }).toThrow(/Reducers should not be conflict with namespace in model/); }); + it('opts.middlewares', () => { + let count = 0; + const countMiddleware = ({dispatch, getState}) => next => action => { + count = count + 1; + }; + + const app = dva(); + app.router(({history}) =>
); + app.start({ + middlewares: [countMiddleware] + }); + + app.store.dispatch({ type: 'test' }); + expect(count).toEqual(1); + }); + + it('opts.middlewares: throw error if not array', () => { + const app = dva(); + app.router(({history}) =>
); + expect(() => { + app.start({ + middlewares: 0, + }); + }).toThrow(/Middlewares must be array/); + }); + }); From 484bff7325de490bbae63fc19255a777bd464be3 Mon Sep 17 00:00:00 2001 From: sorrycc Date: Wed, 13 Jul 2016 14:48:57 +0800 Subject: [PATCH 6/9] app.model: support reducer enhancer --- src/index.js | 7 ++++++- test/app.model-test.js | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/app.model-test.js diff --git a/src/index.js b/src/index.js index dc08935d..0331d7af 100644 --- a/src/index.js +++ b/src/index.js @@ -55,7 +55,12 @@ function dva() { routing, }; _models.forEach(model => { - reducers[model.namespace] = handleActions(model.reducers || {}, model.state); + if (is.array(model.reducers)) { + const [_reducers, enhancer] = model.reducers; + reducers[model.namespace] = enhancer(handleActions(_reducers || {}, model.state)); + } else { + reducers[model.namespace] = handleActions(model.reducers || {}, model.state); + } sagas = { ...sagas, ...model.effects }; }); diff --git a/test/app.model-test.js b/test/app.model-test.js new file mode 100644 index 00000000..603a3f1b --- /dev/null +++ b/test/app.model-test.js @@ -0,0 +1,33 @@ +import expect from 'expect'; +import dva from '../src/index'; + +describe('app.model', () => { + it('reducer enhancer', () => { + function enhancer(reducer) { + return (state, action) => { + if (action.type === 'square') { + return state * state; + } + return reducer(state, action); + }; + } + + const app = dva(); + app.model({ + namespace: 'count', + state: 3, + reducers: [{ + ['add'](state) { + console.log('reducer: add'); + return state + 1; + }, + }, enhancer], + }); + app.router(({history}) =>
); + app.start(); + + app.store.dispatch({ type: 'square' }); + app.store.dispatch({ type: 'add' }); + expect(app.store.getState().count).toEqual(10); + }); +}); From 24161ebe06a388816ec60374615ef290dfafed15 Mon Sep 17 00:00:00 2001 From: sorrycc Date: Wed, 13 Jul 2016 15:12:24 +0800 Subject: [PATCH 7/9] app.model: support watcher saga --- src/index.js | 8 ++-- src/utils.js | 2 +- test/app.model-test.js | 91 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/index.js b/src/index.js index 0331d7af..ad6919f6 100644 --- a/src/index.js +++ b/src/index.js @@ -132,12 +132,14 @@ function dva() { if (Array.isArray(saga)) { [_saga, opts] = saga; opts = opts || {}; - check(opts.type, is.sagaType, 'Type must be takeEvery or takeLatest'); - warn(opts.type, v => v === 'takeLatest', 'takeEvery is the default type, no need to set it by opts'); + check(opts.type, is.sagaType, 'Type must be takeEvery, takeLatest or watcher'); + warn(opts.type, v => v !== 'takeEvery', 'takeEvery is the default type, no need to set it by opts'); _type = opts.type; } - if (_type === 'takeEvery') { + if (_type === 'watcher') { + return _saga; + } else if (_type === 'takeEvery') { return function*() { yield takeEvery(k, _saga); }; diff --git a/src/utils.js b/src/utils.js index 71b2504f..75a60c30 100644 --- a/src/utils.js +++ b/src/utils.js @@ -23,7 +23,7 @@ export const is = { array : Array.isArray, object : isPlainObject, jsx : v => v && v.$$typeof && v.$$typeof.toString() === 'Symbol(react.element)', - sagaType : v => v === 'takeEvery' || v === 'takeLatest', + sagaType : v => v === 'takeEvery' || v === 'takeLatest' || v === 'watcher', }; /** diff --git a/test/app.model-test.js b/test/app.model-test.js index 603a3f1b..e403b55c 100644 --- a/test/app.model-test.js +++ b/test/app.model-test.js @@ -1,5 +1,6 @@ import expect from 'expect'; import dva from '../src/index'; +import { take, call } from '../effects'; describe('app.model', () => { it('reducer enhancer', () => { @@ -18,7 +19,6 @@ describe('app.model', () => { state: 3, reducers: [{ ['add'](state) { - console.log('reducer: add'); return state + 1; }, }, enhancer], @@ -30,4 +30,93 @@ describe('app.model', () => { app.store.dispatch({ type: 'add' }); expect(app.store.getState().count).toEqual(10); }); + + it('effects: type takeEvery', () => { + let count = 0; + const app = dva(); + app.model({ + namespace: 'count', + state: 0, + effects: { + ['add']: function*() { + count = count + 1; + }, + } + }); + app.router(({history}) =>
); + app.start(); + + app.store.dispatch({type: 'add'}); + app.store.dispatch({type: 'add'}); + expect(count).toEqual(2); + }); + + it('effects: type takeLatest', (done) => { + let count = 0; + const app = dva(); + const delay = (timeout) => { + return new Promise(resolve => { + setTimeout(resolve, timeout); + }); + }; + app.model({ + namespace: 'count', + state: 0, + effects: { + ['add']: [function*() { + yield call(delay, 1); + count = count + 1; + }, { + type: 'takeLatest', + }], + } + }); + app.router(({history}) =>
); + app.start(); + + // Only catch the last one. + app.store.dispatch({type: 'add'}); + app.store.dispatch({type: 'add'}); + + setTimeout(() => { + expect(count).toEqual(1); + done(); + }, 100); + }); + + it('effects: type watcher', (done) => { + let count = 0; + const app = dva(); + const delay = (timeout) => { + return new Promise(resolve => { + setTimeout(resolve, timeout); + }); + }; + app.model({ + namespace: 'count', + state: 0, + effects: { + ['addWatcher']: [function*() { + while(true) { + yield take('add'); + yield delay(1); + count = count + 1; + } + }, { + type: 'watcher', + }], + } + }); + app.router(({history}) =>
); + app.start(); + + // Only catch the first one. + app.store.dispatch({type: 'add'}); + app.store.dispatch({type: 'add'}); + + setTimeout(() => { + expect(count).toEqual(1); + done(); + }, 100); + }); }); From 43cc82b82709cd236956c08c82b37aba74ce94c0 Mon Sep 17 00:00:00 2001 From: sorrycc Date: Wed, 13 Jul 2016 15:32:37 +0800 Subject: [PATCH 8/9] app.model: support onError for effects exception --- src/index.js | 22 +++++++++++++++++----- test/app.model-test.js | 24 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/index.js b/src/index.js index ad6919f6..d23bc0dd 100644 --- a/src/index.js +++ b/src/index.js @@ -11,7 +11,11 @@ import document from 'global/document'; import window from 'global/window'; import { is, check, warn } from './utils'; -function dva() { +function dva(opts = {}) { + const onError = opts.onError || function(error) { + throw new Error(error); + }; + let _routes = null; const _models = []; const app = { @@ -33,7 +37,7 @@ function dva() { _routes = routes; } - // start usage: + // Usage: // app.start(); // app.start(container); // app.start(container, opts); @@ -137,15 +141,23 @@ function dva() { _type = opts.type; } + function* sagaWithErrorCatch() { + try { + yield _saga(); + } catch (e) { + onError(e); + } + } + if (_type === 'watcher') { - return _saga; + return sagaWithErrorCatch; } else if (_type === 'takeEvery') { return function*() { - yield takeEvery(k, _saga); + yield takeEvery(k, sagaWithErrorCatch); }; } else { return function*() { - yield takeLatest(k, _saga); + yield takeLatest(k, sagaWithErrorCatch); }; } } diff --git a/test/app.model-test.js b/test/app.model-test.js index e403b55c..fd236230 100644 --- a/test/app.model-test.js +++ b/test/app.model-test.js @@ -119,4 +119,28 @@ describe('app.model', () => { done(); }, 100); }); + + it('effects: onError', () => { + const errors = []; + const app = dva({ + onError: (error) => { + errors.push(error.message); + }, + }); + + app.model({ + namespace: 'count', + state: 0, + effects: { + ['add']: function*() { + throw new Error('effect error'); + }, + } + }); + app.router(({history}) =>
); + app.start(); + app.store.dispatch({type: 'add'}); + + expect(errors).toEqual(['effect error']); + }); }); From 27cb1fb0c49bc13a3a1ffffd0d7938410cb10d0d Mon Sep 17 00:00:00 2001 From: sorrycc Date: Wed, 13 Jul 2016 16:06:25 +0800 Subject: [PATCH 9/9] app.model: support onError for subscriptions exception --- src/index.js | 28 ++++++++++++++++------------ test/app.model-test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/index.js b/src/index.js index d23bc0dd..8efdceb6 100644 --- a/src/index.js +++ b/src/index.js @@ -12,8 +12,14 @@ import window from 'global/window'; import { is, check, warn } from './utils'; function dva(opts = {}) { - const onError = opts.onError || function(error) { - throw new Error(error); + const onError = opts.onError || function(err) { + throw new Error(err); + }; + const onErrorWrapper = (err) => { + if (err) { + if (is.string(err)) err = new Error(err); + onError(err); + } }; let _routes = null; @@ -105,16 +111,14 @@ function dva(opts = {}) { sagaMiddleware.run(rootSaga); // Handle subscriptions. - document.addEventListener('DOMContentLoaded', () => { - _models.forEach(({ subscriptions }) => { - if (subscriptions) { - check(subscriptions, is.array, 'Subscriptions must be an array'); - subscriptions.forEach(sub => { - check(sub, is.func, 'Subscription must be an function'); - sub(store.dispatch); - }); - } - }); + _models.forEach(({ subscriptions }) => { + if (subscriptions) { + check(subscriptions, is.array, 'Subscriptions must be an array'); + subscriptions.forEach(sub => { + check(sub, is.func, 'Subscription must be an function'); + sub(store.dispatch, onErrorWrapper); + }); + } }); // Render and hmr. diff --git a/test/app.model-test.js b/test/app.model-test.js index fd236230..05beeac2 100644 --- a/test/app.model-test.js +++ b/test/app.model-test.js @@ -143,4 +143,38 @@ describe('app.model', () => { expect(errors).toEqual(['effect error']); }); + + it('subscriptions: onError', (done) => { + const errors = []; + const app = dva({ + onError: (error) => { + errors.push(error.message); + }, + }); + + app.model({ + namespace: 'count', + state: 0, + effects: { + ['add']: function*() { + throw new Error('effect error'); + }, + }, + subscriptions: [ + function(dispatch, done) { + dispatch({ type: 'add' }); + setTimeout(() => { + done('subscription error'); + }, 100); + }, + ] + }); + app.router(({history}) =>
); + app.start(); + + setTimeout(() => { + expect(errors).toEqual(['effect error', 'subscription error']); + done(); + }, 500); + }); });