fix(functions): preselect list param defaults in multi-select prompts - #11062
Ishkirat-Singh wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements preselection of default values for list function parameters in multi-select prompts. It updates promptSelectMultiple to check if each option's value is included in the resolved defaults, adds a corresponding unit test, and updates the changelog. The reviewer suggested a more robust approach to preselection by stringifying the elements of resolvedDefault outside the loop to prevent potential runtime type mismatches and avoid recreating the mapped array on every iteration.
| const response = await checkbox({ | ||
| // `default` only serves non-interactive mode; the checkbox prompt itself | ||
| // preselects through `checked` on each choice. | ||
| default: resolvedDefault, | ||
| message: prompt, | ||
| choices: input.multiSelect.options.map((option: SelectOptions<string>): ListItem => { | ||
| return { | ||
| checked: false, | ||
| checked: resolvedDefault?.includes(option.value.toString()) ?? false, | ||
| name: option.label, | ||
| value: option.value.toString(), | ||
| }; |
There was a problem hiding this comment.
To make the preselection more robust against potential runtime type mismatches (e.g., if default values are parsed as numbers or booleans from environment files or configuration), we should stringify the elements of resolvedDefault before performing the .includes() check. Pre-mapping the defaults outside the loop also avoids recreating the mapped array on every iteration.
const stringifiedDefaults = resolvedDefault?.map((val) => String(val));
const response = await checkbox({
// default only serves non-interactive mode; the checkbox prompt itself
// preselects through checked on each choice.
default: resolvedDefault,
message: prompt,
choices: input.multiSelect.options.map((option: SelectOptions<string>): ListItem => {
return {
checked: stringifiedDefaults?.includes(option.value.toString()) ?? false,
name: option.label,
value: option.value.toString(),
};There was a problem hiding this comment.
Done in 4c59a1a: defaults are stringified once into a Set before the choices are built.
a36b048 to
e958bab
Compare
promptSelectMultiple passed the resolved default to the checkbox prompt as `default`, but @inquirer/checkbox has no such option: it preselects through `checked` on each choice, and every choice was hardcoded to `checked: false`. A `defineList` param with a multi-select input therefore always opened with nothing selected, whatever its declared default. Mark the choices whose value is in the resolved default as checked. `default` is kept because the non-interactive path still returns it.
e958bab to
62eacf8
Compare
|
Updated to latest and verified against #11086, per @ajperel's note on #11059.
|
ajperel
left a comment
There was a problem hiding this comment.
You'll need to update merge conflicts in CHANGELOG before I can merge this, but overall this looks good. Thank you! Maybe in the future we'll look into making harder to do this incorrectly (cause I can easily see this bug happening elsewhere) but either way this is a good fix for now.
| @@ -1,2 +1,8 @@ | |||
| - [changed] Improve formatting and user experience for Cloud Functions parameter and secret prompts. | |||
| - [fixed] Fail fast with an actionable error and remediation instructions when declarative security APIs (IAM and Cloud Resource Manager) are disabled on the project. | |||
There was a problem hiding this comment.
This is re-adding a bunch of removed CHANGELOG entries from the last release. It needs to just be current HEAD state + your change.
Description
promptSelectMultiplepassed the resolved default to thecheckboxprompt asdefault, but@inquirer/checkboxhas no such option — it preselects throughcheckedon each choice, and every choice was hardcoded tochecked: false. AdefineListparam with a multi-select input therefore always opened with nothing selected, whatever default it declared.This marks the choices whose value is in the resolved default as
checked.defaultis kept because the non-interactive path (guard) still returns it.Found while fixing #11053 in the same file; there is no separate issue for this one.
Scenarios Tested
preselects the default values in a multi-select prompttosrc/deploy/functions/params.spec.ts: a list param with options a/b/c and default["b", "c"]produces choices checked[false, true, true]and resolves to the expectedParamValue.npx mocha src/deploy/functions/params.spec.ts: 19 passing; the new test fails againstmainwithout the fix.Sample Commands
N/A — the change is in the interactive
firebase deploy --only functionsparam prompt.