fix(ui): persist follow-up prompt defaults#6126
fix(ui): persist follow-up prompt defaults#6126xsfX20 wants to merge 3 commits intoFlowiseAI:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the FollowUpPrompts component by introducing helper functions to centralize configuration handling and default value logic, while also ensuring state synchronization after saving. Review feedback highlights a validation bug where the value 0 is incorrectly treated as invalid and suggests simplifying nullish checks using the == null idiom for improved readability.
| followUpPromptsConfig[selectedProvider][param] === '' | ||
| ) { | ||
| const value = getProviderInputValue(followUpPromptsConfig, selectedProvider, inputParam) | ||
| if (!value || value === '') { |
There was a problem hiding this comment.
This check incorrectly treats 0 as an invalid value, which would disable the save button if a user sets a field like 'Temperature' to 0. Using value == null will correctly handle 0 as a valid input while still checking for null and undefined.
| if (!value || value === '') { | |
| if (value == null || value === '') { |
| const key = getProviderInputKey(inputParam) | ||
| const value = providerConfig[key] | ||
|
|
||
| return value !== undefined && value !== null && value !== '' ? value : inputParam.default |
There was a problem hiding this comment.
The check for undefined and null can be simplified. Using loose equality (== null) checks for both null and undefined, which is a standard idiom in JavaScript and aligns with the general rules for this repository. This improves readability.
| return value !== undefined && value !== null && value !== '' ? value : inputParam.default | |
| return value != null && value !== '' ? value : inputParam.default |
References
- In JavaScript/TypeScript, use loose equality (== null) as a standard idiom for a 'nullish' check that covers both null and undefined.
| if ((providerConfig[key] === undefined || providerConfig[key] === null || providerConfig[key] === '') && inputParam.default !== undefined) { | ||
| providerConfig[key] = inputParam.default | ||
| } |
There was a problem hiding this comment.
The check for undefined and null can be simplified. Using loose equality (== null) checks for both null and undefined, which is a standard idiom in JavaScript and aligns with the general rules for this repository. This improves readability.
| if ((providerConfig[key] === undefined || providerConfig[key] === null || providerConfig[key] === '') && inputParam.default !== undefined) { | |
| providerConfig[key] = inputParam.default | |
| } | |
| if ((providerConfig[key] == null || providerConfig[key] === '') && inputParam.default !== undefined) { | |
| providerConfig[key] = inputParam.default | |
| } |
References
- In JavaScript/TypeScript, use loose equality (== null) as a standard idiom for a 'nullish' check that covers both null and undefined.
|
Addressed the validation issue from the review.
Pushed in |
|
Follow-up update: I pushed one more small cleanup in
Scope is still intentionally narrow to the follow-up prompts config flow. |
Summary
Testing
git diff --checkpnpm eslint packages/ui/src/ui-component/extended/FollowUpPrompts.jsx(fails locally because this checkout does not currently have the eslint binary installed)