Skip to content
This repository has been archived by the owner on Dec 19, 2017. It is now read-only.

Commit

Permalink
Fix issue with improper disposal of child routers
Browse files Browse the repository at this point in the history
  • Loading branch information
caseyWebb committed Oct 26, 2016
1 parent 1cd8f67 commit c8c0f83
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 35 deletions.
24 changes: 14 additions & 10 deletions dist/ko-component-router.js
Expand Up @@ -892,6 +892,7 @@ return /******/ (function(modules) { // webpackBootstrap
var childPath = _route$parse2[5];

var samePage = this.pathname() === pathname;
var sameRoute = this.route() === route;

var shouldNavigatePromise = function () {
if (samePage) {
Expand All @@ -918,16 +919,25 @@ return /******/ (function(modules) { // webpackBootstrap

var paramsChanged = !(0, _utils.deepEquals)(params, _this2.prevParams);
var queryChanged = query && !(0, _utils.deepEquals)(query, _this2.prevQuery);
var paramsForcedUpdate = _this2.config._forceReloadOnParamChange && paramsChanged;
var queryForcedUpdate = _this2.config._forceReloadOnQueryChange && queryChanged;
var forceUpdate = paramsForcedUpdate || queryForcedUpdate;

_this2.prevParams = params;
if (query) {
_this2.prevQuery = query;
}

if (!samePage && !firstRun || _this2.config._forceReloadOnParamChange && paramsChanged || _this2.config._forceReloadOnQueryChange && queryChanged) {
if (!sameRoute || forceUpdate) {
if (_this2.$child) {
_this2.$child.destroy();
delete _this2.$child;
}
}

if (!samePage && !firstRun || forceUpdate) {
_this2.isNavigating(true);
_this2.reload();
_this2._beforeNavigateCallbacks = [];
}

var canonicalPath = Context.getCanonicalPath(_this2.getBase().replace(/\/$/, ''), pathname, childPath, _this2.query.getFullQueryString(query, pathname), hash);
Expand Down Expand Up @@ -967,16 +977,14 @@ return /******/ (function(modules) { // webpackBootstrap
delete toCtx.query;
toCtx.route.runPipeline(toCtx).then(function () {
if (fromCtx.route.component === toCtx.route.component) {
(0, _utils.merge)(_this2, toCtx);
if (_this2.config._forceReloadOnParamChange && paramsChanged || _this2.config._forceReloadOnQueryChange && queryChanged) {
var r = toCtx.route;
toCtx.route = { component: '__KO_ROUTER_EMPTY_COMPONENT__' };
_this2.config._forceReloadOnParamChange = false;
_this2.config._forceReloadOnQueryChange = false;
(0, _utils.extend)(_this2, toCtx);
_knockout2.default.tasks.runEarly();
_this2.route(r);
} else {
(0, _utils.merge)(_this2, toCtx);
}
} else {
_this2.config._forceReloadOnParamChange = false;
Expand Down Expand Up @@ -1074,11 +1082,7 @@ return /******/ (function(modules) { // webpackBootstrap
}, {
key: 'reload',
value: function reload() {
if (this.$child) {
this.$child.destroy();
delete this.$child;
}

this._beforeNavigateCallbacks = [];
this.query.reload();
this.state.reload();
}
Expand Down
2 changes: 1 addition & 1 deletion dist/ko-component-router.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion example/dist/bundle.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion example/dist/bundle.js.map

Large diffs are not rendered by default.

24 changes: 14 additions & 10 deletions lib/context.js
Expand Up @@ -128,6 +128,7 @@ var Context = function () {
var childPath = _route$parse2[5];

var samePage = this.pathname() === pathname;
var sameRoute = this.route() === route;

var shouldNavigatePromise = function () {
if (samePage) {
Expand All @@ -154,16 +155,25 @@ var Context = function () {

var paramsChanged = !(0, _utils.deepEquals)(params, _this2.prevParams);
var queryChanged = query && !(0, _utils.deepEquals)(query, _this2.prevQuery);
var paramsForcedUpdate = _this2.config._forceReloadOnParamChange && paramsChanged;
var queryForcedUpdate = _this2.config._forceReloadOnQueryChange && queryChanged;
var forceUpdate = paramsForcedUpdate || queryForcedUpdate;

_this2.prevParams = params;
if (query) {
_this2.prevQuery = query;
}

if (!samePage && !firstRun || _this2.config._forceReloadOnParamChange && paramsChanged || _this2.config._forceReloadOnQueryChange && queryChanged) {
if (!sameRoute || forceUpdate) {
if (_this2.$child) {
_this2.$child.destroy();
delete _this2.$child;
}
}

if (!samePage && !firstRun || forceUpdate) {
_this2.isNavigating(true);
_this2.reload();
_this2._beforeNavigateCallbacks = [];
}

var canonicalPath = Context.getCanonicalPath(_this2.getBase().replace(/\/$/, ''), pathname, childPath, _this2.query.getFullQueryString(query, pathname), hash);
Expand Down Expand Up @@ -203,16 +213,14 @@ var Context = function () {
delete toCtx.query;
toCtx.route.runPipeline(toCtx).then(function () {
if (fromCtx.route.component === toCtx.route.component) {
(0, _utils.merge)(_this2, toCtx);
if (_this2.config._forceReloadOnParamChange && paramsChanged || _this2.config._forceReloadOnQueryChange && queryChanged) {
var r = toCtx.route;
toCtx.route = { component: '__KO_ROUTER_EMPTY_COMPONENT__' };
_this2.config._forceReloadOnParamChange = false;
_this2.config._forceReloadOnQueryChange = false;
(0, _utils.extend)(_this2, toCtx);
_knockout2.default.tasks.runEarly();
_this2.route(r);
} else {
(0, _utils.merge)(_this2, toCtx);
}
} else {
_this2.config._forceReloadOnParamChange = false;
Expand Down Expand Up @@ -310,11 +318,7 @@ var Context = function () {
}, {
key: 'reload',
value: function reload() {
if (this.$child) {
this.$child.destroy();
delete this.$child;
}

this._beforeNavigateCallbacks = [];
this.query.reload();
this.state.reload();
}
Expand Down
26 changes: 14 additions & 12 deletions src/context.js
Expand Up @@ -79,6 +79,7 @@ export default class Context {
const fromCtx = this.toJS()
const [path, params, hash, pathname, querystring, childPath] = route.parse(url)
const samePage = this.pathname() === pathname
const sameRoute = this.route() === route

const shouldNavigatePromise = (() => {
if (samePage) {
Expand All @@ -105,18 +106,25 @@ export default class Context {

const paramsChanged = !deepEquals(params, this.prevParams)
const queryChanged = query && !deepEquals(query, this.prevQuery)
const paramsForcedUpdate = this.config._forceReloadOnParamChange && paramsChanged
const queryForcedUpdate = this.config._forceReloadOnQueryChange && queryChanged
const forceUpdate = paramsForcedUpdate || queryForcedUpdate

this.prevParams = params
if (query) {
this.prevQuery = query
}

if ((!samePage && !firstRun) ||
(this.config._forceReloadOnParamChange && paramsChanged) ||
(this.config._forceReloadOnQueryChange && queryChanged)) {
if (!sameRoute || forceUpdate) {
if (this.$child) {
this.$child.destroy()
delete this.$child
}
}

if ((!samePage && !firstRun) || forceUpdate) {
this.isNavigating(true)
this.reload()
this._beforeNavigateCallbacks = []
}

const canonicalPath = Context
Expand Down Expand Up @@ -168,17 +176,15 @@ export default class Context {
toCtx.route.runPipeline(toCtx)
.then(() => {
if (fromCtx.route.component === toCtx.route.component) {
merge(this, toCtx)
if ((this.config._forceReloadOnParamChange && paramsChanged) ||
(this.config._forceReloadOnQueryChange && queryChanged)) {
const r = toCtx.route
toCtx.route = { component: '__KO_ROUTER_EMPTY_COMPONENT__' }
this.config._forceReloadOnParamChange = false
this.config._forceReloadOnQueryChange = false
extend(this, toCtx)
ko.tasks.runEarly()
this.route(r)
} else {
merge(this, toCtx)
}
} else {
this.config._forceReloadOnParamChange = false
Expand Down Expand Up @@ -270,11 +276,7 @@ export default class Context {
}

reload() {
if (this.$child) {
this.$child.destroy()
delete this.$child
}

this._beforeNavigateCallbacks = []
this.query.reload()
this.state.reload()
}
Expand Down

0 comments on commit c8c0f83

Please sign in to comment.