-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Simplify Uppy #5509
Comments
I agree with the problems but not with the outcome. For a long time I've been weighing the cost/benefit of a complete rewrite vs. incremental improvement. Complete rewrites almost never make sense for mature, big SaaS platforms but for OSS libraries it's a bit different and more common. Nonetheless I dismissed that idea after the refactor to TS, which took longer than expected, and to really approach the justification from a user perspective. Most of your problems are related to how we experience the codebase. While that should be taken seriously as well, that only comes second in importance to what value we deliver to users. The majority of your suggestions is just moving around code in a time consuming way.
No strong feelings about this. It's an internal-only dependency so users don't care.
Hard disagree. 90% of the users don't need another language and we inflate the bundle size significantly.
Note that versioning and releasing should be taken into account. Plugins can depend on different versions of utils but core must be the same. That means tooling needs to be build to release all plugins to bump the core dependency whenever core changes. So not sure about this.
Has it broken that many times? Sounds like an XY problem, if something breaks moving the code together doesn't make it not break. It's like you said, forget to test with it, so we should write tests and have that certainty with less effort?
Open to the idea. But remember that very few users need all or almost all remote sources so having 2 maybe 3 times
I agree with this
This is actually the biggest user pain point of them all. That's why I created #5379, which deserves its own issue and discussion. That's a massive undertaking which requires lots of thought and breakdown, more than a single sentence here.
This is a great and essential plugin. Also it's not a component? It's about managing your existing form submit with file uploads.
Superseded by #5379
Superseded by #5379
We can consider this yes 👍
It's the CDN bundle, another essential package. I've been thinking about Uppy's vision for a long, long time. And what I settled on as the most value for invested time is a future where users:
That's it. Laser focus on delivering value, not moving code around for us, no assumptions on what could be better but based on actual concerns across GitHub, Community Forum, and Intercom. |
I'm not talking about a complete rewrite though, but yea I see that some could be quite big changes. However some are not that crazy and I think can still be done incrementally. but they are all very breaking changes for the user of uppy (developer, not end user). I think with proper types, we can more confidently do refactors like this, so it was a good investment even though it took a long time.
not sure what you mean by users, but if you mean end users who use the webapp that in turn uses Uppy, yea they don't care much about the code. when i say user i'm thinking about the developer who is using Uppy in their project. I think having a simple structure of plugins/packages to choose from makes it easier for them to use Uppy. When I see 50 packages a get a bit overwhelmed. but yea i agree it's mostly of a problem for us (and contributors).
Even if the total bundle size of the npm package
I think npm/semver solves this?
Yes, so I think we agree then, right? calling
not sure why the CDN bundle needs to be an npm module that we publish to npm. can the CDN bundle be installed from npm? I agree with your priorities, but I still think some of the things here could be done without too much effort (although still probably breaking changes). for me what triggered this need was:
|
I mostly agree with you but as mentioned I think we have bigger fish to fry. I appreciate you taking the time in sharing thoughts and suggestions but in the context of having barely any maintainer hours across the board, it would help us a lot more if you wrote an issue per problem/solution instead of a massive one that can practically never be considered done. For your own understanding and ours, I highly recommend using the pitch format (without the sketches) to write issue(s) like these. Another example is how we present bigger initiatives within unified with RFCs. Or the effort I put into #5379. If you'd think critically and you'd only get to make one, at most two, initiatives to improve some of the problems you described what would it be? Then break it down and present it as convincingly as possible. As mentioned, since this issue can never be considered done I'll close it but the discussion can continue here or elsewhere. |
One small contribution from my side
Not saying you should (i think you shouldn't, but yes I think you can do, e.g.: https://cdn.jsdelivr.net/npm/[email protected]/dist/uppy.min.js Overall I think there are good and actionable insights in here. What would be the top ranking actionable thing we could do you think Mikael? Then we can turn that into an issue. Take it one by one until we reach the point where we collectively feel things left are too high hanging fruits. These seem very painful in particular for any meaningful dev velocity and feel like maybe could use a dash of Remco doing another iteration on the monorepo
|
If I have to choose only one, then I think #5379 is probably the one that gives the most value for users and for us, but that is a huge task. Other than that, I think are the ones that require the least effort are the removal of the packages @uppy/aws-s3 @uppy/remote-sources @uppy/store-default @uppy/utils @uppy/locales uppy @uppy/redux-store @uppy/redux-dev-tools @uppy/svelte @uppy/angular @uppy/vue @uppy/react (or a subset of these that don't require much effort) and maybe see if we can reduce the number of tsconfig files to additionally speed up typescript. do we need two sets of files for each plugin (tsconfig.build.json and tsconfig.json)? |
I meant pick one that doesn't have an issue yet. Something extra you'd like to see happen. Regarding moving packages, I still don't see how it improves the situation. We still end up with the same amount of code? I would like to see additional benefits described, such as with #4936. If you see tangible benefits here, perhaps pitch this in a new issue? Using the lord's editor (Neovim) all my TS suggestions are instant so we'd have to look deeper to see what really causes this for you. Regarding tsconfig files, we should keep two tsconfig's per package. One of the reasons our monorepo is slow and complex is precisely because we tried to do everything from the top instead of per package. Settings are also different per package, such as react, vue, svelte, angular, react native, etc. It's best practice to have two tsconfig files per package. |
for those that we remove, obviously we will end up with less code to maintain. for those that we merge into core, we will end up with a simpler codebase with less boilerplate and less build steps. I tried to argue why I think the codebase should be simplified in this issue - I don't have time to make a proper issue/pitch now, but I will see if I can debug my intellisense experience, as it's probably something with my setup, if it's fast for you.
you have a source for this? |
We need to be able to make a distinction for what we build and publish to npm (the source) and type checking non-source files such as tests. For instance, as we've done here tus/tus-node-server#641 |
Problem
Uppy has become somewhat unwieldy and has much more packages than it needs to have, being a relatively simple thing, an uploader library.
Solution
IMO we should reduce the number of packages/plugins.
Candidates:
@uppy/store-default
: merge into core@uppy/locales
merge into core, but still allow people to import specific locales (paths)@uppy/utils
: merge into core@uppy/golden-retriever
,@uppy/compressor
,@uppy/thumbnail-generator
: I feel like these should be part of core functionality in Uppy. as it is now, it has broken so many times because of the complexity and because we forget to test with it.@uppy/remote-sources
: remove this plugin - instead make the API for adding multiple remote sources to uppy core easier to use@uppy/provider-views
,@uppy/companion-client
: merge into one (already suggested in Merge@uppy/companion-client
and@uppy/provider-views
#4936)@uppy/file-input
@uppy/drag-drop
, and@uppy/drop-target
- similarly to@uppy/form
, doesn't have to be a plugin. Remove the plugin and make an easy to use API with good example code, or make a headless component@uppy/status-bar
,@uppy/progress-bar
: merge into dashboard, instead allow people to make their own UI by providing headless components or easy to integrate state with example code@uppy/form
gives people too little freedom. should either be removed and converted into example code, or made into a headless component@uppy/informer
merge into dashboard.@uppy/image-editor
merge into dashboard.@uppy/aws-s3
already deprecated and it's currently just an alias - remove ituppy
: not sure why we need this package, I think it can be removedMaybe more controversial:
@uppy/redux-store
: provide example code instead. (i don't think this is very popular any more, and can probably be replaced with useUppyState)@uppy/redux-dev-tools
: remove this plugin@uppy/svelte
: remove this plugin (there seems to be no source code here)@uppy/angular
,@uppy/vue
,@uppy/react
: remove and provide example code instead. Merge essential helpers like useUppyState into core.Somewhat related to #5379
Alternatives
Only some of the above suggestions.
The text was updated successfully, but these errors were encountered: