From 439787021bdab456310b9493c9f66e7ba95cada2 Mon Sep 17 00:00:00 2001 From: prakash Date: Mon, 22 Jun 2026 00:41:33 +0530 Subject: [PATCH 1/2] refactor(core): remove duplicate companion/provider bridge types, wire real types --- packages/@uppy/aws-s3/src/index.ts | 9 +-- packages/@uppy/core/src/Uppy.ts | 16 ++---- .../core/src/companion-client/Provider.ts | 25 +++++---- .../src/companion-client/RequestClient.ts | 17 +++--- .../src/companion-client/SearchProvider.ts | 9 ++- .../@uppy/core/src/companion-client/index.ts | 5 +- .../ProviderView/ProviderView.tsx | 39 +++++++++---- .../core/src/provider-views/utils/addFiles.ts | 6 +- .../utils/companionFileToUppyFile.ts | 6 +- .../core/src/utils/CompanionClientProvider.ts | 56 ------------------- packages/@uppy/core/src/utils/index.ts | 5 -- packages/@uppy/webdav/src/Webdav.tsx | 4 +- 12 files changed, 74 insertions(+), 123 deletions(-) delete mode 100644 packages/@uppy/core/src/utils/CompanionClientProvider.ts diff --git a/packages/@uppy/aws-s3/src/index.ts b/packages/@uppy/aws-s3/src/index.ts index a6fe7c5ee..6b2ef0b4d 100644 --- a/packages/@uppy/aws-s3/src/index.ts +++ b/packages/@uppy/aws-s3/src/index.ts @@ -5,14 +5,9 @@ import { type PluginOpts, type Uppy, } from '@uppy/core' +import type { RequestOptions } from '@uppy/core/companion-client' import { RequestClient } from '@uppy/core/companion-client' -import type { - Body, - LocalUppyFile, - Meta, - RequestOptions, - UppyFile, -} from '@uppy/core/utils' +import type { Body, LocalUppyFile, Meta, UppyFile } from '@uppy/core/utils' import { createAbortError, filterFilesToEmitUploadStarted, diff --git a/packages/@uppy/core/src/Uppy.ts b/packages/@uppy/core/src/Uppy.ts index 7a6485011..b06baabd6 100644 --- a/packages/@uppy/core/src/Uppy.ts +++ b/packages/@uppy/core/src/Uppy.ts @@ -7,9 +7,12 @@ import { nanoid } from 'nanoid/non-secure' import type { h } from 'preact' import packageJson from '../package.json' with { type: 'json' } import type BasePlugin from './BasePlugin.js' +import type Provider from './companion-client/Provider.js' +import type SearchProvider from './companion-client/SearchProvider.js' import getFileName from './getFileName.js' import locale from './locale.js' import { debugLogger, justErrorsLogger } from './loggers.js' +import type ProviderView from './provider-views/ProviderView/ProviderView.js' import type { Restrictions, ValidateableFile } from './Restricter.js' import { defaultOptions as defaultRestrictionOptions, @@ -20,8 +23,6 @@ import DefaultStore, { type Store } from './store/index.js' import supportsUploadProgress from './supportsUploadProgress.js' import type { Body, - CompanionClientProvider, - CompanionClientSearchProvider, CompanionFile, FileProgressNotStarted, FileProgressStarted, @@ -165,8 +166,6 @@ export interface BaseProviderPlugin { * UnknownProviderPlugin can be any Companion plugin (such as Google Drive) * that uses the Companion-assisted OAuth flow. * As the plugins are passed around throughout Uppy we need a generic type for this. - * It may seems like duplication, but this type safe. Changing the type of `storage` - * will error in the `Provider` class of @uppy/core/companion-client and vice versa. * * Note that this is the *plugin* class, not a version of the `Provider` class. * `Provider` does operate on Companion plugins with `uppy.getPlugin()`. @@ -178,16 +177,13 @@ export type UnknownProviderPlugin< BaseProviderPlugin & { rootFolderId: string | null files: UppyFile[] - provider: CompanionClientProvider - // Can't be typed unfortunately, we can't depend on `provider-views` in `core`. - view: any + provider: Provider + view: ProviderView } /* * UnknownSearchProviderPlugin can be any search Companion plugin (such as Unsplash). * As the plugins are passed around throughout Uppy we need a generic type for this. - * It may seems like duplication, but this type safe. Changing the type of `title` - * will error in the `SearchProvider` class of @uppy/core/companion-client and vice versa. * * Note that this is the *plugin* class, not a version of the `SearchProvider` class. * `SearchProvider` does operate on Companion plugins with `uppy.getPlugin()`. @@ -203,7 +199,7 @@ export type UnknownSearchProviderPlugin< B extends Body, > = UnknownPlugin & BaseProviderPlugin & { - provider: CompanionClientSearchProvider + provider: SearchProvider } // for better readability diff --git a/packages/@uppy/core/src/companion-client/Provider.ts b/packages/@uppy/core/src/companion-client/Provider.ts index bbea03aca..fe732d54f 100644 --- a/packages/@uppy/core/src/companion-client/Provider.ts +++ b/packages/@uppy/core/src/companion-client/Provider.ts @@ -5,13 +5,12 @@ import type { UnknownProviderPlugin, Uppy, } from '../index.js' -import { - type CompanionClientProvider, - getSocketHost, - type RequestOptions, -} from '../utils/index.js' +import { getSocketHost } from '../utils/index.js' import type { CompanionPluginOptions } from './index.js' -import RequestClient, { authErrorStatusCode } from './RequestClient.js' +import RequestClient, { + authErrorStatusCode, + type RequestOptions, +} from './RequestClient.js' export interface Opts extends PluginOpts, CompanionPluginOptions { pluginId: string @@ -31,10 +30,10 @@ function getOrigin() { return location.origin } -export default class Provider - extends RequestClient - implements CompanionClientProvider -{ +export default class Provider< + M extends Meta, + B extends Body, +> extends RequestClient { #refreshingTokenPromise: Promise | undefined provider: string @@ -273,11 +272,13 @@ export default class Provider } async login({ - uppyVersions, + uppyVersions = '', authFormData, signal, }: { - uppyVersions: string + // `uppyVersions` is optional because callers such as `ProviderView` do not + // have it on hand; the OAuth/simple-auth helpers always receive a string. + uppyVersions?: string authFormData: unknown signal: AbortSignal }): Promise { diff --git a/packages/@uppy/core/src/companion-client/RequestClient.ts b/packages/@uppy/core/src/companion-client/RequestClient.ts index 4e3bbf558..a5e70d329 100644 --- a/packages/@uppy/core/src/companion-client/RequestClient.ts +++ b/packages/@uppy/core/src/companion-client/RequestClient.ts @@ -1,13 +1,7 @@ import pRetry, { AbortError } from 'p-retry' import packageJson from '../../package.json' with { type: 'json' } import type Uppy from '../index.js' -import type { - Body, - Meta, - RemoteUppyFile, - RequestOptions, - UppyFile, -} from '../utils/index.js' +import type { Body, Meta, RemoteUppyFile, UppyFile } from '../utils/index.js' import { ErrorWithCause, fetchWithNetworkError, @@ -16,6 +10,15 @@ import { } from '../utils/index.js' import AuthError from './AuthError.js' +export type RequestOptions = { + method?: string + data?: Record + skipPostResponse?: boolean + signal?: AbortSignal + authFormData?: unknown + qs?: Record +} + type CompanionHeaders = Record | undefined export type Opts = { diff --git a/packages/@uppy/core/src/companion-client/SearchProvider.ts b/packages/@uppy/core/src/companion-client/SearchProvider.ts index 793bd33d6..3d9cd0615 100644 --- a/packages/@uppy/core/src/companion-client/SearchProvider.ts +++ b/packages/@uppy/core/src/companion-client/SearchProvider.ts @@ -1,5 +1,4 @@ import type { Body, Meta, Uppy } from '../index.js' -import type { CompanionClientSearchProvider } from '../utils/index.js' import RequestClient, { type Opts } from './RequestClient.js' const getName = (id: string): string => { @@ -9,10 +8,10 @@ const getName = (id: string): string => { .join(' ') } -export default class SearchProvider - extends RequestClient - implements CompanionClientSearchProvider -{ +export default class SearchProvider< + M extends Meta, + B extends Body, +> extends RequestClient { provider: string id: string diff --git a/packages/@uppy/core/src/companion-client/index.ts b/packages/@uppy/core/src/companion-client/index.ts index bdbf0b66e..0c968ed72 100644 --- a/packages/@uppy/core/src/companion-client/index.ts +++ b/packages/@uppy/core/src/companion-client/index.ts @@ -5,6 +5,9 @@ export type { CompanionPluginOptions } from './CompanionPluginOptions.js' export { default as getAllowedHosts } from './getAllowedHosts.js' export { default as Provider } from './Provider.js' -export { default as RequestClient } from './RequestClient.js' +export { + default as RequestClient, + type RequestOptions, +} from './RequestClient.js' export { default as SearchProvider } from './SearchProvider.js' export * as tokenStorage from './tokenStorage.js' diff --git a/packages/@uppy/core/src/provider-views/ProviderView/ProviderView.tsx b/packages/@uppy/core/src/provider-views/ProviderView/ProviderView.tsx index ece73bf93..f6dbe4451 100644 --- a/packages/@uppy/core/src/provider-views/ProviderView/ProviderView.tsx +++ b/packages/@uppy/core/src/provider-views/ProviderView/ProviderView.tsx @@ -66,6 +66,17 @@ const getDefaultState = ( type Optional = Pick, K> & Omit +/** + * Shape of the responses Companion returns from its `list` and `search` + * endpoints. The `Provider` methods are generic, so we pass this as the + * expected response type at the call sites. + */ +type ProviderListResponse = { + username: string + nextPagePath: string | null + items: CompanionFile[] +} + export interface Opts { provider: UnknownProviderPlugin['provider'] viewType: 'list' | 'grid' @@ -248,10 +259,13 @@ export default class ProviderView { await this.#withAbort(async (signal) => { const scopePath = currentFolder.type === 'root' ? undefined : currentFolderId - const { items } = await this.provider.search!(searchString, { - signal, - path: scopePath, - }) + const { items } = await this.provider.search( + searchString, + { + signal, + path: scopePath, + }, + ) // For each searched file, build the entire path (from the root all the way to the leaf node) // This is because we need to make sure all ancestor folders are present in the partialTree before we open the folder or check the file. @@ -389,10 +403,10 @@ export default class ProviderView { let currentPagePath = folderId let currentItems: CompanionFile[] = [] do { - const { username, nextPagePath, items } = await this.provider.list( - currentPagePath, - { signal }, - ) + const { username, nextPagePath, items } = + await this.provider.list(currentPagePath, { + signal, + }) // It's important to set the username during one of our first fetches this.plugin.setPluginState({ username }) @@ -478,10 +492,11 @@ export default class ProviderView { ) { this.isHandlingScroll = true await this.#withAbort(async (signal) => { - const { nextPagePath, items } = await this.provider.list( - currentFolder.nextPagePath, - { signal }, - ) + const { nextPagePath, items } = + await this.provider.list( + currentFolder.nextPagePath, + { signal }, + ) const newPartialTree = PartialTreeUtils.afterScrollFolder( partialTree, currentFolderId, diff --git a/packages/@uppy/core/src/provider-views/utils/addFiles.ts b/packages/@uppy/core/src/provider-views/utils/addFiles.ts index 31cee9d98..db962d092 100644 --- a/packages/@uppy/core/src/provider-views/utils/addFiles.ts +++ b/packages/@uppy/core/src/provider-views/utils/addFiles.ts @@ -1,8 +1,8 @@ +import type Provider from '../../companion-client/Provider.js' +import type SearchProvider from '../../companion-client/SearchProvider.js' import type { UnknownPlugin } from '../../index.js' import type { Body, - CompanionClientProvider, - CompanionClientSearchProvider, CompanionFile, Meta, UppyFileNonGhost, @@ -13,7 +13,7 @@ import companionFileToUppyFile from './companionFileToUppyFile.js' const addFiles = ( companionFiles: CompanionFile[], plugin: UnknownPlugin, - provider: CompanionClientProvider | CompanionClientSearchProvider, + provider: Provider | SearchProvider, ): void => { const uppyFiles = companionFiles.map((f) => companionFileToUppyFile(f, plugin, provider), diff --git a/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts b/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts index dee607c40..3886a9298 100644 --- a/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts +++ b/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts @@ -1,8 +1,8 @@ +import type Provider from '../../companion-client/Provider.js' +import type SearchProvider from '../../companion-client/SearchProvider.js' import type { UnknownPlugin } from '../../index.js' import type { Body, - CompanionClientProvider, - CompanionClientSearchProvider, CompanionFile, Meta, RemoteUppyFile, @@ -11,7 +11,7 @@ import type { const companionFileToUppyFile = ( file: CompanionFile, plugin: UnknownPlugin, - provider: CompanionClientProvider | CompanionClientSearchProvider, + provider: Provider | SearchProvider, ): RemoteUppyFile => { const name = file.name || file.id diff --git a/packages/@uppy/core/src/utils/CompanionClientProvider.ts b/packages/@uppy/core/src/utils/CompanionClientProvider.ts deleted file mode 100644 index 6cce7c558..000000000 --- a/packages/@uppy/core/src/utils/CompanionClientProvider.ts +++ /dev/null @@ -1,56 +0,0 @@ -import type { CompanionFile } from './CompanionFile.js' - -export type RequestOptions = { - method?: string - data?: Record - skipPostResponse?: boolean - signal?: AbortSignal - authFormData?: unknown - qs?: Record -} - -/** - * CompanionClientProvider is subset of the types of the `Provider` - * class from @uppy/core/companion-client. - * - * This is needed as the `Provider` class is passed around in Uppy and we - * need to have shared types for it. Although we are duplicating some types, - * this is still safe as `Provider implements CompanionClientProvider` - * so any changes here will error there and vice versa. - * - * TODO: remove this once companion-client and provider-views are merged into a single plugin. - */ -export interface CompanionClientProvider { - name: string - provider: string - login(options?: RequestOptions): Promise - logout(options?: RequestOptions): Promise - fetchPreAuthToken(): Promise - fileUrl: (a: string) => string - list( - directory: string | null, - options: RequestOptions, - ): Promise<{ - username: string - nextPagePath: string | null - items: CompanionFile[] - }> - // Optional: not every provider implements server-side search. - search?: ( - text: string, - options?: RequestOptions & { - path?: string | null | undefined - cursor?: string | undefined - }, - ) => Promise<{ - username: string - nextPagePath: string | null - items: CompanionFile[] - }> -} -export interface CompanionClientSearchProvider { - name: string - provider: string - fileUrl: (a: string) => string - search(text: string, queries?: string): Promise -} diff --git a/packages/@uppy/core/src/utils/index.ts b/packages/@uppy/core/src/utils/index.ts index 1b663a7ef..31c293293 100644 --- a/packages/@uppy/core/src/utils/index.ts +++ b/packages/@uppy/core/src/utils/index.ts @@ -3,11 +3,6 @@ export { AbortSignal, createAbortError, } from './AbortController.js' -export type { - CompanionClientProvider, - CompanionClientSearchProvider, - RequestOptions, -} from './CompanionClientProvider.js' export type { CompanionFile } from './CompanionFile.js' export { default as canvasToBlob } from './canvasToBlob.js' diff --git a/packages/@uppy/webdav/src/Webdav.tsx b/packages/@uppy/webdav/src/Webdav.tsx index 696c761ac..db87ff374 100644 --- a/packages/@uppy/webdav/src/Webdav.tsx +++ b/packages/@uppy/webdav/src/Webdav.tsx @@ -27,10 +27,10 @@ class WebdavSimpleAuthProvider extends Provider< > { async login({ authFormData, - uppyVersions, + uppyVersions = '', signal, }: { - uppyVersions: string + uppyVersions?: string authFormData: unknown signal: AbortSignal }) { From 1f4b8e187c8845fd899f0fecde3aa1027b4ec600 Mon Sep 17 00:00:00 2001 From: prakash Date: Wed, 24 Jun 2026 16:43:31 +0530 Subject: [PATCH 2/2] apply feedback --- packages/@uppy/core/src/Uppy.ts | 20 +++++++++++++++++-- .../core/src/provider-views/utils/addFiles.ts | 12 +++++++---- .../utils/companionFileToUppyFile.ts | 12 +++++++---- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/packages/@uppy/core/src/Uppy.ts b/packages/@uppy/core/src/Uppy.ts index b06baabd6..7a05862c6 100644 --- a/packages/@uppy/core/src/Uppy.ts +++ b/packages/@uppy/core/src/Uppy.ts @@ -177,7 +177,19 @@ export type UnknownProviderPlugin< BaseProviderPlugin & { rootFolderId: string | null files: UppyFile[] - provider: Provider + // Structural subset of the real `Provider` class,structural rather than the nominal class type so custom + // providers matching the public surface aren't forced to inherit from `Provider`. + provider: Pick< + Provider, + | 'name' + | 'provider' + | 'login' + | 'logout' + | 'fetchPreAuthToken' + | 'fileUrl' + | 'list' + | 'search' + > view: ProviderView } @@ -199,7 +211,11 @@ export type UnknownSearchProviderPlugin< B extends Body, > = UnknownPlugin & BaseProviderPlugin & { - provider: SearchProvider + // Structural subset of the real `SearchProvider` class (see note above). + provider: Pick< + SearchProvider, + 'name' | 'provider' | 'fileUrl' | 'search' + > } // for better readability diff --git a/packages/@uppy/core/src/provider-views/utils/addFiles.ts b/packages/@uppy/core/src/provider-views/utils/addFiles.ts index db962d092..8fc63a38b 100644 --- a/packages/@uppy/core/src/provider-views/utils/addFiles.ts +++ b/packages/@uppy/core/src/provider-views/utils/addFiles.ts @@ -1,6 +1,8 @@ -import type Provider from '../../companion-client/Provider.js' -import type SearchProvider from '../../companion-client/SearchProvider.js' -import type { UnknownPlugin } from '../../index.js' +import type { + UnknownPlugin, + UnknownProviderPlugin, + UnknownSearchProviderPlugin, +} from '../../index.js' import type { Body, CompanionFile, @@ -13,7 +15,9 @@ import companionFileToUppyFile from './companionFileToUppyFile.js' const addFiles = ( companionFiles: CompanionFile[], plugin: UnknownPlugin, - provider: Provider | SearchProvider, + provider: + | UnknownProviderPlugin['provider'] + | UnknownSearchProviderPlugin['provider'], ): void => { const uppyFiles = companionFiles.map((f) => companionFileToUppyFile(f, plugin, provider), diff --git a/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts b/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts index 3886a9298..678ed8d4d 100644 --- a/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts +++ b/packages/@uppy/core/src/provider-views/utils/companionFileToUppyFile.ts @@ -1,6 +1,8 @@ -import type Provider from '../../companion-client/Provider.js' -import type SearchProvider from '../../companion-client/SearchProvider.js' -import type { UnknownPlugin } from '../../index.js' +import type { + UnknownPlugin, + UnknownProviderPlugin, + UnknownSearchProviderPlugin, +} from '../../index.js' import type { Body, CompanionFile, @@ -11,7 +13,9 @@ import type { const companionFileToUppyFile = ( file: CompanionFile, plugin: UnknownPlugin, - provider: Provider | SearchProvider, + provider: + | UnknownProviderPlugin['provider'] + | UnknownSearchProviderPlugin['provider'], ): RemoteUppyFile => { const name = file.name || file.id