feature(multi-bucket): add multi-bucket support to storage components #5562

jordanvn · 2024-08-10T03:47:42Z

Description of changes

Changes:

Adds optional property "bucket" to <StorageImage /> and <StorageManager />.
- This property accepts the argument of either:
  - A string of the "friendly name" of the bucket
  - A StorageBucket object containing the bucket name generated on S3 and the region it exists in
- If omitted, these storage components will use whichever bucket the user has designated as their "default" bucket in their configuration.

Issue #, if available

N/A

Description of how you validated changes

Functionality of the backend support of multiple buckets was validated
Example apps were modified to use multi-bucket setup
Multi-bucket support was confirmed in example apps

Checklist

Have read the Pull Request Guidelines
PR description included
yarn test passes and tests are updated/added
PR title and commit messages follow conventional commit syntax
If this change should result in a version bump, changeset added (This can be done after creating the PR.) This does not apply to changes made to docs, e2e, examples, or other private packages.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

changeset-bot · 2024-08-10T03:47:46Z

🦋 Changeset detected

Latest commit: b2349b1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@aws-amplify/ui-react-storage	Minor

Not sure what this means? Click here to learn what changesets are.

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

dbanksdesign

Looks good!

dbanksdesign · 2024-08-13T17:01:21Z

packages/react-storage/src/components/StorageManager/types.ts

@@ -12,6 +12,15 @@ import {
 } from './ui';
 import { StorageManagerDisplayText, PathCallback, UploadTask } from './utils';

+interface BucketInfo {


Nit: should we share this type between components?

Yes, that makes sense to me. It didn't seem substantial enough to create a new shared types file for storage components, so I went with the definition in the Storage Manager types. There are already exported types and interfaces in that file, and none aside from props in Storage Image types.

Can we just import and extend FileUploaderProps in the assignment of StorageManagerProps and dedupe this file?

Are you suggesting the removal of this file and providing its exports from FileUploader types? Or reducing StorageManagerProps to export interface StorageManagerProps extends FileUploaderProps {}?

Keeping this file and doing the latter

Got it. Thanks for the clarification

Updated file to extend FileUploaderProps instead of duplicating type declaration

jordanvn · 2024-09-26T22:53:05Z

docs/src/pages/[platform]/connected-components/storage/fileuploader/examples/BucketExact.tsx

+    <FileUploader
+      acceptedFileTypes={['image/*']}
+      bucket={{
+        bucketName: 'my-bucket-96e87892835c413e9963f3004a44e1ff',


Note: this is not an actual bucket name

Can you add a note so we don't wonder about that later, or make it more obvious like "my-bucket-xxxxxxx"

Yep, I like going with "my-bucket-xxxxxxxx"

jordanvn · 2024-09-26T22:56:38Z

packages/react-storage/src/components/StorageImage/__tests__/StorageImage.test.tsx

@@ -170,6 +170,27 @@ describe('StorageImage', () => {
    );
  });

+  it('should pass bucket to getUrl when supplied', () => {


Note: we're not duplicating the testing of the functionality of the API supporting this, so this test is just confirming that the "bucket" property is successfully passed from the component to the API in the expected format.

calebpollman

Looking good! Minor feedback, might make sense to add a FileUploader example app as well imo

calebpollman · 2025-01-13T19:27:32Z

packages/react-storage/src/components/FileUploader/hooks/useUploadFiles/useUploadFiles.ts

+import { StorageBucket } from '../../types';
 import { FileStatus } from '../../types';
 import { FileUploaderProps } from '../../types';


These can be consolidated to a single import statement (same with the '../../utils' imports)

calebpollman · 2025-01-13T19:58:33Z

packages/react-storage/src/components/FileUploader/types.ts

  /**
   * Component overrides
   */
+  bucket?: never;


The jsdoc description on lines 80-82 is for components, would define bucket last on the interface since there is no documentation for it on this interface

calebpollman · 2025-01-13T19:59:42Z

packages/react-storage/src/components/FileUploader/utils/__tests__/getInput.spec.ts

@@ -60,6 +60,7 @@ describe('getInput', () => {
    const expected: UploadDataWithPathInput = {
      data: file,
      options: {
+        bucket: undefined,


Why do we need to add explicit bucket: undefined to the test inputs in this file?

calebpollman · 2025-01-13T21:01:09Z

packages/react-storage/src/components/StorageImage/types.ts

@@ -36,6 +38,7 @@ export interface StorageImageProps extends Omit<ImageProps, 'src'> {

 type OmittedPropKey =
  | 'accessLevel'
+  | 'bucket'


Think the intention of the omission would be clearer if documented the same way as path

calebpollman · 2025-01-13T21:02:38Z

packages/react-storage/src/components/StorageManager/types.ts

  /**
   * S3 bucket key, allows either a `string` or a `PathCallback`:
   * - `string`: `path` is prefixed to the file `key` for each file
   * - `PathCallback`: callback provided an input containing the current `identityId`,
   *    resolved value is prefixed to the file `key` for each file
   */
+  bucket?: StorageBucket;


The description here is for the path prop 😅

initial commit to add 'bucket' property to storage components

82099b3

jordanvn changed the title ~~feature(multi-bucket): initial commit to add 'bucket' property to storage components~~ feature(multi-bucket): add 'bucket' property to storage components Aug 10, 2024

jordanvn changed the title ~~feature(multi-bucket): add 'bucket' property to storage components~~ feature(multi-bucket): add multi-bucket support to storage components Aug 10, 2024

chore: use StorageBucket type in StorageImagePathProps

f45e688

dbanksdesign reviewed Aug 13, 2024

View reviewed changes

jordanvn added 15 commits August 20, 2024 08:06

remove duplicate StorageBucket type declaration

130914d

chore: update aws-amplify version to include multi-bucket support

6759502

docs: include references to new 'bucket' prop and its usage

6d200a9

Merge branch 'main' into multi-bucket-support

77de102

more explicitly clarifying that can be a string in docs example

5ea81d5

Merge branch 'main' into multi-bucket-support

98b51db

chore: changing reference of storage manager to file uploader

79d1209

chore: updating yarn.lock

9916524

chore: undoing unnecessary linting changes

bb0c2f7

chore: moving yarn.lock from main branch parity

0fab899

Merge branch 'main' into multi-bucket-support

77119df

chore: updating yarn.lock to main

cd12258

chore: add missing references to 'bucket'

4c6eae2

chore: adding tests and new example app

d36d03e

Merge branch 'main' into multi-bucket-support

f3953b6

jordanvn temporarily deployed to ci September 26, 2024 22:46 — with GitHub Actions Inactive

jordanvn had a problem deploying to ci September 26, 2024 22:46 — with GitHub Actions Failure

jordanvn temporarily deployed to ci September 26, 2024 22:46 — with GitHub Actions Inactive

jordanvn marked this pull request as ready for review September 26, 2024 22:49

jordanvn requested a review from a team as a code owner September 26, 2024 22:49

jordanvn marked this pull request as draft September 26, 2024 22:51

jordanvn commented Sep 26, 2024

View reviewed changes

jordanvn temporarily deployed to ci October 10, 2024 22:00 — with GitHub Actions Inactive

Merge branch 'main' into multi-bucket-support

2895e9b

jordanvn temporarily deployed to ci October 11, 2024 19:22 — with GitHub Actions Inactive

Merge branch 'main' into multi-bucket-support

cc79b45

jordanvn temporarily deployed to ci October 11, 2024 21:05 — with GitHub Actions Inactive

Merge branch 'main' into multi-bucket-support

23e7619

jordanvn temporarily deployed to ci December 5, 2024 22:53 — with GitHub Actions Inactive

dbanksdesign approved these changes Dec 13, 2024

View reviewed changes

Merge branch 'main' into multi-bucket-support

b2349b1

calebpollman temporarily deployed to ci January 13, 2025 19:40 — with GitHub Actions Inactive

calebpollman had a problem deploying to ci January 13, 2025 19:40 — with GitHub Actions Failure

calebpollman temporarily deployed to ci January 13, 2025 19:40 — with GitHub Actions Inactive

Merge branch 'main' into multi-bucket-support

2674696

jordanvn temporarily deployed to ci January 16, 2025 18:22 — with GitHub Actions Inactive

calebpollman reviewed Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(multi-bucket): add multi-bucket support to storage components #5562

feature(multi-bucket): add multi-bucket support to storage components #5562

jordanvn commented Aug 10, 2024

changeset-bot bot commented Aug 10, 2024 •

edited

Loading

dbanksdesign left a comment

dbanksdesign Aug 13, 2024

jordanvn Aug 20, 2024

calebpollman Sep 30, 2024

jordanvn Sep 30, 2024

calebpollman Sep 30, 2024

jordanvn Sep 30, 2024

jordanvn Sep 30, 2024

jordanvn Sep 26, 2024

reesscot Sep 30, 2024

jordanvn Sep 30, 2024

jordanvn Sep 26, 2024

calebpollman left a comment

calebpollman Jan 13, 2025

calebpollman Jan 13, 2025

calebpollman Jan 13, 2025

calebpollman Jan 13, 2025

calebpollman Jan 13, 2025

feature(multi-bucket): add multi-bucket support to storage components #5562

Are you sure you want to change the base?

feature(multi-bucket): add multi-bucket support to storage components #5562

Conversation

jordanvn commented Aug 10, 2024

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

changeset-bot bot commented Aug 10, 2024 • edited Loading

🦋 Changeset detected

dbanksdesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calebpollman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Aug 10, 2024 •

edited

Loading