transloadit / uppy

The next open source file uploader for web browsers :dog:
https://uppy.io
MIT License
29.17k stars 2.01k forks source link

`@uppy/vue` requires installing dashboard even if it's not used #4089

Open a-kriya opened 2 years ago

a-kriya commented 2 years ago

Initial checklist

Link to runnable example

No response

Steps to reproduce

Expected behavior

Successful build.

@uppy/dashboard, @uppy/drag-drop, @uppy/file-input, @uppy/progress-bar, and @uppy/status-bar are now peer dependencies. This means you don’t install all these packages if you only need one. To migrate: install only the packages you need. If you use the Dashboard component, you need @uppy/dashboard, and so onwards.

From https://uppy.io/docs/migration-guides.html#uppy-vue

Actual behavior

Build failure:

image

TomaszPilch commented 2 years ago

same problem with @uppy/react

aduh95 commented 2 years ago

The workaround is to import the individual modules:

import DragDrop from '@uppy/vue/lib/drag-drop.js'
import StatusBar from '@uppy/vue/lib/status-bar.js'

We should probably update the docs to only show imports that targets the individual components modules. We also still have the possibility to put back the uppy plugins as direct dependencies instead of peers in a semver-minor, so if someone feels like it would be a better option, please speak up.

Murderlon commented 2 years ago

The workaround is to import the individual modules:

import DragDrop from '@uppy/vue/lib/drag-drop.js'
import StatusBar from '@uppy/vue/lib/status-bar.js'

I didn't know these kind of imports were required when you use peer dependencies. If we can't import from @uppy/vue directly, I'm not sure if it makes sense.

a-kriya commented 2 years ago

We also still have the possibility to put back the uppy plugins as direct dependencies instead of peers in a semver-minor

Just for the sake of simplicity of using it, I would prefer that. Would dashboard not get tree-shaken if I were to import {DragDrop} from '@uppy/vue'?

Murderlon commented 2 years ago

It would be tree-shaken. But the install footprint is bigger. I think we are going to try to add exports map to see if we can have normal imports with peer deps

paulm17 commented 2 years ago

@aduh95. I'm using my own UI for uppy. Strictly using uppy as the infra so to speak.

This was my packages:

"devDependencies": {
    "crypto-js": "4.0.0",
    "@uppy/core": "2.3.3",
    "@uppy/golden-retriever": "2.1.1",
    "@uppy/react": "2.2.3",
    "@uppy/tus": "2.4.3",
    "typescript": "~4.5"
  }

Upgrading from 2.3.3 to the latest is now asking me to install dashboard and then uppy/drag-drop and that's when I reverted. I won't be using any of the new features, I don't need them. So I shouldn't have to install the unneeded packages?

Thanks!

a-kriya commented 2 years ago

@paulm17 It's still installing them automatically for you, since they're direct dependencies of @uppy/react@2.x.x rather than peer-deps in 3.x.x.

paulm17 commented 2 years ago

@paulm17 It's still installing them automatically for you, since they're direct dependencies of @uppy/react@2.x.x rather than peer-deps in 3.x.x.

@a-kriya If that's the case. Why am I getting, when I upgrade the dependencies I listed above:

https://nextjs.org/docs/messages/module-not-found
info  - automatically enabled Fast Refresh for 1 custom loader
wait  - compiling /_error (client and server)...
error - ../../node_modules/@uppy/react/lib/Dashboard.js:2:0
Module not found: Can't resolve '@uppy/dashboard'

Anyway, let me clarify so there is no confusion.

We also still have the possibility to put back the uppy plugins as direct dependencies

I would vote to revert back to how 2.x handles this issue.

a-kriya commented 2 years ago

@paulm17 Things changed in v3 as I mentioned above. What I'll say is, you can upgrade to v3 and the only additional thing you have to do is list @uppy/dashboard and perhaps one other package in your package.json. Those packages were already getting installed in v2. To confirm, you can run the following command for both v2 and v3:

npm ls @uppy/dashboard

And you'll notice that it's there in both cases.

I would vote to revert back to how 2.x handles this issue.

Yep, that is what the maintainers will figure out for this ticket.

aduh95 commented 2 years ago

@paulm17 to clarify, you don't have to download all the Uppy plugins if you change your code to only import the things you need – if you import {} from '@uppy/react', it tries to load everything, if instead you load the file individually (e.g. import useUppy from '@uppy/react/lib/useUppy.js'), you wouldn't see the error.

aduh95 commented 2 years ago

@paulm17 to clarify, you don't have to download all the Uppy plugins if you change your code to only import the things you need – if you import {} from '@uppy/react', it tries to load everything, if instead you load the file individually (e.g. import useUppy from '@uppy/react/lib/useUppy.js'), you wouldn't see the error.

paulm17 commented 2 years ago

@aduh95

I did not know that! Works and I'm a happy camper, thanks a lot! 🥳

You can disregard my previous comment, as this change makes it superfluous.

a-kriya commented 2 years ago

The workaround is to import the individual modules:

import DragDrop from '@uppy/vue/lib/drag-drop.js'
import StatusBar from '@uppy/vue/lib/status-bar.js'

Requires additional declaration in a separate .d.ts file when importing in a TS project.

aduh95 commented 2 years ago

The workaround is to import the individual modules:

import DragDrop from '@uppy/vue/lib/drag-drop.js'
import StatusBar from '@uppy/vue/lib/status-bar.js'

Requires additional declaration in a separate .d.ts file when importing in a TS project.

We do include those .d.ts file in the npm package (e.g. @uppy/vue/types/drag-drop.d.ts), I'm not sure what we can do to make TS recognize that this file references the module in the lib/ folder, something like a declare module I think, but help is welcome is someone knows what need to be done and wants to open a PR.

entinio commented 1 year ago

1 year later, and this issue still isn't solved...

Using import DragDrop from '@uppy/vue/lib/drag-drop.js' only still lands on a typescript error as well:

Could not find a declaration file for module '@uppy/vue/lib/drag-drop.js'.

Murderlon commented 1 year ago

@entinio feel free to send a pull request