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

Merge @uppy/companion-client and @uppy/provider-views #4936

Open
2 tasks done
Murderlon opened this issue Feb 20, 2024 · 0 comments
Open
2 tasks done

Merge @uppy/companion-client and @uppy/provider-views #4936

Murderlon opened this issue Feb 20, 2024 · 0 comments
Labels

Comments

@Murderlon
Copy link
Member

Initial checklist

  • I understand this is a feature request and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Problem

Separated plugins

provider-views and companion-client always belong together and operate on each other, but they are separate plugins, and not direct dependencies of each other. Most code just references code of the other plugin, but we can't use types from the other plugin as we don't have access to it.

To prevent incorrect types and introducing a breaking change by merging the packages, I deemed the best approach to duplicate some of the types into utils (CompanionClientProvider, CompanionFile) and create UnknownProviderPlugin and UnknownSearchProviderPlugin in core.

Faulty inheritance

Another place of needless complexity, is the View base class, which is used for ProviderView and SearchProviderView (that was my abstraction 😬🔫 ). View sets the provider property on the class, but that's different for ProviderView and SearchProviderView. Some type trickery is needed for this.

The "tag file"

In Uppy we have UppyFile, CompanionFile (introduced here), and Tagfile. The last one no one understands why it's there (as seen from an existing todo comment). Having this third type going around the codebase started posing problems, as now Restricter and some methods on Uppy now needed to deal with this.

For Restricter I solved this with yet another type: ValidateableFile. There had to be a type to indicate the minimal amount of properties present in order to validate it, then all file types can have overlap with it. Unfortunately RestrictionError has to maintain UppyFile, so only in Restricter a cast from ValidateableFile to UppyFile is required to please TS.

Solution

  • Merge the two plugins
  • Remove the View class in favor of composition.
  • Remove the "tag file" concept. It should be possible to use UppyFile instead.

Alternatives

Don't merge the plugins, only fix the View and "tag file" problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant