@uppy/aws-s3: add new S3 client but do not use it yet

[qxprakash]

Key Changes

• Removed accessKeyId and secretAccessKey from S3Config in types.ts
• Added signRequest callback option , so that consumers can provide their own signing implementation
• Deleted _sign() and _getSignatureKey() methods from S3.ts
• Modified _signedRequest() to invoke the signRequest callback instead of signing internally
• Replaced node:Buffer in _validateData() in s3.ts with ArrayBuffer , this was a node only data struct which was not supported on the browser , and was not supported on the Browser.
• Added test-utils/sigv4-signer.js a standalone SigV4 signing implementation for tests only.
• Refactored all test files to use createSigV4Signer() and pass it via the signRequest
• lots of other fixes to get the test , lint , typecheck to work.
• details on how to run tests using minio docker container , included in README.md
• there might be still some rough edges in the code, which I'm fixing.

Cleanup

• Removed Jest, added Vitest
• Removed Rollup build pipeline (now uses tsc)
• Removed tests for external S3 providers (Backblaze, Cloudflare, DigitalOcean, etc.)
• Removed performance benchmarks and Hono example
• Removed unused dev dependencies
• Kept only Minio integration tests with Docker Compose setup

Review Notes

go through the changes in order as mentioned above in Key Changes additionally I've marked the functions which I've modified in src/S3.ts as // ! MODIFIED

---

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: a1db477730b652fccba3dbfe91113bb094999905

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR

Some errors occurred when validating the changesets config:
The package or glob expression "example-aws-companion" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "example-aws-nodejs" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "example-aws-php" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "example-companion-digitalocean-spaces" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.

[qxprakash]

I think I didn't clearly mentioned my high level goal with this PR (I thought it was evident from the changes, but I’ll clarify it anyway )

This PR is meant to be small.

The goal was intentionally limited:

• Copy s3mini codebase and make it part of the Uppy monorepo , wire up lint , tests etc
• Migrate Jest -> Vitest, keep only Minio tests , refactor and make tests pass to use the signRequest callback.
• Remove dependence of @node/types for browser compatibility
• Key change: Remove existing _sign() method -> replace with signRequest callback

I kept methods like createBucket, listObjects, copyObject, etc. because removing them would break the existing test suite. requiring us to refactor / delete the tests again I wanted to keep the changes minimal

Now coming to your questions ,

Most importantly, this should not be a new package, this should be a folder inside @uppy/aws-s3, there is no need for publishing more packages again

Yes It won't be , it will be inside @uppy-aws-s3 , I just kept it separate just for now , so it's easier to iterate

Feels like this was done too quickly with AI without thinking critically what we need for Uppy.

I'm curious to know what made you think this ? can you point me to specific code ? , have I started writing code like AI 🤣 , FYI : all the code is hand-rolled no AI is used in it. btw , if you closesly look at the changes and compare it to the original client you'll find I've borrowed most of the code from _sign into createSigV4Signer , small refactor in singedRequest which now uses the callback instead of _sign to get the singed headers that's it.

I would also expect this PR to have all new tests green in CI

you mean new unit tests ?

[qxprakash]

i'm not sure but it looks to me like none of the tests are running in CI:

@uppy/aws-s3-client:test
  cache bypass, force executing 0e22966801f1dd72
  
   RUN  v3.2.4 /home/runner/work/uppy/uppy/packages/@uppy/aws-s3-client
  
  [[email protected]] injecting env (0) from .env -- tip: 🔐 encrypt with Dotenvx: https://dotenvx.com/
  stdout | tests/minio.test.js
  [[email protected]] injecting env (0) from .env -- tip: ⚙️  override existing env vars with { override: true }
  
  stderr | tests/minio.test.js
  No credentials found. Please set the BUCKET_ENV_ environment variables.
  
   ↓ tests/minio.test.js (1 test | 1 skipped)
  
   Test Files  1 skipped (1)
        Tests  1 skipped (1)
     Start at  12:32:47
     Duration  383ms (transform 115ms, setup 0ms, collect 127ms, tests 0ms, environment 0ms, prepare 62ms)

https://github.com/transloadit/uppy/actions/runs/19763867805/job/56631908294?pr=6073

Yes, they won’t run because the we haven’t configured the environment variables for the bucket. you can spin up a minio container and then run the tests against it (when running locally). I’ve added the details in README regarding this. we can also configure the credentials for a hosted minio instance in Github action for tests.

[mifi]

Yes, they won’t run because the we haven’t configured the environment variables for the bucket. you can spin up a minio container and then run the tests against it (when running locally). I’ve added the details in README regarding this. we can also configure the credentials for a hosted minio instance in Github action for tests.

IMO we should run tests in CI. If it's hard to get minio running, maybe we could simulate S3 using a node http server, or find a project that already does this

[qxprakash]

Yes, they won’t run because the we haven’t configured the environment variables for the bucket. you can spin up a minio container and then run the tests against it (when running locally). I’ve added the details in README regarding this. we can also configure the credentials for a hosted minio instance in Github action for tests.

IMO we should run tests in CI. If it's hard to get minio running, maybe we could simulate S3 using a node http server, or find a project that already does this

It's running now @mifi , I didn't thought it would be this easy , apparently github action runner ( ubuntu-latest ) comes with docker compose pre-installed, and our test suite uses docker compose so it was seamless , I previously thought we might need to spin up another runner specifically for minio

[qxprakash]

We should strictly keep only those methods in the S3Mini class that are related to file uploads. All other methods and their related tests should be removed ( as Merlijn pointed above ). I’ll add new tests for the methods which I'll implement. Currently, part of the test setup depends on a few bucket level methods for example :

https://github.com/qxprakash/uppy/blob/5a1669623b116d2f09b770ba8b33e72cabbd9a36/packages/%40uppy/aws-s3/tests/s3-client/_shared.test.js#L58

which we won’t need in Uppy. I think It’s better to clean these up now than later.