transloadit / uppy

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

uppy/core 3.3.1 throws error after new build #4940

Closed okkan closed 8 months ago

okkan commented 8 months ago

Initial checklist

Link to runnable example

No response

Steps to reproduce

we have been using 3.3.1 version for a long time, previous builds are working but today we built our env. uppy threw error.

... just install @uppy/core 3.3.1 with "@uppy/aws-s3": "3.2.1", "@uppy/dashboard": "3.5.0", "@uppy/drag-drop": "3.0.2", "@uppy/file-input": "3.0.2", "@uppy/progress-bar": "3.0.2", "@uppy/react": "3.1.3", "@uppy/tus": "3.1.2",

Expected behavior

it should work as expected

Actual behavior

it throws Error: Cannot find module '@uppy/core/lib/EventManager.js'

Murderlon commented 8 months ago

I can't reproduce here on the latest versions: https://codesandbox.io/p/sandbox/uppy-dashboard-xpxuhd?file=%2Fsrc%2Findex.js%3A28%2C23

Do you have a reproducible example on CodeSandbox or StackBlitz?

okkan commented 8 months ago

i dont have an example right now, also we cant use latest version because of p-queue issue (it doesnt work with meteorjs, https://github.com/transloadit/uppy/issues/4637), only working version was 3.3.1, without any change about uppy, our build started to throw error. Possibly you made a mistake with old versions.

Murderlon commented 8 months ago

Note that it's impossible to override versions on npm, so making a "mistake with old versions" can't be the case.

Uppy is ESM-only, as is p-queue, so it might be a bundler issue with meteorjs?

okkan commented 8 months ago

ok we found the solution,

we didnt have @uppy/utils in package.json, so it was using the latest version of it, we added it then we got another error about aws-s3-multipart, same thing we didnt have it in package.json even we dont use it, then we added it to package.json with the compatible version, it fixed.

sorry for taking your time.

Murderlon commented 8 months ago

You shouldn't put @uppy/utils in your package.json, it's an internal-only package and all packages that need it have it in their dependencies already.

Trying to manually resolve our internal dependencies will give you more problems sooner or later, it's probably better to look into your bundler.

okkan commented 8 months ago

but when we dont put it, it uses latest version of it and latest version of utils is not compatible with our 3.3.1 core version

rschlack commented 8 months ago

Hi Murderlon, I work with Okkan. If we don't put the @uppy/utils in our package file then utils/EventManager.js contains this line of code which references a file that does not exist in 3.3.1 core.

export { default } from '@uppy/core/lib/EventManager.js'

Murderlon commented 8 months ago

Our policy for breaking changes considers features/fixes which require changes across multiple packages to work not breaking. That means you should always upgrade packages together, not separately. You should either pin all the versions that work for you and stay on there or always upgrade packages together. You can't pin core and then keep installing newer versions of the other packages.

If you must, it's probably better to use overrides for npm or resolutions for yarn.

okkan commented 8 months ago

we didnt update or upgrade any package of uppy, you made a change in uppy/utils and it doesnt work with 3.3.1 so we had to put its an old version to package.json.

Murderlon commented 8 months ago

I'll talk to the team to see if something when wrong with versioning here.

Murderlon commented 8 months ago

EventManager is back in utils so if it resolved to the latest in your build then everything should be fine.

https://github.com/transloadit/uppy/pull/4952

rschlack commented 8 months ago

Thank you. This solved the issue.