Prompt for which functions to update during functions:secrets:set

[Berlioz]

No description provided.

[gemini-code-assist]

Code Review

This pull request updates the functions:secrets:set command to filter endpoints using isFirebaseManaged and introduces an interactive checkbox prompt allowing users to select which functions to re-deploy when multiple endpoints are affected. Feedback on these changes highlights two issues: first, using e.id as the checkbox value can cause conflicts since function IDs are not unique across regions (suggesting using the array index instead); second, passing an empty array [] as a fallback for e.labels causes a TypeScript type mismatch, which should be resolved by using an empty object {}.

[gemini-code-assist]

Using e.id as the choice value in the checkbox prompt is problematic because function IDs are not guaranteed to be unique across different regions (e.g., a function named foo can be deployed in both us-central1 and us-east1). If a user deselects one of them, both will still be kept in endpointsToUpdate because selectedEndpointIds.includes(e.id) will match both. Additionally, there is a missing space in the prompt message concatenation. Using the array index as the choice value resolves this issue cleanly.

Suggested change

	if (endpointsToUpdate.length > 1) {
	const choices = endpointsToUpdate.map((e): Choice<string> => {
	const currentVersion = secrets.getSecretVersions(e)[secret.name];
	return {
	name: `${e.id}(${e.region}) - current secret version: ${currentVersion ?? "unknown"}`,
	value: e.id,
	checked: true,
	};
	});
	const selectedEndpointIds = await checkbox<string>({
	message:
	"Which functions do you want to re-deploy?" +
	"Press Space to select functions, then Enter to confirm your choices.",
	choices: choices,
	});
	endpointsToUpdate = endpointsToUpdate.filter((e) => selectedEndpointIds.includes(e.id));
	}
	if (endpointsToUpdate.length > 1) {
	const choices = endpointsToUpdate.map((e, idx): Choice<number> => {
	const currentVersion = secrets.getSecretVersions(e)[secret.name];
	return {
	name: e.id + "(" + e.region + ") - current secret version: " + (currentVersion ?? "unknown"),
	value: idx,
	checked: true,
	};
	});
	const selectedIndices = await checkbox<number>({
	message:
	"Which functions do you want to re-deploy? " +
	"Press Space to select functions, then Enter to confirm your choices.",
	choices: choices,
	});
	endpointsToUpdate = endpointsToUpdate.filter((_, idx) => selectedIndices.includes(idx));
	}

[inlined]

This is a fair critique only because the UI allowed you to select a function per region. Given that all functions with the same ID share the same source code (unless they're in different codebases for some reason???) I'm OK with just making this selecting ID irrespective of region. Or you can do Gemini's suggestion much easier by just making it a Choice, setting value to e, and you no longer need to filter.

[gemini-code-assist]

Passing an empty array [] as a fallback for e.labels is a type mismatch because isFirebaseManaged expects an object of type { [key: string]: any }. Under strict TypeScript compilation, this will cause a type error. Use an empty object {} instead.

Suggested change

	.filter((e) => isFirebaseManaged(e.labels ?? []))
	.filter((e) => isFirebaseManaged(e.labels ?? {}))

References

1. TypeScript: Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. Use strict null checks and handle undefined/null explicitly. ^(link)

[inlined]

True bug. Labels are not hashtags, they're key value pairs

[inlined]

True bug. Labels are not hashtags, they're key value pairs

[inlined]

This whole if + indentation is unnecessary since you early exited if length === 0 and the value cannot be negative.

[inlined]

This is a fair critique only because the UI allowed you to select a function per region. Given that all functions with the same ID share the same source code (unless they're in different codebases for some reason???) I'm OK with just making this selecting ID irrespective of region. Or you can do Gemini's suggestion much easier by just making it a Choice, setting value to e, and you no longer need to filter.