-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add support for multipart uploads with lib-storage #678
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
feat: Add support for multipart uploads with lib-storage #678
Conversation
+ Fix handleRemove when there is no inputRef + Pass contentType to upload file
🦋 Changeset detectedLatest commit: 9a92b1f The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces AWS SDK lib-storage multipart uploads in the S3 adapter, adds an UploadFileOptions parameter (contentType), and changes getPublicUrl to return undefined. Adds a new getPublicUrl service and updates schema resolvers and generators/templates to use it. Updates dependencies and constants accordingly. Minor UI guard added in file-input components. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Upload Service
participant Adapter as S3 Adapter
participant S3 as AWS S3
Client->>API: uploadFile(contents, contentType?)
API->>Adapter: uploadFile(path, contents, { contentType })
Adapter->>S3: Upload.init(Bucket, Key, Body, ContentType)
Note over Adapter,S3: lib-storage decides single vs multipart
Adapter->>S3: upload.done()
S3-->>Adapter: Completed (ETag, parts)
Adapter->>S3: HeadObject (metadata)
Adapter-->>API: FileMetadata
API-->>Client: FileMetadata
sequenceDiagram
autonumber
participant GQL as publicUrl resolver
participant Service as getPublicUrl
participant DB as Prisma
participant Config as STORAGE_ADAPTERS
participant Adapter as Storage Adapter
GQL->>Service: getPublicUrl(fileOrId)
alt input is ID
Service->>DB: findUniqueOrThrow(id)
DB-->>Service: File
end
Service->>Config: resolve adapter by name
Service->>Adapter: getPublicUrl(storagePath)
Adapter-->>Service: URL or undefined
Service-->>GQL: URL or undefined
GQL-->>Caller: URL or null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (6)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used📓 Path-based instructions (6)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Files:
**/*📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Files:
examples/todo-with-auth0/packages/{admin,backend}/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (examples/todo-with-auth0/CLAUDE.md)
Files:
examples/todo-with-auth0/{packages/admin/src/{app,components,pages,hooks,services}/**/*.{ts,tsx},packages/backend/src/{modules,plugins,prisma,services,utils}/**/*.{ts,tsx}}📄 CodeRabbit inference engine (examples/todo-with-auth0/CLAUDE.md)
Files:
🧬 Code graph analysis (2)examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.ts (2)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsx (1)
129-129: Missing guard for consistency and safety.Line 129 in
handleCanceldirectly accessesinputRef.current.valuewithout any guard, whilehandleRemove(lines 122-124) attempts to add protection. This inconsistency could lead to runtime errors.Apply this diff to add the same guard:
const handleCancel = (): void => { cancelUpload(); - inputRef.current.value = ''; + if (inputRef.current instanceof HTMLInputElement) { + inputRef.current.value = ''; + } };examples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsx (1)
127-130: Add the same guard for consistency and safety.Line 129 accesses
inputRef.current.valuewithout checking ifinputRef.currentexists, whilehandleRemoveincludes this guard. This inconsistency creates a potential null pointer exception risk ifinputRef.currentisundefinedwhen the cancel button is clicked.Apply this diff to add the guard:
const handleCancel = (): void => { cancelUpload(); - inputRef.current.value = ''; + if (inputRef.current) { + inputRef.current.value = ''; + } };
🧹 Nitpick comments (2)
plugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsx (1)
1-1: Consider enabling TypeScript checking for better type safety.The
@ts-nocheckdirective disables all TypeScript checking for this template file. While this might be intentional for generator templates, enabling type checking would catch issues like the ineffective type assertion at lines 122-124 during development.If this is a generator template that needs to work across different type environments, consider using more targeted
@ts-expect-erroror@ts-ignorecomments for specific lines rather than disabling all checks.examples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsx (1)
20-36: Add JSDoc comments to exported declarations.The exported interfaces (
FileUploadInput,FileInputProps) and function (FileInput) are missing JSDoc comments. As per coding guidelines for files inexamples/todo-with-auth0/packages/admin/src/components/, all exported functions, interfaces, and classes should include JSDoc with descriptions, params, returns, and fields.Example:
/** * Represents the uploaded file data. */ export interface FileUploadInput { /** Unique identifier for the uploaded file */ id: string; /** Original filename */ filename: string; /** Public URL to access the file, if available */ publicUrl?: string | null; } /** * Props for the FileInput component. */ export interface FileInputProps { /** Additional CSS classes */ className?: string; /** Whether the input is disabled */ disabled?: boolean; // ... continue for other fields } /** * A file input component with drag-and-drop support and upload progress. * * @param props - The component props * @returns A file input element */ export function FileInput({ // ... }: FileInputProps): ReactElement { // ... }Based on coding guidelines.
Also applies to: 49-59
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (12)
examples/todo-with-auth0/packages/admin/src/generated/graphql.tsxis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/package.jsonis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/adapters/s3.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/schema/public-url.field.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/services/get-public-url.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/services/upload-file.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/types/adapter.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/web/src/generated/graphql.tsxis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlplugins/plugin-storage/src/generators/fastify/storage-module/generated/template-paths.tsis excluded by!**/generated/**,!**/generated/**plugins/plugin-storage/src/generators/fastify/storage-module/generated/template-renderers.tsis excluded by!**/generated/**,!**/generated/**plugins/plugin-storage/src/generators/fastify/storage-module/generated/typed-templates.tsis excluded by!**/generated/**,!**/generated/**
📒 Files selected for processing (19)
.changeset/multipart-upload-support.md(1 hunks)examples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsx(1 hunks)examples/todo-with-auth0/packages/backend/baseplate/file-id-map.json(1 hunks)examples/todo-with-auth0/packages/backend/package.json(1 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts(4 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.ts(2 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/services/.templates-info.json(1 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.ts(1 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.ts(1 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts(3 hunks)plugins/plugin-storage/src/constants/storage-packages.ts(1 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/extractor.json(2 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/storage-module.generator.ts(2 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts(4 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/schema/public-url.field.ts(1 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.ts(1 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/upload-file.ts(1 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts(3 hunks)plugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
**/*.{ts,tsx}: TypeScript with strict type checking
Always include return types on top-level functions including React components (React.ReactElement)
Include absolute paths in import statements via tsconfig paths (@src/is the alias forsrc/)
If a particular interface or type is not exported, change the file so it is exportedIf a particular interface or type is not exported, update the TypeScript file so it is exported
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.tsplugins/plugin-storage/src/constants/storage-packages.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.tsplugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsxplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/schema/public-url.field.tsplugins/plugin-storage/src/generators/fastify/storage-module/storage-module.generator.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/upload-file.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.tsexamples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsxexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Node 16 module resolution - include file extensions in imports (
.js)
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.tsplugins/plugin-storage/src/constants/storage-packages.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.tsplugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsxplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/schema/public-url.field.tsplugins/plugin-storage/src/generators/fastify/storage-module/storage-module.generator.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/upload-file.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.tsexamples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsxexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
**/*.{ts,tsx,js}: Sort imports by group: external libs first, then local imports
Use camelCase for variables/functions, PascalCase for types/classes
Order functions such that functions are placed below the variables/functions they use
We use the prefer using nullish coalescing operator (??) ESLint rule instead of a logical or (||), as it is a safer operator
Use console.info/warn/error instead of console.log
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.tsplugins/plugin-storage/src/constants/storage-packages.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.tsplugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsxplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/schema/public-url.field.tsplugins/plugin-storage/src/generators/fastify/storage-module/storage-module.generator.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/upload-file.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.tsexamples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsxexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Use kebab-case for file names
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.tsplugins/plugin-storage/src/constants/storage-packages.tsexamples/todo-with-auth0/packages/backend/baseplate/file-id-map.jsonplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.tsplugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsxplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/schema/public-url.field.tsexamples/todo-with-auth0/packages/backend/package.jsonplugins/plugin-storage/src/generators/fastify/storage-module/storage-module.generator.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/upload-file.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.tsexamples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsxplugins/plugin-storage/src/generators/fastify/storage-module/extractor.jsonexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts
examples/todo-with-auth0/packages/{admin,backend}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (examples/todo-with-auth0/CLAUDE.md)
examples/todo-with-auth0/packages/{admin,backend}/src/**/*.{ts,tsx}: Always use .js extensions in import statements, even when importing from TypeScript files
Useimport typefor type-only imports in TypeScript
Always specify explicit return types for all functions
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.tsexamples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsxexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
examples/todo-with-auth0/{packages/admin/src/{app,components,pages,hooks,services}/**/*.{ts,tsx},packages/backend/src/{modules,plugins,prisma,services,utils}/**/*.{ts,tsx}}
📄 CodeRabbit inference engine (examples/todo-with-auth0/CLAUDE.md)
Add JSDoc to all exported functions, interfaces, and classes including descriptions, params, returns, and fields (exclude generated code)
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.tsexamples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsxexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
plugins/plugin-*/**/*.tsx
📄 CodeRabbit inference engine (plugins/CLAUDE.md)
plugins/plugin-*/**/*.tsx: All CSS classes used inclassNameattributes within plugin components MUST be prefixed with the plugin name to avoid style conflicts.
When using utility functions likecn(), all CSS classes passed must also be prefixed with the plugin name.
Files:
plugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsx
.changeset/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/**/*.md: When adding a new feature or changing an existing feature, add a new Changeset in the .changeset/ directory
Changeset files must follow the specified frontmatter format with a package entry set to patch and a description body
Files:
.changeset/multipart-upload-support.md
examples/todo-with-auth0/{package.json,packages/**/package.json}
📄 CodeRabbit inference engine (examples/todo-with-auth0/CLAUDE.md)
examples/todo-with-auth0/{package.json,packages/**/package.json}: Module system must be ESM only ("type": "module"in package.json)
Package manager is pnpm 10+ (enforce viapackageManagerfield)
Node version 22+ with Volta pinned to 22.18.0
Files:
examples/todo-with-auth0/packages/backend/package.json
🧬 Code graph analysis (8)
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.ts (3)
examples/todo-with-auth0/packages/web/src/generated/graphql.tsx (1)
File(157-168)examples/todo-with-auth0/packages/admin/src/generated/graphql.tsx (1)
File(157-168)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/config/adapters.config.ts (1)
STORAGE_ADAPTERS(3-3)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts (2)
examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (3)
UploadProgress(48-55)UploadFileOptions(60-69)FileMetadata(6-15)examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/types/adapter.ts (3)
UploadProgress(48-55)UploadFileOptions(60-69)FileMetadata(6-15)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.ts (1)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/config/adapters.config.ts (1)
STORAGE_ADAPTERS(3-3)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/schema/public-url.field.ts (2)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.ts (1)
getPublicUrl(13-36)examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/services/get-public-url.ts (1)
getPublicUrl(13-37)
examples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.ts (1)
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.ts (1)
getPublicUrl(13-37)
examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (2)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts (3)
UploadProgress(50-57)UploadFileOptions(62-71)FileMetadata(8-17)examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/types/adapter.ts (3)
UploadProgress(48-55)UploadFileOptions(60-69)FileMetadata(6-15)
examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts (2)
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.ts (1)
getPublicUrl(13-37)examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (2)
UploadFileOptions(60-69)FileMetadata(6-15)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts (5)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.ts (1)
getPublicUrl(13-36)examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/services/get-public-url.ts (1)
getPublicUrl(13-37)examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (2)
UploadFileOptions(60-69)FileMetadata(6-15)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts (2)
UploadFileOptions(62-71)FileMetadata(8-17)examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/types/adapter.ts (2)
UploadFileOptions(60-69)FileMetadata(6-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Lint Examples (todo-with-auth0)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Test E2E
- GitHub Check: Lint
🔇 Additional comments (24)
examples/todo-with-auth0/packages/backend/package.json (1)
35-35: LGTM! Dependency addition is consistent.The
@aws-sdk/lib-storageversion aligns with other AWS SDK packages in the project (all on 3.370.0), enabling multipart upload support as intended.plugins/plugin-storage/src/constants/storage-packages.ts (1)
3-3: LGTM! Package constant addition is consistent.The version matches the backend package.json and other AWS SDK packages in this constants file.
plugins/plugin-storage/src/generators/fastify/storage-module/storage-module.generator.ts (2)
86-86: LGTM! Package dependency addition is correct.The
@aws-sdk/lib-storagepackage is properly added to the production dependencies with a version consistent across the codebase.
246-253: LGTM! Template rendering integration is proper.The servicesGetPublicUrl template rendering is correctly positioned in the build flow (after main module render, before adapters config) and follows the established pattern for template rendering.
examples/todo-with-auth0/packages/backend/src/modules/storage/services/.templates-info.json (1)
17-21: LGTM! Metadata entry is properly structured.The template info entry for
get-public-url.tsfollows the established pattern and correctly references the storage-module generator and services-get-public-url template.examples/todo-with-auth0/packages/backend/src/modules/storage/services/upload-file.ts (1)
32-34: LGTM! Options propagation enables ContentType support.The change correctly passes the
contentTypeoption to the adapter'suploadFilemethod, enabling proper content type handling for S3 uploads. Based on the PR context, the adapter signature has been updated to accept this third parameter.plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/upload-file.ts (1)
27-29: LGTM! Template change mirrors generated implementation.The template correctly passes
contentTypefrom options to the adapter, consistent with the generated service file and supporting the multipart upload feature.plugins/plugin-storage/src/generators/fastify/storage-module/extractor.json (2)
116-116: LGTM! Dependency reference updated correctly.The schema-public-url template now properly references the new services-get-public-url generator, centralizing public URL resolution through the service layer.
180-188: LGTM! Generator template entry is well-structured.The services-get-public-url entry is properly configured with correct type, path, dependencies, and variables, following the established pattern for service templates in this generator.
examples/todo-with-auth0/packages/backend/baseplate/file-id-map.json (1)
139-139: LGTM! File mapping entry is properly added.The mapping for the get-public-url service is correctly structured and consistently placed among other storage module service mappings.
examples/todo-with-auth0/packages/backend/src/modules/storage/schema/public-url.field.ts (1)
14-16: LGTM! Clean refactor to centralized service.The migration to
getPublicUrl(file)removes duplication and improves maintainability. The?? nullfallback correctly handles GraphQL's nullable string type semantics.plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/services/get-public-url.ts (1)
1-36: LGTM! Well-structured service with proper error handling.The service correctly validates adapter existence and capability before attempting URL retrieval. JSDoc is complete, explicit return types are present, and error messages are actionable.
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.ts (1)
1-37: LGTM! Backend implementation aligns with template.The service mirrors the template's validation logic and error handling. All coding guidelines are satisfied: explicit return types, JSDoc documentation, and
.jsimport extensions..changeset/multipart-upload-support.md (1)
1-10: LGTM! Changeset properly documents the feature.The changeset format is correct with appropriate patch-level versioning and a clear description of the multipart upload support additions.
examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (3)
45-69: LGTM! Well-designed interfaces for multipart upload support.The new
UploadProgressandUploadFileOptionsinterfaces provide comprehensive control over multipart uploads with clear documentation including defaults and constraints. All fields are appropriately optional.
92-96: LGTM! Signature extended cleanly with backward compatibility.The optional
optionsparameter maintains backward compatibility while enabling new multipart upload features. The updated example demonstrates proper usage.
196-196: LGTM! Return type semantics improved.Changing from
string | nulltostring | undefinedbetter represents "not available" semantics and aligns with TypeScript best practices for optional values.plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/schema/public-url.field.ts (1)
11-13: LGTM! Template mirrors backend implementation.The async resolver correctly delegates to the centralized
getPublicUrlservice and handles the nullable return type appropriately.plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts (1)
47-71: LGTM! Type definitions match backend implementation.The template interfaces mirror the backend types with complete JSDoc documentation, maintaining consistency across generated and non-generated code.
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts (2)
93-98: LGTM! Return type aligned with interface change.The change from
nulltoundefinedwhenpublicUrlis not configured improves semantic clarity and type consistency.
192-229: LGTM! Robust multipart upload implementation.The migration to
Uploadfrom@aws-sdk/lib-storageproperly:
- Configures multipart settings (partSize, queueSize) from options
- Tracks progress with safe arithmetic (
loaded && totalcheck,loaded ?? 0fallback)- Sets ContentType metadata for proper MIME type handling
- Enables automatic single-part vs multipart optimization
The implementation correctly follows the Upload API pattern and includes appropriate error propagation.
examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts (3)
10-10: LGTM! Imports properly support multipart uploads.The new imports for
Uploadfrom@aws-sdk/lib-storageandUploadFileOptionstype are correctly structured and necessary for the multipart upload functionality.Also applies to: 19-19
92-97: LGTM! Return type correctly aligns with service layer.The change from
string | nulltostring | undefinedaligns with the service layer's return type (Promise<string | undefined>) and follows TypeScript best practices by preferringundefinedovernullfor optional values.
191-228: LGTM! Multipart upload implementation is solid.The migration to
@aws-sdk/lib-storage'sUploadclass is well-implemented:
- Backwards compatible: The optional
UploadFileOptionsparameter preserves existing call sites- Progress tracking: Correctly wired via
httpUploadProgressevents with proper handling of undefined values- Security:
ServerSideEncryption: 'AES256'andContentTypemetadata enhance security posture- Error handling:
upload.done()ensures completion and propagates errors appropriately- Metadata consistency: Fetching metadata after upload provides accurate file information
The implementation correctly passes through
contentType,partSize, andqueueSizeoptions to theUploadinstance, enabling fine-grained control over multipart upload behavior.
| if (inputRef.current as HTMLInputElement | undefined) { | ||
| inputRef.current.value = ''; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify the guard - type assertion doesn't perform runtime type check.
The type assertion as HTMLInputElement | undefined is misleading. At runtime, this is just a truthy check (if (inputRef.current)). Type assertions don't perform runtime validation; they only affect TypeScript's compile-time type checking.
Apply this diff to simplify:
- if (inputRef.current as HTMLInputElement | undefined) {
+ if (inputRef.current) {
inputRef.current.value = '';
}If you truly need to verify the type at runtime, use instanceof:
- if (inputRef.current as HTMLInputElement | undefined) {
+ if (inputRef.current && inputRef.current instanceof HTMLInputElement) {
inputRef.current.value = '';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (inputRef.current as HTMLInputElement | undefined) { | |
| inputRef.current.value = ''; | |
| } | |
| if (inputRef.current) { | |
| inputRef.current.value = ''; | |
| } |
🤖 Prompt for AI Agents
examples/todo-with-auth0/packages/admin/src/components/ui/file-input.tsx lines
122-124: the current guard uses a TypeScript type assertion (as HTMLInputElement
| undefined) which does nothing at runtime; remove the assertion and use a
simple truthy check (if (inputRef.current) { inputRef.current.value = '' }) or,
if you must ensure it's an HTMLInputElement at runtime, use instanceof (if
(inputRef.current instanceof HTMLInputElement) { ... }); update the condition
accordingly and drop the misleading type assertion.
| if (inputRef.current as HTMLInputElement | undefined) { | ||
| inputRef.current.value = ''; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type assertion provides no runtime safety.
The type assertion as HTMLInputElement | undefined is a compile-time construct that doesn't perform any runtime check. This condition still just checks the truthiness of inputRef.current, leaving the code vulnerable to runtime errors if inputRef.current is null, undefined, or not an HTMLInputElement.
Apply this diff to add a proper runtime guard:
- if (inputRef.current as HTMLInputElement | undefined) {
- inputRef.current.value = '';
- }
+ if (inputRef.current && 'value' in inputRef.current) {
+ inputRef.current.value = '';
+ }Alternatively, use instanceof for stricter type checking:
- if (inputRef.current as HTMLInputElement | undefined) {
- inputRef.current.value = '';
- }
+ if (inputRef.current instanceof HTMLInputElement) {
+ inputRef.current.value = '';
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (inputRef.current as HTMLInputElement | undefined) { | |
| inputRef.current.value = ''; | |
| } | |
| if (inputRef.current instanceof HTMLInputElement) { | |
| inputRef.current.value = ''; | |
| } |
🤖 Prompt for AI Agents
In
plugins/plugin-storage/src/generators/react/upload-components/templates/components/ui/file-input.tsx
around lines 122 to 124, the condition uses a TypeScript-only assertion ("as
HTMLInputElement | undefined") which provides no runtime safety; replace that
with a real runtime guard by checking that inputRef.current is truthy and is an
instance of HTMLInputElement (e.g., if (inputRef.current && inputRef.current
instanceof HTMLInputElement) { inputRef.current.value = '' }) so the code only
accesses .value when the ref is actually an HTMLInputElement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts (1)
196-208: Consider exposing advanced Upload configuration options.The implementation correctly uses
Uploadfrom@aws-sdk/lib-storage, which automatically optimizes between single-part and multipart uploads based on file size. However, theUploadclass supports additional configuration that might be beneficial:
- Part size and concurrency: Configurable via
partSizeandqueueSizeparameters for tuning performance- Progress tracking: The Upload instance emits progress events that could be useful for user feedback
- Abort capability:
upload.abort()allows canceling in-flight uploadsThe current implementation uses sensible defaults, which is appropriate for most use cases.
If advanced use cases emerge later, consider extending
UploadFileOptionsto expose these parameters:export interface UploadFileOptions { /** MIME type of the file (e.g., 'image/jpeg', 'application/pdf') */ contentType?: string; + /** Part size for multipart uploads in bytes (default: 5MB) */ + partSize?: number; + /** Number of concurrent upload operations (default: 4) */ + queueSize?: number; + /** Callback for upload progress tracking */ + onProgress?: (progress: { loaded: number; total?: number }) => void; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
examples/todo-with-auth0/packages/admin/baseplate/generated/src/components/ui/file-input.tsxis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/adapters/s3.tsis excluded by!**/generated/**,!**/generated/**examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/types/adapter.tsis excluded by!**/generated/**,!**/generated/**
📒 Files selected for processing (5)
.changeset/multipart-upload-support.md(1 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts(4 hunks)examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts(3 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts(4 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts
🧰 Additional context used
📓 Path-based instructions (7)
.changeset/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/**/*.md: When adding a new feature or changing an existing feature, add a new Changeset in the .changeset/ directory
Changeset files must follow the specified frontmatter format with a package entry set to patch and a description body
Files:
.changeset/multipart-upload-support.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
**/*.{ts,tsx}: TypeScript with strict type checking
Always include return types on top-level functions including React components (React.ReactElement)
Include absolute paths in import statements via tsconfig paths (@src/is the alias forsrc/)
If a particular interface or type is not exported, change the file so it is exportedIf a particular interface or type is not exported, update the TypeScript file so it is exported
Files:
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Node 16 module resolution - include file extensions in imports (
.js)
Files:
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
**/*.{ts,tsx,js}: Sort imports by group: external libs first, then local imports
Use camelCase for variables/functions, PascalCase for types/classes
Order functions such that functions are placed below the variables/functions they use
We use the prefer using nullish coalescing operator (??) ESLint rule instead of a logical or (||), as it is a safer operator
Use console.info/warn/error instead of console.log
Files:
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/code-style.mdc)
Use kebab-case for file names
Files:
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.tsplugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.tsexamples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
examples/todo-with-auth0/packages/{admin,backend}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (examples/todo-with-auth0/CLAUDE.md)
examples/todo-with-auth0/packages/{admin,backend}/src/**/*.{ts,tsx}: Always use .js extensions in import statements, even when importing from TypeScript files
Useimport typefor type-only imports in TypeScript
Always specify explicit return types for all functions
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
examples/todo-with-auth0/{packages/admin/src/{app,components,pages,hooks,services}/**/*.{ts,tsx},packages/backend/src/{modules,plugins,prisma,services,utils}/**/*.{ts,tsx}}
📄 CodeRabbit inference engine (examples/todo-with-auth0/CLAUDE.md)
Add JSDoc to all exported functions, interfaces, and classes including descriptions, params, returns, and fields (exclude generated code)
Files:
examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts
🧬 Code graph analysis (3)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts (2)
examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (2)
UploadFileOptions(48-51)FileMetadata(6-15)examples/todo-with-auth0/packages/backend/baseplate/generated/src/modules/storage/types/adapter.ts (2)
UploadFileOptions(48-51)FileMetadata(6-15)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts (2)
examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (2)
UploadFileOptions(48-51)FileMetadata(6-15)plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts (2)
UploadFileOptions(50-53)FileMetadata(8-17)
examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts (2)
examples/todo-with-auth0/packages/backend/src/modules/storage/services/get-public-url.ts (1)
getPublicUrl(13-37)examples/todo-with-auth0/packages/backend/src/modules/storage/types/adapter.ts (2)
UploadFileOptions(48-51)FileMetadata(6-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test E2E
- GitHub Check: Lint
🔇 Additional comments (9)
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/types/adapter.ts (3)
47-53: LGTM!The
UploadFileOptionsinterface is well-defined with clear JSDoc documentation. The optionalcontentTypefield appropriately allows callers to specify MIME types when needed.
76-80: LGTM!The
uploadFilesignature correctly extends the public API with an optionaloptionsparameter. The parameter is properly typed and maintains backward compatibility since it's optional.
174-180: Consider clarifying the semantic difference between null and undefined.The return type change from
string | nulltostring | undefinedis semantically meaningful:undefinedbetter represents "not available" or "not configured" in JavaScript/TypeScript. However, this could impact consumers who explicitly check fornull.The change is consistent with the optional method pattern (notice
getPublicUrl?), where absence of implementation also results inundefined.This concern was already raised in the changeset review.
plugins/plugin-storage/src/generators/fastify/storage-module/templates/module/adapters/s3.ts (3)
8-19: LGTM!The imports correctly add
UploadFileOptionsto the type imports andUploadfrom@aws-sdk/lib-storageto support multipart uploads.
93-98: LGTM!The
getPublicUrlimplementation correctly returnsundefinedwhen no public URL is configured, aligning with the updated type signature.
203-203: No action required for undefined ContentType in S3 upload.
AWS S3 defaults an unspecified Content-Type to application/octet-stream (some SDKs may guess from the filename), so leavingoptions?.contentTypeundefined is acceptable.examples/todo-with-auth0/packages/backend/src/modules/storage/adapters/s3.ts (3)
10-20: LGTM!The imports are correctly organized with external libraries first (AWS SDK), followed by local type imports. The relative import on line 20 properly includes the
.jsextension as required by Node 16 module resolution and the coding guidelines.
92-97: LGTM!The
getPublicUrlimplementation is consistent with the template and correctly returnsundefinedwhen no public URL is configured.
196-208: LGTM!The
uploadFileimplementation correctly uses theUploadclass from@aws-sdk/lib-storagefor automatic multipart upload optimization. TheContentTypeis properly threaded from the options parameter, andupload.done()is called to finalize the upload.The same optional improvement mentioned in the template review applies here: advanced Upload configuration could be exposed if future use cases require it.
| --- | ||
| '@baseplate-dev/plugin-storage': patch | ||
| --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Verification inconclusive
Verify patch version is appropriate for the return type change.
The changeset designates this as a patch release. However, changing getPublicUrl return type from string | null to string | undefined (as seen in the type definitions) could be a breaking change for consumers who explicitly check for null (e.g., if (url === null)). While functionally similar, this type change might warrant a minor version bump.
Consider whether code exists that explicitly checks for null:
🏁 Script executed:
#!/bin/bash
# Description: Search for explicit null checks on getPublicUrl results
# Search for patterns that explicitly check for null
rg -nP --type=ts --type=tsx 'getPublicUrl\([^)]+\)\s*===\s*null' -A 2 -B 2
rg -nP --type=ts --type=tsx 'null\s*===\s*getPublicUrl' -A 2 -B 2Length of output: 55
🏁 Script executed:
#!/bin/bash
# Search for explicit null checks on getPublicUrl results
rg -nP 'getPublicUrl\([^)]*\)\s*===\s*null' -g '*.ts' -g '*.tsx' -C2
rg -nP 'null\s*===\s*getPublicUrl\([^)]*\)' -g '*.ts' -g '*.tsx' -C2Length of output: 0
Bump release to minor
Change the changeset’s release type from patch to minor—altering getPublicUrl’s return from string | null to string | undefined can break consumers checking specifically for null.
🤖 Prompt for AI Agents
In .changeset/multipart-upload-support.md around lines 1 to 3, the changeset
currently sets the release type to 'patch' but the change is breaking
(getPublicUrl's return type changes from string | null to string | undefined),
so update the changeset release type from 'patch' to 'minor' to reflect the
API-affecting change; edit the file to replace 'patch' with 'minor' so the
package is released as a minor version.
Summary by CodeRabbit
New Features
Bug Fixes
Chores