Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meta: validate defaultOptions for stricter option types #4901

Merged
merged 3 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/@uppy/companion-client/src/Provider.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import type { Uppy, BasePlugin } from '@uppy/core'
import type { Body, Meta, UppyFile } from '@uppy/utils/lib/UppyFile'
import type { PluginOpts } from '@uppy/core/lib/BasePlugin.ts'
import RequestClient, {
authErrorStatusCode,
type RequestOptions,
} from './RequestClient.ts'
import * as tokenStorage from './tokenStorage.ts'

// TODO: remove deprecated options in next major release
export type Opts = {
export interface Opts extends PluginOpts {
/** @deprecated */
serverUrl?: string
/** @deprecated */
Expand Down
13 changes: 10 additions & 3 deletions packages/@uppy/core/src/BasePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,22 @@ import type { State, Uppy } from './Uppy'
export type PluginOpts = {
locale?: Locale
id?: string
[key: string]: unknown
}

export type OnlyOptionals<T> = Pick<
T,
{
// eslint-disable-next-line @typescript-eslint/ban-types
[K in keyof T]-?: {} extends Pick<T, K> ? K : never
}[keyof T]
>

/**
* DefinePluginOpts marks all of the passed AlwaysDefinedKeys as “required” or “always defined”.
*/
export type DefinePluginOpts<
Opts extends PluginOpts,
AlwaysDefinedKeys extends string,
Opts,
AlwaysDefinedKeys extends keyof OnlyOptionals<Opts>,
> = Opts & Required<Pick<Opts, AlwaysDefinedKeys>>

export default class BasePlugin<
Expand Down
4 changes: 2 additions & 2 deletions packages/@uppy/image-editor/src/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import getCanvasDataThatFitsPerfectlyIntoContainer from './utils/getCanvasDataTh
import getScaleFactorThatRemovesDarkCorners from './utils/getScaleFactorThatRemovesDarkCorners.ts'
import limitCropboxMovementOnMove from './utils/limitCropboxMovementOnMove.ts'
import limitCropboxMovementOnResize from './utils/limitCropboxMovementOnResize.ts'
import type { ImageEditorOpts } from './ImageEditor.tsx'
import type ImageEditor from './ImageEditor.tsx'

type Props<M extends Meta, B extends Body> = {
currentImage: UppyFile<M, B>
storeCropperInstance: (cropper: Cropper) => void
opts: ImageEditorOpts
opts: ImageEditor<M, B>['opts']
i18n: I18n
// eslint-disable-next-line react/no-unused-prop-types
save: () => void // eslint confused
Expand Down
34 changes: 22 additions & 12 deletions packages/@uppy/image-editor/src/ImageEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ declare module '@uppy/core' {
}
}

export interface Opts extends UIPluginOptions {
target: string | HTMLElement
interface Opts extends UIPluginOptions {
target?: string | HTMLElement
quality?: number
cropperOptions?: Cropper.Options & {
croppedCanvasOptions?: Cropper.GetCroppedCanvasOptions
Expand All @@ -54,21 +54,22 @@ export interface Opts extends UIPluginOptions {
cropWidescreenVertical?: boolean
}
}
export type { Opts as ImageEditorOptions }

type PluginState<M extends Meta, B extends Body> = {
currentImage: UppyFile<M, B> | null
}

const defaultCropperOptions = {
viewMode: 0,
viewMode: 0 as const,
background: false,
autoCropArea: 1,
responsive: true,
minCropBoxWidth: 70,
minCropBoxHeight: 70,
croppedCanvasOptions: {},
initialAspectRatio: 0,
} satisfies Opts['cropperOptions']
} satisfies Partial<Opts['cropperOptions']>

const defaultActions = {
revert: true,
Expand All @@ -80,7 +81,7 @@ const defaultActions = {
cropSquare: true,
cropWidescreen: true,
cropWidescreenVertical: true,
} satisfies Opts['actions']
} satisfies Partial<Opts['actions']>

const defaultOptions = {
target: 'body',
Expand All @@ -89,17 +90,26 @@ const defaultOptions = {
quality: 0.8,
actions: defaultActions,
cropperOptions: defaultCropperOptions,
} satisfies Opts

export type ImageEditorOpts = DefinePluginOpts<
Opts,
keyof typeof defaultOptions
>
} satisfies Partial<Opts>

type InternalImageEditorOpts = Omit<
DefinePluginOpts<Opts, keyof typeof defaultOptions>,
'actions' | 'cropperOptions'
> & {
actions: DefinePluginOpts<
NonNullable<Opts['actions']>,
keyof typeof defaultActions
>
cropperOptions: DefinePluginOpts<
NonNullable<Opts['cropperOptions']>,
keyof typeof defaultCropperOptions
>
}

export default class ImageEditor<
M extends Meta,
B extends Body,
> extends UIPlugin<ImageEditorOpts, M, B, PluginState<M, B>> {
> extends UIPlugin<InternalImageEditorOpts, M, B, PluginState<M, B>> {
static VERSION = packageJson.version

cropper: Cropper
Expand Down
2 changes: 2 additions & 0 deletions packages/@uppy/xhr-upload/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('XHRUpload', () => {
core.use(XHRUpload, {
id: 'XHRUpload',
endpoint: 'https://fake-endpoint.uppy.io',
// @ts-expect-error that option does not exist
some: 'option',
getResponseData,
})
Expand Down Expand Up @@ -65,6 +66,7 @@ describe('XHRUpload', () => {
core.use(XHRUpload, {
id: 'XHRUpload',
endpoint: 'https://fake-endpoint.uppy.io',
// @ts-expect-error that option doesn't exist
some: 'option',
validateStatus,
getResponseError(responseText) {
Expand Down
7 changes: 5 additions & 2 deletions packages/@uppy/xhr-upload/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ function setTypeInBlob<M extends Meta, B extends Body>(file: UppyFile<M, B>) {
}

const defaultOptions = {
endpoint: '',
formData: true,
fieldName: 'file',
method: 'post',
Expand All @@ -132,7 +131,7 @@ const defaultOptions = {
validateStatus(status) {
return status >= 200 && status < 300
},
} satisfies XhrUploadOpts<any, any>
} satisfies Partial<XhrUploadOpts<any, any>>

type Opts<M extends Meta, B extends Body> = DefinePluginOpts<
XhrUploadOpts<M, B>,
Expand Down Expand Up @@ -169,6 +168,8 @@ export default class XHRUpload<

// Simultaneous upload limiting is shared across all uploads with this plugin.
if (internalRateLimitedQueue in this.opts) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore untyped internal
this.requests = this.opts[internalRateLimitedQueue]
} else {
this.requests = new RateLimitedQueue(this.opts.limit)
Expand Down Expand Up @@ -608,6 +609,8 @@ export default class XHRUpload<

// No limit configured by the user, and no RateLimitedQueue passed in by a "parent" plugin
// (basically just AwsS3) using the internal symbol
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore untyped internal
if (this.opts.limit === 0 && !this.opts[internalRateLimitedQueue]) {
this.uppy.log(
'[XHRUpload] When uploading multiple files at once, consider setting the `limit` option (to `10` for example), to limit the number of concurrent uploads, which helps prevent memory and network issues: https://uppy.io/docs/xhr-upload/#limit-0',
Expand Down
Loading