feat: support cname #19
Conversation
WalkthroughThis set of changes includes version upgrades for GitHub Actions workflows, updates to the Node.js version requirement, alterations to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OSSObject
participant Server
User->>OSSObject: Initialize OSSObject with cname option
OSSObject->>OSSObject: Check if cname is true
alt cname is true and bucket is provided
OSSObject->>OSSObject: Set this.#bucket and this.#bucketEndpoint
else no bucket is provided
OSSObject->>OSSObject: Handle error for missing bucket
end
User->>OSSObject: Use OSSObject for operations
OSSObject->>Server: Request with custom domain endpoint
Server-->>OSSObject: Response
OSSObject-->>User: Return response
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (2)
src/OSSObject.ts (2)
Line range hint
375-376: Avoid using the delete operator.The use of the
deleteoperator can lead to performance degradation. Consider usingundefinedassignment instead to clear properties.- delete options.headers['Content-Type']; + options.headers['content-type'] = undefined;
Line range hint
915-917: Optimize header manipulation to avoid performance issues.Similar to the previous comment, replacing the
deleteoperator withundefinedassignment can help maintain performance, especially in critical paths.- delete options.headers['Content-Type']; + options.headers['content-type'] = undefined;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- .github/workflows/nodejs.yml (2 hunks)
- README.md (3 hunks)
- package.json (5 hunks)
- src/OSSObject.ts (2 hunks)
- test/OSSObject.test.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/nodejs.yml
- test/OSSObject.test.ts
Additional context used
Biome
src/OSSObject.ts
[error] 375-376: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 915-917: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
LanguageTool
README.md
[uncategorized] ~26-~26: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Context: ... OSS, Object Storage Service. Equal to well known Amazon S3....
[grammar] ~146-~146: An article may be missing. (BUNCH_OF)
Context: ... aliyun too, you can settrueto save lot of money. - [timeout] {String|Number} inst...
[grammar] ~166-~166: There seems to be a noun/verb agreement error. Did you mean “accelerates” or “accelerated”? (SINGULAR_NOUN_VERB_AGREEMENT)
Context: ...ion: 'oss-cn-hangzhou', }); ``` 2. use accelerate endpoint - Global accelerate endpoint:...
[uncategorized] ~401-~401: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...nd an object to the bucket, it's almost same as put, but it can add content to exist...
[uncategorized] ~401-~401: Possible missing article found. (AI_HYDRA_LEO_MISSING_AN)
Context: ... same as put, but it can add content to existing object rather than override it. All pa...
[uncategorized] ~444-~444: Possible missing preposition found. (AI_HYDRA_LEO_MISSING_OF)
Context: ...ebaseUrl, will usebaseUrlinstead the defaultbucket and endpoint. Suggest ...
[grammar] ~543-~543: Possible subject-verb agreement error. (IS_AND_ARE)
Context: ... example: - Head an exists object and get object meta info ```js await this.stor...
[grammar] ~709-~709: In standard English, the word ‘exists’ is a verb. Please verify that the determiner ‘an’ is correct, or that a noun is not missing between ‘an’ and ‘exists’. (A_INFINITIVE)
Context: ...lso delete success. example: - Delete an exists object ```js await store.delete('ossde...
[grammar] ~736-~736: It seems that a pronoun is missing. (IF_VB)
Context: ... [sourceBucket] {String} source Bucket. if doesn't exist，sourceBucketis same bu...
[grammar] ~736-~736: Is an article like ‘a’ or ‘an’ missing? (BE_NN)
Context: ...Bucket. if doesn't exist，sourceBucketis same bucket. - [options] {Object} optio...
[grammar] ~798-~798: In standard English, the word ‘exists’ is a verb. Please verify that the determiner ‘an’ is correct, or that a noun is not missing between ‘an’ and ‘exists’. (A_INFINITIVE)
Context: ...## .putMeta(name, meta[, options]) Set an exists object meta. parameters: - name {Stri...
[uncategorized] ~845-~845: The preposition ‘at’ seems more likely in this position. (AI_HYDRA_LEO_REPLACE_IN_AT)
Context: ...Object>} object names, max 1000 objects in once. - key {String} object name - ...
[grammar] ~916-~916: The modal verb ‘will’ requires the verb’s base form. (MD_BASEFORM)
Context: ...meta info list EachObjectMetawill contains blow properties: - name {String} obje...
[grammar] ~981-~981: The modal verb ‘will’ requires the verb’s base form. (MD_BASEFORM)
Context: ...meta info list EachObjectMetawill contains blow properties: - name {String} obje...
[grammar] ~1059-~1059: The modal verb ‘will’ requires the verb’s base form. (MD_BASEFORM)
Context: ...meta info list EachObjectMetawill contains blow properties: - name {String} obje...
[grammar] ~1115-~1115: There is an agreement error between ‘put’ and ‘object’. Insert ‘a(n)’ or change the noun to plural. (PRP_VB_NN)
Context: ...for download or upload object. When you put object with signatureUrl ,you need to pass `Co...
[uncategorized] ~1138-~1138: The abbreviation “e.g.” (= for example) requires two periods. (E_G)
Context: ...ject} set the custom value for callback,eg. {var1: value1,var2:value2} Success wil...
[typographical] ~1195-~1195: Consider adding a comma after ‘Basically’ for more clarity. (RB_LY_COMMA)
Context: ...## .asyncSignatureUrl(name[, options]) Basically the same as signatureUrl, if refreshSTS...
[uncategorized] ~1219-~1219: The abbreviation “e.g.” (= for example) requires two periods. (E_G)
Context: ...ject} set the custom value for callback,eg. {var1: value1,var2:value2} Success wil...
[misspelling] ~1299-~1299: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’. (EN_A_VS_AN)
Context: ...ssdemo.txt', 'public-read');- Set an history object's ACLjs const versi...
[misspelling] ~1337-~1337: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’. (EN_A_VS_AN)
Context: ...'); console.log(result.acl);- Get an history object's ACLjs const versi...
[misspelling] ~1388-~1388: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’. (EN_A_VS_AN)
Context: ...sole.log(result.status);- Restore an history objectjs const versionId =...
[uncategorized] ~1517-~1517: The abbreviation “e.g.” (= for example) requires two periods. (E_G)
Context: ...ng} the object name - tag {Object} tag, eg.{var1: value1,var2:value2}- [options...
[uncategorized] ~1535-~1535: The abbreviation “e.g.” (= for example) requires two periods. (E_G)
Context: ...ng} the object name - tag {Object} tag, eg.{var1: value1,var2:value2}- [options...
[grammar] ~1574-~1574: The modal verb ‘will’ requires the verb’s base form. (MD_BASEFORM)
Context: ...s Each error return by OSS server will contains these properties: - name {String} erro...
Markdownlint
README.md
24-24: null (MD025, single-title, single-h1)
Multiple top-level headings in the same document
184-184: null (MD034, no-bare-urls)
Bare URL used
Additional comments not posted (9)
package.json (4)
4-6: Specify Node.js version requirement.Adding a Node.js version requirement ensures that the package will only be used with compatible versions of Node.js, which can help avoid runtime issues due to incompatible language features or APIs.
71-76: Update module exports for CommonJS and ESM compatibility.The updated export paths facilitate the use of this package with both CommonJS and ESM module systems, which enhances its usability and compatibility across different Node.js environments.
82-83: Set main and types entry points.Defining explicit 'main' and 'types' fields helps ensure that the package's primary entry points are clear, improving its integration with TypeScript and JavaScript projects.
13-13: Modify CI script to includeattw --pack.This change in the CI script to include
attw --packmight be intended to perform additional checks or packaging steps before publication. Ensure thatattwis properly configured and that its inclusion does not introduce unexpected behaviors during CI builds.src/OSSObject.ts (3)
67-72: Add optionalcnameproperty toOSSObjectClientInitOptions.This addition supports the new feature for handling custom domain names (CNAMEs), aligning with the PR's objective. The documentation link provided in the comment helps developers understand the context and usage of this property.
80-96: Refactor constructor logic to supportcname.The constructor now conditionally requires a
bucketbased on thecnamesetting, which allows flexibility in configuration. The logic to handle bucket naming and endpoint configuration based oncnameis clear and well-implemented.
902-905: Validate bucket names dynamically.The method
#getCopySourceNamedynamically checks and constructs the bucket name, which is crucial for ensuring the validity of bucket operations. This addition helps prevent errors related to incorrect bucket names.README.md (2)
178-178: Clarify the use of the global accelerate endpoint.The explanation about using the global accelerate endpoint could be enhanced by specifying when and why one might want to use this endpoint over others.
+ // Use this endpoint for faster global data transfer speeds.
1637-1639: Updated contributors section with dynamic content.The change from a static list to a dynamic image link for contributors is a modern approach that keeps the list updated automatically. Ensure that the image link is always accessible to avoid broken images on the README.
| @@ -159,7 +159,7 @@ const store = new OSSObject({ | |||
| accessKeyId: 'your access key', | |||
| accessKeySecret: 'your access secret', | |||
| bucket: 'your bucket name', | |||
| region: 'oss-cn-hangzhou' | |||
| region: 'oss-cn-hangzhou', | |||
There was a problem hiding this comment.
Specify the type of region in the documentation.
The region parameter is used in the initialization of OSSObject, but its type isn't specified in the documentation. This could lead to confusion about what values are expected (string, object, etc.).
- region: '<oss region>',
+ region: '<oss region>', // Type: stringCommittable 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.
| region: 'oss-cn-hangzhou', | |
| region: 'oss-cn-hangzhou', // Type: string |
| See https://help.aliyun.com/zh/oss/user-guide/map-custom-domain-names-5 | ||
|
|
||
| ```js | ||
| const { OSSObject } = require('oss-client'); | ||
|
|
||
| const store = new OSSObject({ | ||
| accessKeyId: 'your access key', | ||
| accessKeySecret: 'your access secret', | ||
| cname: true, | ||
| endpoint: 'your custome domain', | ||
| // your custom domain endpoint | ||
| endpoint: 'https://my-static.domain.com', |
There was a problem hiding this comment.
Ensure the cname property is adequately explained.
The cname property is used in the example for custom domains, but there's no explanation of what it does or how it affects the behavior of the OSSObject. This could confuse users unfamiliar with the concept of CNAME in DNS settings.
+ // Set cname to true to use a custom domain name for bucket access.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.
| See https://help.aliyun.com/zh/oss/user-guide/map-custom-domain-names-5 | |
| ```js | |
| const { OSSObject } = require('oss-client'); | |
| const store = new OSSObject({ | |
| accessKeyId: 'your access key', | |
| accessKeySecret: 'your access secret', | |
| cname: true, | |
| endpoint: 'your custome domain', | |
| // your custom domain endpoint | |
| endpoint: 'https://my-static.domain.com', | |
| See https://help.aliyun.com/zh/oss/user-guide/map-custom-domain-names-5 | |
| ```js | |
| const { OSSObject } = require('oss-client'); | |
| const store = new OSSObject({ | |
| accessKeyId: 'your access key', | |
| accessKeySecret: 'your access secret', | |
| cname: true, // Set cname to true to use a custom domain name for bucket access. | |
| // your custom domain endpoint | |
| endpoint: 'https://my-static.domain.com', |
Tools
Markdownlint
184-184: null (MD034, no-bare-urls)
Bare URL used
| @@ -40,6 +39,7 @@ | |||
| "xml2js": "^0.6.2" | |||
| }, | |||
| "devDependencies": { | |||
| "@arethetypeswrong/cli": "^0.15.3", | |||
There was a problem hiding this comment.
Tip
Codebase Verification
No usage of @arethetypeswrong/cli found in npm scripts.
The dependency @arethetypeswrong/cli is listed in package.json, but it is not referenced in the npm scripts. Please ensure that this tool is integrated into the project's workflow if it is intended to be used.
package.json: Dependency listed but not used in scripts.
Analysis chain
Add new dependency @arethetypeswrong/cli.
The addition of @arethetypeswrong/cli to devDependencies suggests improvements in type checking or code quality tools. Confirm that this tool integrates well with existing workflows and that it is used in the scripts where necessary.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `@arethetypeswrong/cli` in npm scripts.
# Test: Search for usage in npm scripts.
grep -R '@arethetypeswrong/cli' package.json
Length of output: 84
Script:
#!/bin/bash
# Description: Verify the usage of `@arethetypeswrong/cli` in npm scripts.
# Extract and check the scripts section in package.json for any usage of @arethetypeswrong/cli.
jq '.scripts' package.json | grep '@arethetypeswrong/cli'
Length of output: 55
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 94.32% 94.37% +0.05%
==========================================
Files 17 17
Lines 1867 1885 +18
Branches 280 285 +5
==========================================
+ Hits 1761 1779 +18
Misses 106 106 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/nodejs.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/nodejs.yml
[skip ci] ## [2.2.0](v2.1.0...v2.2.0) (2024-06-25) ### Features * support cname ([#19](#19)) ([8c64904](8c64904))
closes #18
https://help.aliyun.com/zh/oss/user-guide/map-custom-domain-names-5
Summary by CodeRabbit
New Features
cnameoption for OSSObject.Documentation
endpointvalues.Chores
>= 16.0.0.