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.

Actionable comments posted: 0

🧹 Nitpick comments (4)

package.json (1)

53-59: Dev dependency bumps: ensure toolchain compatibility

tsd, tshy, and tshy-after all 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 test and inspect the generated dist layout in CI.

src/index.ts (2)

217-355: New DeleteMulti, tagging, and multipart types look coherent

The new option/result interfaces:

• Reuse RequestOptions, UserMeta, ObjectCallback, and NormalSuccessResponse consistently.
• Mirror expected OSS semantics for delete-multi (quiet, DeletedObject fields), 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 progress callbacks now appear in multiple interfaces with very similar signatures. You could factor out a shared type 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 for deleteMulti on IObjectSimple as well

The new methods on IObjectSimple (append, deleteMulti, tagging, multipart, listUploads, uploadPartCopy) line up with their option/result interfaces and the SimpleClient test implementation.

One small ergonomic gap: IObjectSimple.deleteMulti only exposes the two specific overloads

• names: string[]
• names: Array<{ key: string; versionId?: string }>

while the concrete SimpleClient implementation also has a union signature. That means a value typed as string[] | Array<{ key: string; versionId?: string }> cannot be passed through an IObjectSimple reference, 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 implementation

The SimpleClient class:

• Implements append, deleteMulti, tagging, multipart, listUploads, and uploadPartCopy with signatures matching IObjectSimple.
• Uses overloaded declarations for deleteMulti followed by a union implementation signature, which is a good pattern for accommodating both string[] and { key, versionId? }[] inputs from callers.
• Keeps all implementations trivial (console logging + any casting) as appropriate for tsd-style type tests.

One small nuance: delete is declared as taking RequestOptions | DeleteObjectOptions, whereas IObjectSimple.delete only mentions DeleteObjectOptions. That widening is safe but slightly asymmetrical; you could simplify it to DeleteObjectOptions for 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

📥 Commits

Reviewing files that changed from the base of the PR and between a141564 and 27c58d1.

📒 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 consistent

Disabling tshy.selfLink fits with the explicit tshy.exports mapping 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 under type: "module" without any .js extension 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 nested package.json or .cjs extension is needed here.

Likely an incorrect or invalid review comment.

src/index.ts (1)

124-127: Append types align well with existing put-object shapes

Introducing AppendObjectOptions extending PutObjectOptions and adding the optional data field to AppendObjectResult keeps append operations symmetric with normal put (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 API

The additional imports (AppendObject*, DeleteMulti*, tagging, multipart, listUploads, uploadPartCopy types, PartInfo, SourceData) line up with the interfaces exported from src/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 operations

The added tsd scenarios exercise:

• deleteMulti with both string-name arrays and versioned key objects, with and without quiet/timeout.
• Tagging operations (get/put/delete) with and without versionId and timeout, 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 position types, 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.ts and confirm that the method overloads behave as intended from a consumer’s point of view.

LGTM