-
Notifications
You must be signed in to change notification settings - Fork 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
fix: update option list search logic to handle string and array form_data values #2701
Conversation
…data values --bug=1053919 --user=刘瑞斌 【函数库】非必填启动参数开启启动参数后再次编辑多选置空 https://www.tapd.cn/57709429/s/1677136
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
} else { | ||
return form_data[item.field].indexOf([value_field]) === -1 | ||
} | ||
}) | ||
if (find) { | ||
return { [item.field]: form_data[item.field] } | ||
} |
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 code looks generally correct, but there are a few things to consider for optimization:
-
Code Style: The use of
const
for local variables is good practice. However, TypeScript types should be explicitly defined when using optional chaining (?.
) because it can sometimes lead to unexpectedundefined
values. -
Type Checking: Ensure that the types of inputs to your function and methods are correctly handled. If
'string'
is expected and you want more complex string matching logic (e.g., substring search), this could benefit from additional type checks or functions likeString.includes
. -
Performance Considerations: Since your current implementation iterates over all options to find a match based on a single criteria, this might not scale well with large lists. Consider sorting the list or creating an index if possible.
// Example of potential performance improvement const optionListIndex = new Map(item.option_list.map(opt => [opt[value_field], opt])); if (optionListIndex.has(form_data[item.field])) { return { [item.field]: form_data[item.field] }; }
-
Functionality Clarification: Currently, if no match is found, the returned object will be empty
{}
. For users expecting some indication, you might want to add logging or display messages indicating that no valid option was found.
Based on these points, here's a slightly improved version:
/**
* Renders data based on form inputs.
* @param form_data - An object containing form input data.
* @param items - An array of configuration objects defining how to render each field.
* @returns A JavaScript object representing the rendered data.
*/
function render(form_data, items) {
let result = {};
items.forEach((item) => {
if (form_data[item.field] !== undefined && item.value_field && item.option_list) {
const value_field = item.value_field;
const option_list = item.option_list;
// Create index for faster lookup if needed
const optionListIndex = new Map(option_list.map(opt => [opt[value_field], opt]));
const findValue = () => {
if (typeof form_data[item.field] === 'string') {
return option_list.findIndex(i => i[value_field] === form_data[item.field]);
} else {
return typeof form_data[item.field][value_field] !== 'undefined';
}
};
const index = findValue();
if (index >= 0 || optionListIndex.has(form_data[item.field])) {
result[item.field] = form_data[item.field];
}
}
});
return result;
}
This version adds basic functionality around indexing, which can improve performance. It also includes error handling for undefined indexes in case indexOf
returns -1
.
fix: update option list search logic to handle string and array form_data values --bug=1053919 --user=刘瑞斌 【函数库】非必填启动参数开启启动参数后再次编辑多选置空 https://www.tapd.cn/57709429/s/1677136