always run onEnter and onLeave handlers on routes, closes #21

michalkvasnicak · Mar 10, 2016 · 0e52e58 · 0e52e58
1 parent 4ab06fe
commit 0e52e58
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 130 deletions.
diff --git a/src/Router.js b/src/Router.js
@@ -21,12 21,15 @@ function instantiateRoutes(routes) {
  });
 }
 
 const REPLACE_STATE = 'replace';
 const PUSH_STATE = 'push';
 const DO_NOTHING = 'nope';
 
 export default class Router {
  constructor(
  routes = [],
  history,
- onTransition = function transitionFinished() {},
- resolveOnLoad = true
  onTransition = function transitionFinished() {}
  ) {
  invariant(Array.isArray(routes), `Routes should be an array, ${typeof routes} given.`);
  invariant(
@@ -36,9 39,6 @@ export default class Router {
 
  this.routes = instantiateRoutes(routes);
 
- // should onEnter handler run on initial load?
- this.resolveOnLoad = resolveOnLoad;
-
  // enable queries means that query parameters can be used directly as objects
  this.history = history;
 
@@ -51,8 51,6 @@ export default class Router {
  notFound: []
  };
 
- this.location = null;
-
  this.handlerWrappers = {
  onEnter(onEnter) {
  return onEnter();
@@ -71,59 69,12 @@ export default class Router {
  }
 
  _handleChange(location) {
- this.location = location;
-
  if (location.action === 'POP') {
  // on handle pop state (we are moving in history)
- // just match route and call change success because we are assuming that everything has been already resolved
- // so just change route
  const path = location.pathname;
  const query = parseQuery(location.search);
 
- resolveWithFirstMatched(this.routes, path, query).then(
- (newRoute) => {
- const currentRoute = this._currentRoute;
-
- // replace state with new route if is not set (initial load)
- if (!location.state) {
- this._callEventListeners('changeStart', currentRoute, newRoute, this);
-
- if (this.resolveOnLoad) {
- const handlerWrappers = this.handlerWrappers;
-
- // call only on enter handler (initial run)
- runRouteHandlers('onEnter', newRoute, handlerWrappers, currentRoute, newRoute, this).then(
- () => resolveComponents(newRoute.components).then(
- (components) => {
- return this._finishRun({ ...newRoute, components }, path, query, true);
- },
- this._rejectTransition('Route components cannot be resolved')
- ),
- this._rejectTransition('Route onEnter handlers are rejected.')
- );
- } else {
- resolveComponents(newRoute.components).then(
- (components) => {
- return this._finishRun({ ...newRoute, components }, path, query, true);
- },
- this._rejectTransition('Route components cannot be resolved')
- );
- }
- } else {
- this._currentRoute = newRoute;
-
- this._callEventListeners('changeSuccess', newRoute);
-
- // do nothing about state because it is already store
- this.onTransition(null, newRoute);
- }
- },
- () => {
- const e = new RouteNotFoundError('Route not found');
- this._callEventListeners('notFound', location.pathname, parseQuery(location.search));
- this.onTransition(e);
- }
- );
  this.run(path, query, !!location.state ? DO_NOTHING : REPLACE_STATE);
  }
  }
 
@@ -172,18 123,33 @@ export default class Router {
  };
  }
 
- _finishRun(resolvedRoute, path, query, replace = false) {
  /**
  * Finishes run route resolving
  *
  * @param {Object} resolvedRoute
  * @param {String} path
  * @param {Object} query
  * @param {String} action
  * @returns {Object}
  * @private
  */
  _finishRun(resolvedRoute, path, query, action) {
  this._currentRoute = resolvedRoute;
  this._callEventListeners('changeSuccess', resolvedRoute);
 
- if (replace) {
- this.history.replaceState(
- resolvedRoute,
- createHref(path, query)
- );
- } else {
- this.history.pushState(resolvedRoute, createHref(path, query));
  /* eslint-disable default-case */
  switch (action) {
  case PUSH_STATE:
  this.history.pushState(resolvedRoute, createHref(path, query));
  break;
  case REPLACE_STATE:
  this.history.replaceState(
  resolvedRoute,
  createHref(path, query)
  );
  break;
  }
  /* eslint-enable default-case */
 
  this.onTransition(null, resolvedRoute);
 
@@ -229,9 195,10 @@ export default class Router {
  *
  * @param {String} path
  * @param {Object} query
  * @param {String} action
  * @returns {Promise}
  */
- run(path, query = {}) {
  run(path, query = {}, action = PUSH_STATE) {
  const runResolvedRoute = (resolvedRoute) => {
  const currentRoute = this._currentRoute;
  this._callEventListeners('changeStart', currentRoute, resolvedRoute, this);
@@ -243,7 210,7 @@ export default class Router {
  () => runRouteHandlers('onEnter', resolvedRoute, handlerWrappers, currentRoute, resolvedRoute, this).then(
  () => resolveComponents(resolvedRoute.components).then(
  (components) => {
- return this._finishRun({ ...resolvedRoute, components }, path, query);
  return this._finishRun({ ...resolvedRoute, components }, path, query, action);
  },
  this._rejectTransition('Route components cannot be resolved')
  ),

diff --git a/src/createRoutex.js b/src/createRoutex.js
@@ -20,24 20,12 @@ import {
  * @param {Array} routes
  * @param {Object} history
  * @param {?Function} onTransition
- * @param {?Boolean} resolveOnLoad should on enter handler run on initial load?
  * @returns {{router: Router, store: store, reducer: {router: reducer}}}
  */
-export default function createRoutex(routes, history, onTransition, resolveOnLoad) {
 export default function createRoutex(routes, history, onTransition) {
  const initialReducerState = { state: 'INITIAL', route: null };
- const args = [];
-
- if (arguments.length >= 4) {
- args.push(onTransition, resolveOnLoad);
- } else {
- if (typeof onTransition === 'boolean') {
- args.push(undefined, onTransition);
- } else {
- args.push(onTransition, undefined);
- }
- }
 
- const router = new Router(routes, history, ...args);
  const router = new Router(routes, history, onTransition);
 
  const store = (next) => (reducer, initialState) => {
  const modifiedInitialState = initialState;

diff --git a/test/Router.spec.js b/test/Router.spec.js
@@ -30,7 30,7 @@ describe('Router', () => {
 
  describe('#listen()', () => {
  it(
- 'starts listening to pop state events and replaces state on initial and replaces state if undefined (resolveOnLoad = true, run onEnter handlers)',
  'starts listening to pop state events and replaces state on initial and replaces state if undefined',
  (done) => {
  const changeStart = spy();
  const changeSuccess = spy();
@@ -73,51 73,6 @@ describe('Router', () => {
  }
  );
 
- it(
- 'starts listening to pop state events and replaces state on initial and replaces state if undefined (resolveOnLoad = false, do not run onEnter handlers)',
- (done) => {
- const changeStart = spy();
- const changeSuccess = spy();
- const onEnter = spy();
- const history = createMemoryHistory();
-
- const router = new Router(
- [{ path: '/', component: 'A', onEnter, attrs: { simple: 1 } }],
- history,
- (err, resolvedRoute) => {
- try {
- expect(err).to.be.equal(null);
- expect(resolvedRoute).to.be.an('object');
- expect(resolvedRoute.pathname).to.be.equal('/');
- expect(resolvedRoute.fullPath).to.be.equal('/');
- expect(resolvedRoute.components).to.be.eql(['A']);
- expect(resolvedRoute.attrs).to.be.deep.equal({ simple: 1 });
-
- expect(history.replaceState.calledOnce).to.be.equal(true);
- expect(history.replaceState.getCall(0).args[0]).to.be.equal(resolvedRoute);
- expect(history.replaceState.getCall(0).args[1]).to.be.equal('/');
-
- expect(changeStart.called).to.be.equal(true);
- expect(changeSuccess.calledOnce).to.be.equal(true);
- expect(onEnter.called).to.be.equal(false);
-
- done();
- } catch (e) {
- done(e);
- }
- },
- false
- );
-
- spy(history, 'replaceState');
-
- router.addChangeStartListener(changeStart);
- router.addChangeSuccessListener(changeSuccess);
-
- router.listen();
- }
- );
-
  it(
  'starts listening to pop state events and calls not found listeners if current location is not mapped to route',
  (done) => {

diff --git a/test/routex.spec.js b/test/routex.spec.js
@@ -147,8 147,6 @@ describe('routex', () => {
  });
 
  it('changes state using change success action if pop state event is emitted', (done) => {
- let _stepper;
-
  const childState = {
  pathname: '/child',
  query: {},
@@ -193,9 191,7 @@ describe('routex', () => {
  }
  ];
 
- _stepper = spy(stepper(steps, done));
-
- store = createRoutexStore(history, _stepper, {
  store = createRoutexStore(history, stepper(steps, done), {
  router: {
  state: 'TRANSITIONED',
  route: {