feat: add type definitions for additional OSS object operations #11
Conversation
Add comprehensive type definitions and tests for multiple OSS operations: - Batch operations: deleteMulti for deleting multiple objects - Object tagging: getObjectTagging, putObjectTagging, deleteObjectTagging - Multipart upload: initMultipartUpload, completeMultipartUpload, multipartUpload, multipartUploadCopy, abortMultipartUpload, listUploads, uploadPartCopy - Append operations: append for appendable objects All methods include complete TypeScript type definitions with options and result interfaces, along with comprehensive type tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe pull request expands the SimpleClient API surface with new methods for object append, bulk deletion, object tagging, multipart uploads, and part copy operations. Corresponding TypeScript types and test coverage are added. Build configuration is updated with entry points and tooling dependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Areas requiring attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @killagu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the type safety and developer experience for interacting with Alibaba Cloud OSS by introducing a broad set of new TypeScript type definitions. It covers advanced object operations such as batch deletions, object tagging, and a full suite of multipart upload functionalities, along with append operations. These additions are complemented by thorough type tests, ensuring the robustness and accuracy of the new definitions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant number of new type definitions for OSS operations, along with corresponding type tests. The changes are well-organized and the new types are comprehensive. My review focuses on improving consistency and clarity in the newly added interfaces. I've identified a few areas where the API can be made more consistent with existing patterns in the library and where some redundancy can be removed.
| export interface DeletedObject { | ||
| Key: string; | ||
| VersionId?: string; | ||
| DeleteMarker?: boolean; | ||
| DeleteMarkerVersionId?: string; | ||
| } |
There was a problem hiding this comment.
The properties in the DeletedObject interface use PascalCase (Key, VersionId, etc.). This is inconsistent with other data-transfer interfaces in this file, such as ObjectMeta and the newly added Upload, which use camelCase. For a consistent API surface, I recommend using camelCase for these properties as well. The library can handle the mapping from the PascalCase XML response internally.
| export interface DeletedObject { | |
| Key: string; | |
| VersionId?: string; | |
| DeleteMarker?: boolean; | |
| DeleteMarkerVersionId?: string; | |
| } | |
| export interface DeletedObject { | |
| key: string; | |
| versionId?: string; | |
| deleteMarker?: boolean; | |
| deleteMarkerVersionId?: string; | |
| } |
| export interface MultipartUploadResult { | ||
| res: NormalSuccessResponse; | ||
| bucket: string; | ||
| name: string; | ||
| etag: string; | ||
| /** callback response data */ | ||
| data?: object; | ||
| } |
There was a problem hiding this comment.
The MultipartUploadResult interface is structurally identical to CompleteMultipartUploadResult. To avoid redundancy and improve maintainability, I suggest removing MultipartUploadResult and using CompleteMultipartUploadResult for the multipartUpload method as well. The multipartUpload is a high-level wrapper that internally completes the upload, so it makes sense for it to return the same result type. You would then also update the multipartUpload method signature in the IObjectSimple interface.
| export interface UploadPartCopyOptions extends RequestOptions { | ||
| /** version id of source object */ | ||
| versionId?: string; | ||
| headers?: IncomingHttpHeaders; | ||
| mime?: string; | ||
| } |
There was a problem hiding this comment.
The mime property in UploadPartCopyOptions seems out of place. The uploadPartCopy operation copies a part of an existing object, and the MIME type of an object is typically determined during initMultipartUpload. It's not usually specified on a per-part basis during a copy operation. If this is not supported by the OSS API for this specific operation, I recommend removing it to avoid confusion.
export interface UploadPartCopyOptions extends RequestOptions {
/** version id of source object */
versionId?: string;
headers?: IncomingHttpHeaders;
}| /** | ||
| * Upload a file to OSS using multipart uploads. | ||
| */ | ||
| multipartUpload(name: string, file: string | Buffer | Readable, options?: MultipartUploadOptions): Promise<MultipartUploadResult>; |
There was a problem hiding this comment.
To address the redundancy between MultipartUploadResult and CompleteMultipartUploadResult, I suggest changing the return type of multipartUpload to Promise<CompleteMultipartUploadResult>. This would allow you to remove the MultipartUploadResult interface entirely.
multipartUpload(name: string, file: string | Buffer | Readable, options?: MultipartUploadOptions): Promise<CompleteMultipartUploadResult>;
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
package.json (1)
53-59: Dev dependency bumps: ensure toolchain compatibility
tsd,tshy, andtshy-afterall moved to newer major/minor versions. That’s fine, but these tools can change CLI behavior or defaults, so please confirm the generated d.ts output and type tests still behave as expected across Node/TS versions you care about.If you’d like, I can sketch a small script to run
npm testand inspect the generateddistlayout in CI.src/index.ts (2)
217-355: New DeleteMulti, tagging, and multipart types look coherentThe new option/result interfaces:
- Reuse
RequestOptions,UserMeta,ObjectCallback, andNormalSuccessResponseconsistently.- Mirror expected OSS semantics for delete-multi (
quiet,DeletedObjectfields), object tagging (status + tag map), and multipart lifecycle (init, complete, resumable upload/copy, abort, and list uploads).- Use specific structs (
SourceData,Upload,UploadPartCopySourceData) to keep response shapes explicit.From a type-safety perspective this all looks solid and matches how the tests consume these types.
One optional improvement: the various
progresscallbacks now appear in multiple interfaces with very similar signatures. You could factor out a sharedtype ProgressCallback = (...) => void | Promise<void>(or a small family of them) to reduce duplication and keep future changes localized.Also applies to: 369-411
472-476: Consider exposing the union overload fordeleteMultionIObjectSimpleas wellThe new methods on
IObjectSimple(append, deleteMulti, tagging, multipart, listUploads, uploadPartCopy) line up with their option/result interfaces and theSimpleClienttest implementation.One small ergonomic gap:
IObjectSimple.deleteMultionly exposes the two specific overloads
names: string[]names: Array<{ key: string; versionId?: string }>while the concrete
SimpleClientimplementation also has a union signature. That means a value typed asstring[] | Array<{ key: string; versionId?: string }>cannot be passed through anIObjectSimplereference, even though the implementation can handle it.You can make the interface friendlier by adding a third, more general overload mirroring the implementation and placing it last:
export interface IObjectSimple { … /** * Delete multiple objects from the bucket. */ deleteMulti(names: string[], options?: DeleteMultiOptions): Promise<DeleteMultiResult>; deleteMulti(names: Array<{ key: string; versionId?: string }>, options?: DeleteMultiOptions): Promise<DeleteMultiResult>; + deleteMulti( + names: string[] | Array<{ key: string; versionId?: string }>, + options?: DeleteMultiOptions, + ): Promise<DeleteMultiResult>; … }Also applies to: 498-503, 510-558
index.test-d.ts (1)
60-63: SimpleClient implementation correctly tracks IObjectSimple, with a nice deleteMulti union implementationThe
SimpleClientclass:
- Implements
append, deleteMulti, tagging, multipart, listUploads, and uploadPartCopy with signatures matchingIObjectSimple.- Uses overloaded declarations for
deleteMultifollowed by a union implementation signature, which is a good pattern for accommodating bothstring[]and{ key, versionId? }[]inputs from callers.- Keeps all implementations trivial (console logging +
anycasting) as appropriate for tsd-style type tests.One small nuance:
deleteis declared as takingRequestOptions | DeleteObjectOptions, whereasIObjectSimple.deleteonly mentionsDeleteObjectOptions. That widening is safe but slightly asymmetrical; you could simplify it toDeleteObjectOptionsfor direct alignment if you want to avoid surprises when reading the test implementation.Also applies to: 93-98, 107-155
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
index.test-d.ts(5 hunks)package.json(2 hunks)src/index.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
index.test-d.ts (1)
src/index.ts (25)
AppendObjectOptions(124-127)AppendObjectResult(129-138)DeleteMultiOptions(217-220)DeleteMultiResult(229-233)ObjectTaggingOptions(235-237)GetObjectTaggingResult(239-245)PutObjectTaggingResult(247-251)DeleteObjectTaggingResult(253-257)InitMultipartUploadOptions(259-265)InitMultipartUploadResult(267-272)PartInfo(274-279)CompleteMultipartUploadOptions(281-286)CompleteMultipartUploadResult(288-295)MultipartUploadOptions(297-312)MultipartUploadResult(314-321)SourceData(323-332)MultipartUploadCopyOptions(334-348)AbortMultipartUploadOptions(350-350)AbortMultipartUploadResult(352-354)ListUploadsQuery(356-367)RequestOptions(12-15)ListUploadsResult(378-390)UploadPartCopySourceData(392-397)UploadPartCopyOptions(399-404)UploadPartCopyResult(406-411)
🔇 Additional comments (5)
package.json (2)
11-16: tshy selfLink config looks consistentDisabling
tshy.selfLinkfits with the explicittshy.exportsmapping you already have; nothing stands out as problematic here.
63-65: Remove legacy"types","main", and"module"fields—they're redundant and ignored.Your package.json already defines proper conditional exports with
"require"and"import"conditions. When the"exports"field is present, Node.js uses it exclusively and ignores the legacy fallback fields ("types","main","module"). This modern configuration correctly routes CJS and ESM undertype: "module"without any.jsextension issue.The lines you flagged (63–65) can be safely removed. The actual module resolution is already correct via your
"exports"field, which separates type definitions and entry points for each module type. No nestedpackage.jsonor.cjsextension is needed here.Likely an incorrect or invalid review comment.
src/index.ts (1)
124-127: Append types align well with existing put-object shapesIntroducing
AppendObjectOptionsextendingPutObjectOptionsand adding the optionaldatafield toAppendObjectResultkeeps append operations symmetric with normalput(meta, callback, headers, and callback response data), without changing existing fields.Also applies to: 136-137
index.test-d.ts (2)
11-45: Extended imports are consistent with the new public APIThe additional imports (
AppendObject*,DeleteMulti*, tagging, multipart, listUploads, uploadPartCopy types,PartInfo,SourceData) line up with the interfaces exported fromsrc/index.ts. This keeps the test surface in sync with the public API surface.
203-478: New tsd assertions give broad coverage of the added operationsThe added tsd scenarios exercise:
deleteMultiwith both string-name arrays and versioned key objects, with and withoutquiet/timeout.- Tagging operations (get/put/delete) with and without
versionIdandtimeout, checking both status and tag map types.- Multipart lifecycle: init (with mime/meta/options), complete (with/without callback), upload (string path, Buffer, Readable + options), multipart copy (with
SourceData, offsets, and options).- Abort and listUploads flows, including pagination markers and
encoding-type.- Append flows across string path, Buffer, Readable, different
positiontypes, and callback responses.- Upload-part-copy flows, including empty-range usage and full options including headers and versioning.
These checks collectively validate the new option/result interfaces in
src/index.tsand confirm that the method overloads behave as intended from a consumer’s point of view.
|
我修复一下自动发布的问题 |
|
🎉 This PR is included in version 1.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Add comprehensive type definitions and tests for multiple OSS operations:
All methods include complete TypeScript type definitions with options and result interfaces, along with comprehensive type tests.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores