From 80561053cc9c6205672e2ca4a613a17de7496c0c Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Fri, 12 Apr 2019 13:33:43 +0200 Subject: [PATCH] fix: prevent memory leaks by removing app references (#2706) Fix #2639 --- src/index.js | 14 +++++- test/unit/specs/api.spec.js | 95 +++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 69c3cc0c8..b3eb25dd3 100644 --- a/src/index.js +++ b/src/index.js @@ -89,7 +89,19 @@ export default class VueRouter { this.apps.push(app) - // main app already initialized. + // set up app destroyed handler + // https://github.com/vuejs/vue-router/issues/2639 + app.$once('hook:destroyed', () => { + // clean out app from this.apps array once destroyed + const index = this.apps.indexOf(app) + if (index > -1) this.apps.splice(index, 1) + // ensure we still have a main app or null if no apps + // we do not release the router so it can be reused + if (this.app === app) this.app = this.apps[0] || null + }) + + // main app previously initialized + // return as we don't need to set up new history listener if (this.app) { return } diff --git a/test/unit/specs/api.spec.js b/test/unit/specs/api.spec.js index 5fce2e885..1fba4977b 100644 --- a/test/unit/specs/api.spec.js +++ b/test/unit/specs/api.spec.js @@ -1,4 +1,5 @@ import Router from '../../../src/index' +import Vue from 'vue' describe('router.onReady', () => { it('should work', done => { @@ -185,3 +186,97 @@ describe('router.push/replace callbacks', () => { }) }) }) + +describe('router app destroy handling', () => { + Vue.use(Router) + + let router, app1, app2, app3 + + beforeEach(() => { + router = new Router({ + mode: 'abstract', + routes: [ + { path: '/', component: { name: 'A' }} + ] + }) + + // Add main app + app1 = new Vue({ + router, + render (h) { return h('div') } + }) + + // Add 2nd app + app2 = new Vue({ + router, + render (h) { return h('div') } + }) + + // Add 3rd app + app3 = new Vue({ + router, + render (h) { return h('div') } + }) + }) + + it('all apps point to the same router instance', () => { + expect(app1.$router).toBe(app2.$router) + expect(app2.$router).toBe(app3.$router) + }) + + it('should have all 3 registered apps', () => { + expect(app1.$router.app).toBe(app1) + expect(app1.$router.apps.length).toBe(3) + expect(app1.$router.apps[0]).toBe(app1) + expect(app1.$router.apps[1]).toBe(app2) + expect(app1.$router.apps[2]).toBe(app3) + }) + + it('should remove 2nd destroyed app from this.apps', () => { + app2.$destroy() + expect(app1.$router.app).toBe(app1) + expect(app1.$router.apps.length).toBe(2) + expect(app1.$router.apps[0]).toBe(app1) + expect(app1.$router.apps[1]).toBe(app3) + }) + + it('should remove 1st destroyed app and replace current app', () => { + app1.$destroy() + expect(app3.$router.app).toBe(app2) + expect(app3.$router.apps.length).toBe(2) + expect(app3.$router.apps[0]).toBe(app2) + expect(app1.$router.apps[1]).toBe(app3) + }) + + it('should remove all apps', () => { + app1.$destroy() + app3.$destroy() + app2.$destroy() + expect(app3.$router.app).toBe(null) + expect(app3.$router.apps.length).toBe(0) + }) + + it('should keep current app if already defined', () => { + const app4 = new Vue({ + router, + render (h) { return h('div') } + }) + expect(app4.$router.app).toBe(app1) + expect(app4.$router.apps.length).toBe(4) + expect(app4.$router.apps[3]).toBe(app4) + }) + + it('should replace current app if none is assigned when creating the app', () => { + app1.$destroy() + app3.$destroy() + app2.$destroy() + const app4 = new Vue({ + router, + render (h) { return h('div') } + }) + expect(router.app).toBe(app4) + expect(app4.$router).toBe(router) + expect(app4.$router.apps.length).toBe(1) + expect(app4.$router.apps[0]).toBe(app4) + }) +})