feat: customized http interceptors #863
Conversation
WalkthroughThe changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DesignCore
participant HttpModule
User->>DesignCore: Call initHttp()
DesignCore->>HttpModule: Initialize HTTP settings
User->>DesignCore: Call createHttp(options)
DesignCore->>HttpModule: Create HTTP instance with options
HttpModule->>User: Return HTTP instance
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
enhancement
![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 (1)
packages/http/src/index.js (1)
91-101: Consider adding type annotations or documentation for clarity.Adding comments or using TypeScript type annotations (if applicable) to specify the expected structure of
requestInterceptorsandresponseInterceptorscan enhance code readability and maintainability.Also applies to: 145-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/design-core/index.js (1 hunks)
- packages/http/src/index.js (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
packages/design-core/index.js (1)
37-37: LGTM. Consider documentation updates.The addition of
initHttpandcreateHttpexports from@opentiny/tiny-engine-httplooks good. This change provides easier access to HTTP-related functionality from the design core package.To ensure this change aligns with the project's architecture and doesn't introduce any unintended side effects, please run the following verification:
#!/bin/bash # Description: Verify the impact of new HTTP exports # Test 1: Check if the exported functions are used elsewhere in the project echo "Checking usage of initHttp and createHttp:" rg --type js "import.*\{.*(?:initHttp|createHttp).*\}.*from.*['\"]@opentiny/tiny-engine-design-core['\"]" -g '!packages/design-core/index.js' # Test 2: Verify if there are any existing HTTP-related utilities that might overlap echo "Checking for existing HTTP-related utilities:" rg --type js "(?:initHttp|createHttp|http\..*|axios\..*)" -g '!packages/design-core/index.js' -g '!packages/http/**'Consider updating the project's documentation to reflect these new exports and provide usage examples for
initHttpandcreateHttp.packages/http/src/index.js (1)
147-156: RefactorresponseInterceptorsloop to handle control flow correctly.Similar to the issue with
requestInterceptors, thereturnstatement inside theforEachcallback does not control the loop as intended. This may result in all interceptors being registered, even when some conditions are met to skip them.Refactor the loop using a
for...ofloop:- responseInterceptors.forEach((item) => { + for (const item of responseInterceptors) { if (Array.isArray(item)) { http.interceptors.response.use(...item) - return + continue } http.interceptors.response.use(item) - }) + }
…to feat/http-customized-interceptor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/design-core/index.js (2)
36-36: LGTM! Well-structured HTTP utility exports.
The addition of initHttp and createHttp exports follows the established pattern and promotes centralized access to HTTP utilities through the design-core package. This is a clean way to expose the new HTTP interceptor functionality.
This architectural approach:
- Maintains consistency with other utility exports
- Provides a single entry point for HTTP-related functionality
- Enables better maintainability and dependency management
36-36: Consider adding JSDoc comments for the new exports.
To maintain consistency with other exports and improve developer experience, consider adding documentation that explains the purpose and usage of these HTTP utilities.
Example documentation:
+/**
+ * Initializes HTTP client with global configuration
+ * @param {Object} config - Global HTTP client configuration
+ */
+/**
+ * Creates an HTTP client instance with custom interceptors
+ * @param {Object} options - Configuration including requestInterceptors and responseInterceptors
+ * @returns {Object} Configured HTTP client instance
+ */
export { initHttp, createHttp } from '@opentiny/tiny-engine-http'📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- designer-demo/src/main.js (1 hunks)
- packages/design-core/index.js (1 hunks)
🔇 Additional comments (2)
designer-demo/src/main.js (2)
15-15: LGTM: Import statement is correctly updated.
The addition of createHttp to the import statement is consistent with the new HTTP interceptor functionality.
25-25: Document and verify the HTTP client configuration.
Please address the following concerns:
- Document the purpose of
force: trueoption - Consider adding validation for the interceptors array
- Verify the global impact of this configuration on all HTTP requests
Let's verify the HTTP client usage across the codebase:
Consider adding a comment explaining the configuration:
+// Configure global HTTP client with organization context interceptor
createHttp({ force: true, requestInterceptors: [preRequest] })| const preRequest = (config) => { | ||
| config.headers['x-org-name'] = 123 | ||
| return config | ||
| } |
There was a problem hiding this comment.
Several improvements needed for the preRequest interceptor.
The current implementation has potential issues:
- The hardcoded value '123' needs documentation explaining its purpose
- Missing error handling for undefined headers
- Direct mutation of the config object could cause side effects
- The purpose of 'x-org-name' header should be documented
Consider this safer implementation:
+/**
+ * HTTP request interceptor that adds organization context
+ * @param {Object} config - The axios request configuration
+ * @returns {Object} Modified config object
+ */
const preRequest = (config) => {
- config.headers['x-org-name'] = 123
+ return {
+ ...config,
+ headers: {
+ ...config.headers,
+ 'x-org-name': '123' // TODO: Document the purpose of this value
+ }
+ }
- return config
}📝 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.
| const preRequest = (config) => { | |
| config.headers['x-org-name'] = 123 | |
| return config | |
| } | |
| /** | |
| * HTTP request interceptor that adds organization context | |
| * @param {Object} config - The axios request configuration | |
| * @returns {Object} Modified config object | |
| */ | |
| const preRequest = (config) => { | |
| return { | |
| ...config, | |
| headers: { | |
| ...config.headers, | |
| 'x-org-name': '123' // TODO: Document the purpose of this value | |
| } | |
| } | |
| } |
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
支持传入自定义的拦截器
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
initHttpandcreateHttpmethods for enhanced HTTP functionalities.createHttpnow supports customizable request interceptors, allowing for greater flexibility in handling HTTP communications.preRequestfunction to modify HTTP request configurations.Bug Fixes
createHttpfunction to ensure more robust performance.