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.

Sign up for GitHub

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

@uppy/core: add utility type to help define plugin option types #4885

Merged
merged 7 commits into from
Jan 23, 2024
+68 −27
Merged

@uppy/core: add utility type to help define plugin option types #4885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cb42f6e
@uppy/core: add utility type to help define plugin option types
aduh95 Jan 23, 2024
8bf26de
revert breaking change
aduh95 Jan 23, 2024
9677d5c
Update packages/@uppy/core/src/Uppy.test.ts
aduh95 Jan 23, 2024
711ae0f
Apply suggestions from code review
aduh95 Jan 23, 2024
4f32047
Update packages/@uppy/core/src/Uppy.test.ts
aduh95 Jan 23, 2024
86c1358
Merge branch 'main' of https://github.com/transloadit/uppy
aduh95 Jan 23, 2024
66ce658
fix types CI
aduh95 Jan 23, 2024
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
9 changes: 7 additions & 2 deletions packages/@uppy/core/src/BasePlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ export type PluginOpts = {
[key: string]: unknown
}

export type DefinePluginOpts<
Opts extends PluginOpts,
AlwaysDefinedKeys extends string,
> = Opts & Required<Pick<Opts, AlwaysDefinedKeys>>

export default class BasePlugin<
Opts extends PluginOpts,
M extends Meta,
Expand All @@ -43,9 +48,9 @@ export default class BasePlugin<

VERSION: string

constructor(uppy: Uppy<M, B>, opts?: Partial<Opts>) {
constructor(uppy: Uppy<M, B>, opts?: Opts) {
this.uppy = uppy
this.opts = (opts ?? {}) as Opts
this.opts = opts ?? ({} as Opts)
}

getPluginState(): PluginState {
Expand Down
80 changes: 58 additions & 22 deletions packages/@uppy/core/src/Uppy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import prettierBytes from '@transloadit/prettier-bytes'
import type { Body, Meta } from '@uppy/utils/lib/UppyFile'
import Core from './index.ts'
import UIPlugin from './UIPlugin.ts'
import BasePlugin, { type PluginOpts } from './BasePlugin.ts'
import BasePlugin, {
type DefinePluginOpts,
type PluginOpts,
} from './BasePlugin.ts'
import { debugLogger } from './loggers.ts'
import AcquirerPlugin1 from './mocks/acquirerPlugin1.ts'
import AcquirerPlugin2 from './mocks/acquirerPlugin2.ts'
Expand Down Expand Up @@ -64,30 +67,63 @@ describe('src/Core', () => {
})

it('should be able to .use() without passing generics again', () => {
interface TestOpts extends PluginOpts {
foo?: string
bar: string
{
interface TestOpts extends PluginOpts {
foo?: string
bar: string
}
class TestPlugin<M extends Meta, B extends Body> extends BasePlugin<
TestOpts,
M,
B
> {
foo: string
aduh95 marked this conversation as resolved.
Show resolved Hide resolved

bar: string

constructor(uppy: Core<M, B>, opts: TestOpts) {
super(uppy, opts)
this.id = 'Test'
this.type = 'acquirer'
this.foo = this.opts.foo ?? 'defaultFoo'
this.bar = this.opts.bar
}
}
new Core().use(TestPlugin)
new Core().use(TestPlugin, { foo: '', bar: '' })
// @ts-expect-error boolean not allowed
new Core().use(TestPlugin, { bar: false })
// @ts-expect-error missing option
new Core().use(TestPlugin, { foo: '' })
}
class TestPlugin<M extends Meta, B extends Body> extends BasePlugin<
TestOpts,
M,
B
> {
foo: string

constructor(uppy: Core<M, B>, opts: TestOpts) {
super(uppy, opts)
this.id = 'Test'
this.type = 'acquirer'
this.foo = opts?.foo ?? 'bar'

{
interface TestOpts extends PluginOpts {
foo?: string
bar?: string
}
const defaultOptions = {
foo: 'defaultFoo',
}
class TestPlugin<M extends Meta, B extends Body> extends BasePlugin<
DefinePluginOpts<TestOpts, keyof typeof defaultOptions>,
M,
B
> {
constructor(uppy: Core<M, B>, opts?: TestOpts) {
super(uppy, { ...defaultOptions, ...opts })
this.id = this.opts.id ?? 'Test'
this.type = 'acquirer'
}
}

new Core().use(TestPlugin)
new Core().use(TestPlugin, { foo: '', bar: '' })
new Core().use(TestPlugin, { foo: '' })
new Core().use(TestPlugin, { bar: '' })
// @ts-expect-error boolean not allowed
new Core().use(TestPlugin, { foo: false })
}
new Core().use(TestPlugin)
new Core().use(TestPlugin, { foo: '', bar: '' })
// @ts-expect-error boolean not allowed
new Core().use(TestPlugin, { foo: false })
// @ts-expect-error missing option
new Core().use(TestPlugin, { foo: '' })
})

it('should prevent the same plugin from being added more than once', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/@uppy/core/src/Uppy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1662,7 +1662,7 @@ export class Uppy<M extends Meta, B extends Body> {
/**
* Registers a plugin with Core.
*/
use<T extends typeof BasePlugin<any, M, B> | typeof UIPlugin<any, M, B>>(
use<T extends typeof BasePlugin<any, M, B>>(
Plugin: T,
opts?: ConstructorParameters<T>[1],
): this {
Expand Down
4 changes: 2 additions & 2 deletions packages/@uppy/status-bar/src/StatusBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Body, Meta, UppyFile } from '@uppy/utils/lib/UppyFile'
import type { Uppy, State } from '@uppy/core/src/Uppy.ts'
import type { DefinePluginOpts } from '@uppy/core/lib/BasePlugin.ts'
Copy link
Member

@Murderlon Murderlon Jan 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about exporting all types we want to export on @uppy/core? I think that's less confusing and more clean for implementers of plugins and users of uppy in general.

- import type { Body, Meta, UppyFile } from '@uppy/utils/lib/UppyFile'
- import type { Uppy, State } from '@uppy/core/src/Uppy.ts'
- import type { DefinePluginOpts } from '@uppy/core/lib/BasePlugin.ts'
+ import type { Uppy, State, DefinePluginOpts, Body, Meta, UppyFile } from '@uppy/core'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO DefinePluginOpts only makes sense for plugin implementer, so it makes sense to keep it in BasePlugin (we could also export it from UIPlugin) and we could keep @uppy/core types for end users only.

import { UIPlugin } from '@uppy/core'
import emaFilter from '@uppy/utils/lib/emaFilter'
import getTextDirection from '@uppy/utils/lib/getTextDirection'
Expand Down Expand Up @@ -72,8 +73,7 @@ const defaultOptions = {
* progress percentage and time remaining.
*/
export default class StatusBar<M extends Meta, B extends Body> extends UIPlugin<
StatusBarOptions &
Required<Pick<StatusBarOptions, keyof typeof defaultOptions>>,
DefinePluginOpts<StatusBarOptions, keyof typeof defaultOptions>,
Murderlon marked this conversation as resolved.
Show resolved Hide resolved
M,
B
> {
Expand Down
Loading