-
Notifications
You must be signed in to change notification settings - Fork 3.7k
feat(intercrptors): 修改了queue和wrapperHook的返回值以支持异步resolve(false)) #5819
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
base: next
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR fixes a bug in the interceptor system where asynchronous hooks returning false caused TypeScript errors when trying to set properties on the boolean value. The changes ensure that both synchronous and asynchronous hooks can properly return false to intercept API execution.
- Modified
wrapperHookto preservefalsevalues instead of coercing them to the original data - Updated the
queuefunction to handle asynchronousfalsereturns by creating mock Promise objects - Added proper handling for both sync and async
falsereturns to prevent property assignment errors
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (isPromise(res)) { | ||
| return res.then((res) => { | ||
| if (res === false) { | ||
| return { then() {}, catch() {} } as Promise<undefined>; |
Copilot
AI
Oct 15, 2025
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.
The mock Promise object with empty then() and catch() methods may cause issues with Promise chaining. Consider using Promise.reject() or a proper rejected Promise to maintain consistent Promise behavior.
| }); | ||
| } | ||
| if (res === false) { | ||
| return { then() {}, catch() {} } as Promise<undefined>; |
Copilot
AI
Oct 15, 2025
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.
The mock Promise object with empty then() and catch() methods may cause issues with Promise chaining. Consider using Promise.reject() or a proper rejected Promise to maintain consistent Promise behavior.
| // 修改这里:处理异步返回 false 的情况 | ||
| promise = res.then((res) => { | ||
| if (res === false) { | ||
| return { then() {}, catch() {} } as Promise<undefined>; |
Copilot
AI
Oct 15, 2025
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.
The mock Promise object with empty then() and catch() methods may cause issues with Promise chaining. Consider using Promise.reject() or a proper rejected Promise to maintain consistent Promise behavior.
| const result = hook(data, params); | ||
| // 只有当结果为undefined或null时才返回data,保留false值 | ||
| return result === undefined || result === null ? data : result; |
Copilot
AI
Oct 15, 2025
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.
The logic change from || data to explicit undefined/null checking is significant and may affect other use cases. Consider adding unit tests to verify this doesn't break existing functionality where hooks return other falsy values like 0 or empty string.
|
@chouchouji @Otto-J 帮忙看下有问题吗? |
在invoke中,同步返回false会拦截当前api的执行,但是异步返回false时,会导致
wrapperOptions中options[name]报错TypeError: Cannot create property 'success' on boolean 'false',此时options是false了,导致了报错我在本地测试了
return Promisi.resolve(false)和return false,同步和异步返回false均能正常拦截当前api的执行