Skip to content

Commit

Permalink
Fix all breaking todo comments for 3.0 (#3907)
Browse files Browse the repository at this point in the history
- `@uppy/aws/s3-multipart`: remove `client` getter and setter.
  - reason: internal usage only
  - migrate: use exposed options only
- `@uppy/core`: remove `AggregateError` polyfill
  - reason: [should be polyfilled by the user](#3532 (comment))
  - migrate: install `AggregateError` polyfill or use `core-js`
- `@uppy/core`: remove `reset()` method
  - reason: it's a duplicate of `cancelAll`, but with a less intention revealing name
  - migrate: use `cancelAll`
- `@uppy/core`: remove backwards compatible exports (static properties on `Uppy`)
  - reason: transition to ESM
  - migrate: import the `Uppy` class by default and/or use named exports for everything else.
- `@uppy/react`: don't expose `validProps`
  - reason: internal only
  - migrate: do not depend on this
- `@uppy/store-redux`: remove backwards compatible exports (static properties on `ReduxStore`)
  - reason: transition to ESM
  - migrate: use named imports
- `@uppy/thumbnail-generator`: remove `rotateImage`, `protect`, and `canvasToBlob` from prototype.
  - reason: internal only
  - migrate: don't depend on this
  • Loading branch information
Murderlon authored Aug 3, 2022
1 parent 80f622f commit 9a213b5
Show file tree
Hide file tree
Showing 25 changed files with 37 additions and 128 deletions.
5 changes: 0 additions & 5 deletions packages/@uppy/aws-s3-multipart/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ export default class AwsS3Multipart extends BasePlugin {

[Symbol.for('uppy test: getClient')] () { return this.#client }

// TODO: remove getter and setter for #client on the next major release
get client () { return this.#client }

set client (client) { this.#client = client }

/**
* Clean up all references for a file's upload: the MultipartUploader instance,
* any events related to the file, and the Companion WebSocket connection.
Expand Down
2 changes: 0 additions & 2 deletions packages/@uppy/companion/src/server/controllers/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ const meta = async (req, res) => {
return res.json(urlMeta)
} catch (err) {
logger.error(err, 'controller.url.meta.error', req.id)
// @todo send more meaningful error message and status code to client if possible
return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' })
}
}
Expand Down Expand Up @@ -125,7 +124,6 @@ const get = async (req, res) => {

function onUnhandledError (err) {
logger.error(err, 'controller.url.error', req.id)
// @todo send more meaningful error message and status code to client if possible
res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' })
}

Expand Down
2 changes: 0 additions & 2 deletions packages/@uppy/core/src/BasePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ export default class BasePlugin {
throw new Error('Extend the render method to add your plugin to a DOM element')
}

// TODO: remove in the next major version. It's not feasible to
// try to use plugins with other frameworks.
// eslint-disable-next-line class-methods-use-this
update () {}

Expand Down
16 changes: 1 addition & 15 deletions packages/@uppy/core/src/Restricter.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable max-classes-per-file, class-methods-use-this */
/* global AggregateError */
import prettierBytes from '@transloadit/prettier-bytes'
import match from 'mime-match'

Expand All @@ -17,17 +16,6 @@ class RestrictionError extends Error {
isRestriction = true
}

if (typeof AggregateError === 'undefined') {
// eslint-disable-next-line no-global-assign
// TODO: remove this "polyfill" in the next major.
globalThis.AggregateError = class AggregateError extends Error {
constructor (errors, message) {
super(message)
this.errors = errors
}
}
}

class Restricter {
constructor (getOpts, i18n) {
this.i18n = i18n
Expand Down Expand Up @@ -108,12 +96,10 @@ class Restricter {
getMissingRequiredMetaFields (file) {
const error = new RestrictionError(this.i18n('missingRequiredMetaFieldOnFile', { fileName: file.name }))
const { requiredMetaFields } = this.getOpts().restrictions
// TODO: migrate to Object.hasOwn in the next major.
const own = Object.prototype.hasOwnProperty
const missingFields = []

for (const field of requiredMetaFields) {
if (!own.call(file.meta, field) || file.meta[field] === '') {
if (!Object.hasOwn(file.meta, field) || file.meta[field] === '') {
missingFields.push(field)
}
}
Expand Down
5 changes: 0 additions & 5 deletions packages/@uppy/core/src/Uppy.js
Original file line number Diff line number Diff line change
Expand Up @@ -843,11 +843,6 @@ class Uppy {
return this.#runUpload(uploadID)
}

// todo remove in next major. what is the point of the reset method when we have cancelAll or vice versa?
reset (...args) {
this.cancelAll(...args)
}

logout () {
this.iteratePlugins(plugin => {
if (plugin.provider && plugin.provider.logout) {
Expand Down
11 changes: 6 additions & 5 deletions packages/@uppy/core/src/Uppy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import fs from 'node:fs'
import prettierBytes from '@transloadit/prettier-bytes'
import Core from '../lib/index.js'
import UIPlugin from '../lib/UIPlugin.js'
import { debugLogger } from '../lib/loggers.js'
import AcquirerPlugin1 from './mocks/acquirerPlugin1.js'
import AcquirerPlugin2 from './mocks/acquirerPlugin2.js'
import InvalidPlugin from './mocks/invalidPlugin.js'
Expand Down Expand Up @@ -204,7 +205,7 @@ describe('src/Core', () => {
})
})

it('should reset when the reset method is called', () => {
it('should cancel all when the `cancelAll` method is called', () => {
// use DeepFrozenStore in some tests to make sure we are not mutating things
const core = new Core({
store: DeepFrozenStore(),
Expand All @@ -216,7 +217,7 @@ describe('src/Core', () => {
core.on('state-update', coreStateUpdateEventMock)
core.setState({ foo: 'bar', totalProgress: 30 })

core.reset()
core.cancelAll()

expect(coreCancelEventMock).toHaveBeenCalledWith({ reason: 'user' }, undefined, undefined, undefined, undefined, undefined)
expect(coreStateUpdateEventMock.mock.calls.length).toEqual(2)
Expand Down Expand Up @@ -1071,7 +1072,7 @@ describe('src/Core', () => {
)
})

it('allows new files again with allowMultipleUploadBatches: false after reset() was called', async () => {
it('allows new files again with allowMultipleUploadBatches: false after cancelAll() was called', async () => {
const core = new Core({ allowMultipleUploadBatches: false })

core.addFile({
Expand All @@ -1082,7 +1083,7 @@ describe('src/Core', () => {
})
await expect(core.upload()).resolves.toBeDefined()

core.reset()
core.cancelAll()

core.addFile({
source: 'jest',
Expand Down Expand Up @@ -2135,7 +2136,7 @@ describe('src/Core', () => {
console.error = jest.fn()

const core = new Core({
logger: Core.debugLogger,
logger: debugLogger,
})

core.log('test test')
Expand Down
17 changes: 1 addition & 16 deletions packages/@uppy/core/src/index.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,5 @@
export { default } from './Uppy.js'
export { default as Uppy } from './Uppy.js'
export { default as UIPlugin } from './UIPlugin.js'
export { default as BasePlugin } from './BasePlugin.js'
export { debugLogger } from './loggers.js'

// TODO: remove all the following in the next major
/* eslint-disable import/first */
import Uppy from './Uppy.js'
import UIPlugin from './UIPlugin.js'
import BasePlugin from './BasePlugin.js'
import { debugLogger } from './loggers.js'

// Backward compatibility: we want those to keep being accessible as static
// properties of `Uppy` to avoid a breaking change.
Uppy.Uppy = Uppy
Uppy.UIPlugin = UIPlugin
Uppy.BasePlugin = BasePlugin
Uppy.debugLogger = debugLogger

export { Uppy }
4 changes: 0 additions & 4 deletions packages/@uppy/core/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,6 @@ export type ErrorCallback = (error: Error) => void;
export type UploadErrorCallback<TMeta> = (file: UppyFile<TMeta> | undefined, error: Error, response?: ErrorResponse) => void;
export type UploadRetryCallback = (fileID: string) => void;
export type RetryAllCallback = (fileIDs: string[]) => void;

// TODO: reverse the order in the next major version
export type RestrictionFailedCallback<TMeta> = (file: UppyFile<TMeta> | undefined, error: Error) => void;

export interface UppyEventMap<TMeta = Record<string, unknown>> {
Expand Down Expand Up @@ -349,8 +347,6 @@ export class Uppy {
fileID: string
): Promise<UploadResult<TMeta>>

reset(): void

getID(): string

use<TOptions, TInstance extends UIPlugin | BasePlugin<TOptions>>(
Expand Down
2 changes: 1 addition & 1 deletion packages/@uppy/dashboard/src/Dashboard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default class Dashboard extends UIPlugin {
hidePauseResumeButton: false,
hideProgressAfterFinish: false,
doneButtonHandler: () => {
this.uppy.reset()
this.uppy.cancelAll()
this.requestCloseModal()
},
note: null,
Expand Down
6 changes: 2 additions & 4 deletions packages/@uppy/react/src/Dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class Dashboard extends Component {
if (prevProps.uppy !== this.props.uppy) {
this.uninstallPlugin(prevProps)
this.installPlugin()
} else if (nonHtmlPropsHaveChanged(this, prevProps)) {
} else if (nonHtmlPropsHaveChanged(this.props, prevProps)) {
const options = { ...this.props, target: this.container }
delete options.uppy
this.plugin.setOptions(options)
Expand Down Expand Up @@ -50,14 +50,12 @@ class Dashboard extends Component {
}

render () {
// TODO: stop exposing `validProps` as a public property and rename it to `htmlProps`
this.validProps = getHTMLProps(this.props)
return h('div', {
className: 'uppy-Container',
ref: (container) => {
this.container = container
},
...this.validProps,
...getHTMLProps(this.props),
})
}
}
Expand Down
6 changes: 2 additions & 4 deletions packages/@uppy/react/src/DashboardModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class DashboardModal extends Component {
if (prevProps.uppy !== uppy) {
this.uninstallPlugin(prevProps)
this.installPlugin()
} else if (nonHtmlPropsHaveChanged(this, prevProps)) {
} else if (nonHtmlPropsHaveChanged(this.props, prevProps)) {
const options = { ...this.props, onRequestCloseModal: onRequestClose }
delete options.uppy
this.plugin.setOptions(options)
Expand Down Expand Up @@ -135,14 +135,12 @@ class DashboardModal extends Component {
}

render () {
// TODO: stop exposing `validProps` as a public property and rename it to `htmlProps`
this.validProps = getHTMLProps(this.props)
return h('div', {
className: 'uppy-Container',
ref: (container) => {
this.container = container
},
...this.validProps,
...getHTMLProps(this.props),
})
}
}
Expand Down
6 changes: 2 additions & 4 deletions packages/@uppy/react/src/DragDrop.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class DragDrop extends Component {
if (prevProps.uppy !== this.props.uppy) {
this.uninstallPlugin(prevProps)
this.installPlugin()
} else if (nonHtmlPropsHaveChanged(this, prevProps)) {
} else if (nonHtmlPropsHaveChanged(this.props, prevProps)) {
const options = { ...this.props, target: this.container }
delete options.uppy
this.plugin.setOptions(options)
Expand Down Expand Up @@ -63,14 +63,12 @@ class DragDrop extends Component {
}

render () {
// TODO: stop exposing `validProps` as a public property and rename it to `htmlProps`
this.validProps = getHTMLProps(this.props)
return h('div', {
className: 'uppy-Container',
ref: (container) => {
this.container = container
},
...this.validProps,
...getHTMLProps(this.props),
})
}
}
Expand Down
6 changes: 2 additions & 4 deletions packages/@uppy/react/src/ProgressBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ProgressBar extends Component {
if (prevProps.uppy !== this.props.uppy) {
this.uninstallPlugin(prevProps)
this.installPlugin()
} else if (nonHtmlPropsHaveChanged(this, prevProps)) {
} else if (nonHtmlPropsHaveChanged(this.props, prevProps)) {
const options = { ...this.props, target: this.container }
delete options.uppy
this.plugin.setOptions(options)
Expand Down Expand Up @@ -52,14 +52,12 @@ class ProgressBar extends Component {
}

render () {
// TODO: stop exposing `validProps` as a public property and rename it to `htmlProps`
this.validProps = getHTMLProps(this.props)
return h('div', {
className: 'uppy-Container',
ref: (container) => {
this.container = container
},
...this.validProps,
...getHTMLProps(this.props),
})
}
}
Expand Down
6 changes: 2 additions & 4 deletions packages/@uppy/react/src/StatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class StatusBar extends Component {
if (prevProps.uppy !== this.props.uppy) {
this.uninstallPlugin(prevProps)
this.installPlugin()
} else if (nonHtmlPropsHaveChanged(this, prevProps)) {
} else if (nonHtmlPropsHaveChanged(this.props, prevProps)) {
const options = { ...this.props, target: this.container }
delete options.uppy
this.plugin.setOptions(options)
Expand Down Expand Up @@ -67,14 +67,12 @@ class StatusBar extends Component {
}

render () {
// TODO: stop exposing `validProps` as a public property and rename it to `htmlProps`
this.validProps = getHTMLProps(this.props)
return h('div', {
className: 'uppy-Container',
ref: (container) => {
this.container = container
},
...this.validProps,
...getHTMLProps(this.props),
})
}
}
Expand Down
9 changes: 2 additions & 7 deletions packages/@uppy/react/src/nonHtmlPropsHaveChanged.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
'use strict'

// TODO: replace with `Object.hasOwn` when dropping support for older browsers.
const hasOwn = (obj, key) => Object.prototype.hasOwnProperty.call(obj, key)

export default function nonHtmlPropsHaveChanged (component, prevProps) {
return Object.keys(component.props)
// TODO: replace `validProps` with an exported `Symbol('htmlProps')`.
.some(key => !hasOwn(component.validProps, key) && component.props[key] !== prevProps[key])
export default function nonHtmlPropsHaveChanged (props, prevProps) {
return Object.keys(props).some(key => !Object.hasOwn(props, key) && props[key] !== prevProps[key])
}
2 changes: 1 addition & 1 deletion packages/@uppy/redux-dev-tools/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export default class ReduxDevTools extends UIPlugin {
// Implement monitors actions
switch (message.payload.type) {
case 'RESET':
this.uppy.reset()
this.uppy.cancelAll()
return
case 'IMPORT_STATE': {
const { computedStates } = message.payload.nextLiftedState
Expand Down
8 changes: 0 additions & 8 deletions packages/@uppy/store-redux/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,3 @@ export function middleware () {
}

export default ReduxStore

// Backward compatibility: we want these to keep being available as static
// properties of `ReduxStore` to avoid a breaking change.
// TODO: remove these in the next semver-major.
ReduxStore.ReduxStore = ReduxStore
ReduxStore.STATE_UPDATE = STATE_UPDATE
ReduxStore.reducer = reducer
ReduxStore.middleware = middleware
2 changes: 1 addition & 1 deletion packages/@uppy/store-redux/src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('ReduxStore', () => {

it('can be created with named or default import', () => {
const r = createStore()
let store = new ReduxStore.ReduxStore({ store: r })
let store = new ReduxStore({ store: r })
expect(typeof store).toBe('object')
store = new ReduxStore({ store: r })
expect(typeof store).toBe('object')
Expand Down
12 changes: 4 additions & 8 deletions packages/@uppy/thumbnail-generator/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ export default class ThumbnailGenerator extends UIPlugin {
return Promise.all([onload, orientationPromise])
.then(([image, orientation]) => {
const dimensions = this.getProportionalDimensions(image, targetWidth, targetHeight, orientation.deg)
const rotatedImage = this.rotateImage(image, orientation)
const rotatedImage = rotateImage(image, orientation)
const resizedImage = this.resizeImage(rotatedImage, dimensions.width, dimensions.height)
return this.canvasToBlob(resizedImage, this.thumbnailType, 80)
return canvasToBlob(resizedImage, this.thumbnailType, 80)
})
.then(blob => {
return URL.createObjectURL(blob)
Expand Down Expand Up @@ -208,11 +208,12 @@ export default class ThumbnailGenerator extends UIPlugin {
*
* Returns a Canvas with the resized image on it.
*/
// eslint-disable-next-line class-methods-use-this
resizeImage (image, targetWidth, targetHeight) {
// Resizing in steps refactored to use a solution from
// https://blog.uploadcare.com/image-resize-in-browsers-is-broken-e38eed08df01

let img = this.protect(image)
let img = protect(image)

let steps = Math.ceil(Math.log2(img.width / targetWidth))
if (steps < 1) {
Expand Down Expand Up @@ -398,8 +399,3 @@ export default class ThumbnailGenerator extends UIPlugin {
}
}
}

// TODO: remove these methods from the prototype in the next major.
ThumbnailGenerator.prototype.canvasToBlob = canvasToBlob
ThumbnailGenerator.prototype.protect = protect
ThumbnailGenerator.prototype.rotateImage = rotateImage
Loading

0 comments on commit 9a213b5

Please sign in to comment.