Skip to content

Commit

Permalink
Fix uppy.close() crashes when remote-sources or image-editor is ins…
Browse files Browse the repository at this point in the history
…talled (#3914)
  • Loading branch information
Murderlon committed Jul 27, 2022
1 parent 7f5059b commit f1a7e0c
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 5 deletions.
23 changes: 23 additions & 0 deletions e2e/clients/dashboard-ui/app.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,36 @@
import Uppy from '@uppy/core'
import Dashboard from '@uppy/dashboard'
import RemoteSources from '@uppy/remote-sources'
import Webcam from '@uppy/webcam'
import ScreenCapture from '@uppy/screen-capture'
import GoldenRetriever from '@uppy/golden-retriever'
import ImageEditor from '@uppy/image-editor'
import DropTarget from '@uppy/drop-target'
import Audio from '@uppy/audio'
import Compressor from '@uppy/compressor'

import '@uppy/core/dist/style.css'
import '@uppy/dashboard/dist/style.css'

const COMPANION_URL = 'http://companion.uppy.io'

const uppy = new Uppy()
.use(Dashboard, { target: '#app', inline: true })
.use(RemoteSources, { companionUrl: COMPANION_URL })
.use(Webcam, {
target: Dashboard,
showVideoSourceDropdown: true,
showRecordingLength: true,
})
.use(Audio, {
target: Dashboard,
showRecordingLength: true,
})
.use(ScreenCapture, { target: Dashboard })
.use(ImageEditor, { target: Dashboard })
.use(DropTarget, { target: document.body })
.use(Compressor)
.use(GoldenRetriever, { serviceWorker: true })

// Keep this here to access uppy in tests
window.uppy = uppy
8 changes: 8 additions & 0 deletions e2e/cypress/integration/dashboard-ui.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ describe('dashboard-ui', () => {
cy.get('.uppy-Dashboard-input:first').as('file-input')
})

it('should not throw when calling uppy.close()', () => {
cy.get('@file-input').selectFile(['cypress/fixtures/images/cat.jpg', 'cypress/fixtures/images/traffic.jpg'], { force:true })

cy.window().then(({ uppy }) => {
expect(uppy.close()).to.not.throw
})
})

it('should render thumbnails', () => {
cy.get('@file-input').selectFile(['cypress/fixtures/images/cat.jpg', 'cypress/fixtures/images/traffic.jpg'], { force:true })
cy.get('.uppy-Dashboard-Item-previewImg')
Expand Down
3 changes: 1 addition & 2 deletions packages/@uppy/dashboard/src/components/FileCard/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ class FileCard extends Component {
}

handleCancel = () => {
const { currentImage } = this.getPluginState()
const file = this.props.uppy.getFile(currentImage.id)
const file = this.props.files[this.props.fileCardFor]
this.props.uppy.emit('file-editor:cancel', file)
this.props.toggleFileCard(false)
}
Expand Down
6 changes: 4 additions & 2 deletions packages/@uppy/image-editor/src/ImageEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,11 @@ export default class ImageEditor extends UIPlugin {

uninstall () {
const { currentImage } = this.getPluginState()
const file = this.uppy.getFile(currentImage.id)
this.uppy.emit('file-editor:cancel', file)

if (currentImage) {
const file = this.uppy.getFile(currentImage.id)
this.uppy.emit('file-editor:cancel', file)
}
this.unmount()
}

Expand Down
4 changes: 3 additions & 1 deletion packages/@uppy/remote-sources/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export default class RemoteSources extends BasePlugin {
throw new Error(`Invalid plugin: "${pluginId}" is not one of: ${formatter.format(pluginNames)}.`)
}
this.uppy.use(plugin, optsForRemoteSourcePlugin)
this.#installedPlugins.add(plugin)
// `plugin` is a class, but we want to track the instance object
// so we have to do `getPlugin` here.
this.#installedPlugins.add(this.uppy.getPlugin(pluginId))
})
}

Expand Down

0 comments on commit f1a7e0c

Please sign in to comment.