-
Notifications
You must be signed in to change notification settings - Fork 7.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: easy to define fieldName of response data #5442
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web-antd/src/api/request.tsOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThis pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant DefaultResponseInterceptor
participant AuthenticateResponseInterceptor
Client->>DefaultResponseInterceptor: Send API Response
DefaultResponseInterceptor->>DefaultResponseInterceptor: Process Response
DefaultResponseInterceptor->>AuthenticateResponseInterceptor: Pass Processed Response
AuthenticateResponseInterceptor->>Client: Return Final Response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web-antd/src/api/request.ts (1)
74-82
: Consider making response configuration more flexible.The hardcoded configuration values could be externalized to support different API patterns:
- Move configuration to environment variables or config file
- Allow for complex success code validation
Example configuration approach:
+ // config/api.ts + export const apiConfig = { + response: { + codeField: process.env.API_CODE_FIELD || 'code', + dataField: process.env.API_DATA_FIELD || 'data', + // Support range checks or multiple success codes + successCode: (code: number) => code >= 200 && code < 300 || code === 0, + }, + }; client.addResponseInterceptor( defaultResponseInterceptor({ - codeField: 'code', - dataField: 'data', - successCode: 0, + ...apiConfig.response, }), );apps/web-ele/src/api/request.ts (1)
74-82
: Consider extracting common request client configuration.The interceptor configuration is duplicated across web variants. Consider:
- Creating a shared configuration module
- Extracting common request client setup into a base factory
Example approach:
// packages/effects/request/src/factories/create-base-client.ts export function createBaseRequestClient(baseURL: string, options?: RequestClientOptions) { const client = new RequestClient({ ...options, baseURL, }); client.addResponseInterceptor( defaultResponseInterceptor({ codeField: apiConfig.response.codeField, dataField: apiConfig.response.dataField, successCode: apiConfig.response.successCode, }), ); return client; }Then in each web variant:
function createRequestClient(baseURL: string, options?: RequestClientOptions) { const client = createBaseRequestClient(baseURL, options); // Add app-specific interceptors return client; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web-antd/src/api/request.ts
(2 hunks)apps/web-ele/src/api/request.ts
(2 hunks)apps/web-naive/src/api/request.ts
(2 hunks)packages/effects/request/src/request-client/preset-interceptors.ts
(1 hunks)packages/effects/request/src/request-client/request-client.test.ts
(0 hunks)packages/effects/request/src/request-client/request-client.ts
(1 hunks)playground/src/api/request.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/effects/request/src/request-client/request-client.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (6)
packages/effects/request/src/request-client/request-client.ts (1)
3-3
: Ensure the removal of default response interceptor does not affect response handlingThe removal of the default response interceptor from
RequestClient
may impact how responses are processed. Please verify that all instances ofRequestClient
now include the necessary response interceptors to handle responses appropriately.apps/web-naive/src/api/request.ts (2)
10-10
: ImportingdefaultResponseInterceptor
is appropriateThe addition of
defaultResponseInterceptor
to the imports is correct and aligns with the new response handling logic.
74-81
: Confirm the configuration ofdefaultResponseInterceptor
The configuration of
defaultResponseInterceptor
withcodeField: 'code'
,dataField: 'data'
, andsuccessCode: 0
appears appropriate. Ensure that these settings match your API's response structure.playground/src/api/request.ts (2)
10-10
: Correctly importingdefaultResponseInterceptor
The import of
defaultResponseInterceptor
is correctly added to support the new response processing.
74-82
: Validate the interceptor's configurationThe
defaultResponseInterceptor
is configured withcodeField: 'code'
,dataField: 'data'
, andsuccessCode: 0
. Please confirm that these values align with your API's response structure to ensure proper handling.packages/effects/request/src/request-client/preset-interceptors.ts (1)
Line range hint
47-168
: LGTM! Interceptor chain order is correctly maintained.The existing interceptors work well with the new
defaultResponseInterceptor
. The order ensures proper handling:
- Response data format validation
- Authentication checks
- Generic error handling
downloader 和 uploader 没有做相应的调整,不能支持 responseReturn 参数,例如 request client 的 responseReturn 设置为 data 时,此时如果有下载接口,返回 Blob ,downloader 的 responseReturn 需要特殊设置为 body 以获取 Blob 对象。甚至如果需要获取下载接口的返回头中的文件名的话,downloader 的 responseReturn 需要特殊设置为 raw 以获取返回头信息。 |
Description
改进requestClient,更容易地定义返回数据的字段名,支持更多的方式判断是否访问成功的逻辑以兼容不同的后端数据结构。
返回数据的code名称支持自定义且不限于数值类型,判断是否访问成功除了简单对比code是否一致以外也兼容范围判断或者更复杂的判断方式(使用自定义的函数判断是否访问成功),获取接口主要数据的方式除了指定data字段名以外,也可以传入一个自定义的函数用于更复杂的环境(比如有些翻页数据接口把total放在了code这一层)
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
New Features
defaultResponseInterceptor
across multiple projects to standardize API response handlingRefactor
RequestClient
class by removing previous response interceptorChores