@tus/server: introduce options.exposedHeaders #759
Conversation
🦋 Changeset detectedLatest commit: 1140361 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
""" WalkthroughThe changes introduce a new Changes
Assessment against linked issues
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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 (1)
packages/server/src/server.ts (1)
203-206: Consider handling non-array values more explicitly.The implementation works but could be more robust against unexpected runtime values. While TypeScript ensures type safety at compile time, consider a more defensive approach for runtime safety.
- headers.set( - 'Access-Control-Expose-Headers', - [...HEADERS, this.options.exposedHeaders ?? []].join(', ') - ) + headers.set( + 'Access-Control-Expose-Headers', + HEADERS.concat(Array.isArray(this.options.exposedHeaders) ? this.options.exposedHeaders : []).join(', ') + )This change ensures that even if
exposedHeadersis somehow not an array at runtime, the code won't throw an error.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/server/README.md(1 hunks)packages/server/src/server.ts(2 hunks)packages/server/src/test/Server.test.ts(1 hunks)packages/server/src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/server/src/server.ts (1)
packages/utils/src/constants.ts (1)
HEADERS(3-21)
🔇 Additional comments (4)
packages/server/src/types.ts (1)
34-37: LGTM! Good addition of the exposedHeaders option.The type definition and documentation for the new
exposedHeadersoption are correctly implemented, following the same pattern as the existingallowedHeadersoption. This is a useful feature for users who need to expose additional custom headers in CORS responses.packages/server/README.md (1)
101-104: Documentation looks good!The documentation for the new
exposedHeadersoption is clear, concise, and follows the same style as the other options. The placement afterallowedHeadersis logical and makes it easy for users to discover related features.packages/server/src/server.ts (1)
7-7: Updated import looks good.Correctly updated the import to use
HEADERSinstead ofEXPOSED_HEADERSfrom the @tus/utils package.packages/server/src/test/Server.test.ts (1)
166-177: Test case looks great!The test case properly verifies that custom headers are included in the
Access-Control-Expose-Headersresponse header. The structure follows the same pattern as other CORS header tests, and it correctly cleans up by resetting the option after the test.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/tidy-items-fry.md (1)
5-5: Refine changeset summary style
For consistency with other changesets, consider ending the summary sentence with a period and, if helpful, briefly noting default behavior or usage. For example:
“IntroduceexposedHeadersoption for custom Access-Control-Expose-Headers.”
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/tidy-items-fry.md(1 hunks)
🔇 Additional comments (1)
.changeset/tidy-items-fry.md (1)
1-3: Verify version bump type
The addition of a newexposedHeadersoption is indeed a feature enhancement, so marking this as a minor release for@tus/serveris correct.
1 participant
Closes #746