-
Notifications
You must be signed in to change notification settings - Fork 143
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
fix(comp:tree): searchKeys is affected by the side effects of other v… #1483
base: main
Are you sure you want to change the base?
Conversation
{ | ||
immediate: true, | ||
}, | ||
) | ||
|
||
return { searchedKeys } | ||
} |
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.
with a brief code review
In this code patch, the main changes are importing watch from vue and replacing computed with useState from @idux/cdk/utils.
It also modifies the logic of searching nodes by keywords.
The first thing to check is if the changes made are syntactically correct. For example, does the function useSearchable still compile and run? Are there any errors in the code? If not, then the syntax is correct.
Next, we should check for any potential bugs, such as whether the search function works correctly for all conditions. Is it possible for the setSearchedKeys function to not be called when the searchValue changes, or for it to be called twice?
Finally, we should look for any potential improvements that could be made. For example, is there a way to make the code more efficient or readable? Could the logic of the search function be simplified?
This preview will be available after the AzureCI is passed. |
Codecov Report
@@ Coverage Diff @@
## main #1483 +/- ##
=======================================
Coverage 92.80% 92.80%
=======================================
Files 331 331
Lines 30661 30661
Branches 3527 3527
=======================================
Hits 28454 28454
Misses 2207 2207 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
这样修改会导致,改了 dataSource 不生效。
…ariables
fix #1462
PR Checklist
Please check if your PR fulfills the following requirements:
What is the current behavior?
What is the new behavior?
此issue中的demo由于设置了动态disabled,导致修改到了
mergedNodeMap.value
值,而searchKeys
在computed
中被mergedNodeMap.value
收集到了依赖,所以更新了searchKeys
,进而影响到了expandedKeys
; 所以把computed
改为watch
,使得searchKeys
仅被props.searchValue
影响Other information