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

@uppy/aws-s3: improve error when endpoint is not provided #5361

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jul 23, 2024

Fixes: #5358

Copy link
Contributor

Diff output files
diff --git a/packages/@uppy/aws-s3/lib/index.js b/packages/@uppy/aws-s3/lib/index.js
index f60848f..ff90845 100644
--- a/packages/@uppy/aws-s3/lib/index.js
+++ b/packages/@uppy/aws-s3/lib/index.js
@@ -395,6 +395,7 @@ export default class AwsS3Multipart extends BasePlugin {
     ).then(assertServerError);
   }
   getUploadParameters(file, options) {
+    _classPrivateFieldLooseBase(this, _assertHost)[_assertHost]("getUploadParameters");
     const {
       meta,
     } = file;

@Murderlon
Copy link
Member

Aren't the types also incorrect? We're saying it's possible to not pass any options but options are mandatory?

constructor(uppy: Uppy<M, B>, opts?: AwsS3MultipartOptions<M, B>) {

@aduh95 aduh95 merged commit 05d0789 into transloadit:main Jul 29, 2024
17 checks passed
@aduh95 aduh95 deleted the typeerror-s3-getUploadParameters branch July 29, 2024 09:59
@aduh95
Copy link
Contributor Author

aduh95 commented Jul 29, 2024

Aren't the types also incorrect? We're saying it's possible to not pass any options but options are mandatory?

I'm not sure, it's technically correct to do the following:

const uppy = new Uppy().use(AwsS3);
uppy.getPlugin(AwsS3).setOptions({ endpoint: 'http://example.com' });

@Murderlon
Copy link
Member

That could be said about any required option in any plugin. I think it's better to keep it mandatory or wdyt?

@github-actions github-actions bot mentioned this pull request Jul 30, 2024
github-actions bot added a commit that referenced this pull request Jul 30, 2024
| Package              | Version | Package              | Version |
| -------------------- | ------- | -------------------- | ------- |
| @uppy/audio          |   2.0.1 | @uppy/status-bar     |   4.0.1 |
| @uppy/aws-s3         |   4.0.2 | @uppy/store-default  |   4.1.0 |
| @uppy/compressor     |   2.0.1 | @uppy/transloadit    |   4.0.1 |
| @uppy/core           |   4.1.0 | @uppy/utils          |   6.0.1 |
| @uppy/dashboard      |   4.0.2 | @uppy/webcam         |   4.0.1 |
| @uppy/remote-sources |   2.1.0 | uppy                 |   4.1.0 |

- @uppy/remote-sources: fix options type (Merlijn Vos / #5364)
- @uppy/transloadit: do not mark `opts` as mandatory (Antoine du Hamel / #5375)
- @uppy/compressor: mark `quality` as optional (Antoine du Hamel / #5374)
- @uppy/aws-s3: improve error when `endpoint` is not provided (Antoine du Hamel / #5361)
- @uppy/core,@uppy/store-default: export `Store` type (Merlijn Vos / #5373)
- @uppy/dashboard: make `toggleAddFilesPanel` args consistent (Evgenia Karunus / #5365)
- @uppy/dashboard: Dashboard - convert some files to typescript (Evgenia Karunus / #5359)
- @uppy/audio,@uppy/webcam: Don't use `<h1>` in Uppy markup (Evgenia Karunus / #5355)
- @uppy/status-bar: GoldenRetriever + `hideUploadButton=true` (Evgenia Karunus / #5350)
- meta: Bump docker/build-push-action from 6.3.0 to 6.4.1 (dependabot[bot] / #5360)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uppy 4.0.5 error
2 participants