Merge pull request #898 from transloadit/fix/uppy-close

Fix/refactor close() and removePlugin

src/core/Core.js

@@ -886,16 +886,22 @@ class Uppy {
    * @param {object} instance The plugin instance to remove.
    */
   removePlugin (instance) {
-    const list = this.plugins[instance.type]
+    this.emit('plugin-remove', instance)
 
     if (instance.uninstall) {
       instance.uninstall()
     }
 
+    const list = this.plugins[instance.type].slice()
     const index = list.indexOf(instance)
     if (index !== -1) {
       list.splice(index, 1)
+      this.plugins[instance.type] = list
     }
+
+    const updatedState = this.getState()
+    delete updatedState.plugins[instance.id]
+    this.setState(updatedState)
   }
 
   /**
@@ -907,7 +913,7 @@ class Uppy {
     this._storeUnsubscribe()
 
     this.iteratePlugins((plugin) => {
-      plugin.uninstall()
+      this.removePlugin(plugin)
     })
   }
 

src/core/Core.test.js

@@ -256,10 +256,10 @@ describe('src/Core', () => {
     const core = new Core()
     core.use(AcquirerPlugin1)
 
-    // const corePauseEventMock = jest.fn()
     const coreCancelEventMock = jest.fn()
     const coreStateUpdateEventMock = jest.fn()
-    // core.on('pause-all', corePauseEventMock)
+    const plugin = core.plugins.acquirer[0]
+
     core.on('cancel-all', coreCancelEventMock)
     core.on('state-update', coreStateUpdateEventMock)
 
@@ -278,9 +278,8 @@ describe('src/Core', () => {
       plugins: {},
       totalProgress: 0
     })
-    expect(core.plugins.acquirer[0].mocks.uninstall.mock.calls.length).toEqual(
-      1
-    )
+    expect(plugin.mocks.uninstall.mock.calls.length).toEqual(1)
+    expect(core.plugins[Object.keys(core.plugins)[0]].length).toEqual(0)
   })
 
   describe('upload hooks', () => {

src/core/Plugin.js

@@ -85,6 +85,10 @@ module.exports = class Plugin {
 
       // API for plugins that require a synchronous rerender.
       this.rerender = (state) => {
+        // plugin could be removed, but this.rerender is debounced below,
+        // so it could still be called even after uppy.removePlugin or uppy.close
+        // hence the check
+        if (!this.uppy.getPlugin(this.id)) return
         this.el = preact.render(this.render(state), targetElement, this.el)
       }
       this._updateUI = debounce(this.rerender)
@@ -137,7 +141,7 @@ module.exports = class Plugin {
   }
 
   unmount () {
-    if (this.el && this.el.parentNode) {
+    if (this.isTargetDOMEl && this.el && this.el.parentNode) {
       this.el.parentNode.removeChild(this.el)
     }
   }

src/plugins/Dashboard/index.js

@@ -131,6 +131,7 @@ module.exports = class Dashboard extends Plugin {
     this.isModalOpen = this.isModalOpen.bind(this)
 
     this.addTarget = this.addTarget.bind(this)
+    this.removeTarget = this.removeTarget.bind(this)
     this.hideAllPanels = this.hideAllPanels.bind(this)
     this.showPanel = this.showPanel.bind(this)
     this.getFocusableNodes = this.getFocusableNodes.bind(this)
@@ -149,6 +150,16 @@ module.exports = class Dashboard extends Plugin {
     this.install = this.install.bind(this)
   }
 
+  removeTarget (plugin) {
+    const pluginState = this.getPluginState()
+    // filter out the one we want to remove
+    const newTargets = pluginState.targets.filter(target => target.id !== plugin.id)
+
+    this.setPluginState({
+      targets: newTargets
+    })
+  }
+
   addTarget (plugin) {
     const callerPluginId = plugin.id || plugin.constructor.name
     const callerPluginName = plugin.title || callerPluginId
@@ -362,6 +373,8 @@ module.exports = class Dashboard extends Plugin {
 
     this.updateDashboardElWidth()
     window.addEventListener('resize', this.updateDashboardElWidth)
+
+    this.uppy.on('plugin-remove', this.removeTarget)
   }
 
   removeEvents () {
@@ -372,6 +385,8 @@ module.exports = class Dashboard extends Plugin {
 
     this.removeDragDropListener()
     window.removeEventListener('resize', this.updateDashboardElWidth)
+
+    this.uppy.off('plugin-remove', this.removeTarget)
   }
 
   updateDashboardElWidth () {