Skip to content

WIP: nonEmpty param option support #10678

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.

Sign up for GitHub

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

Open
Berlioz wants to merge 3 commits into
base: main
Choose a base branch
from
+56 −17
Open

WIP: nonEmpty param option support #10678

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9581b8e
supports the nonEmpty param option for string and []string type params
Berlioz Jun 17, 2026
05d13e9
forgot to hit save file
Berlioz Jun 17, 2026
5241c1f
clean up error string
Berlioz Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 56 additions & 17 deletions src/deploy/functions/params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@
// Default value. If not provided, a param must be supplied.
default?: T | build.Expression<T>;

// default: false
immutable?: boolean;

// Defines how the CLI will prompt for the value of the param if it's not in .env files
input?: ParamInput<T>;

// Reject the empty string and empty array as valid input.
nonEmpty?: boolean;

@inlined inlined Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how hard it is to make the field only present for the right types. At minimum you can say:

type TypeIfExtends<Result, Test, Base> = Test extends Base ? Result : never;

// ...
nonEmpty: TypeIfExtends<boolean, T, string | string[]>

};

/**
Expand Down Expand Up @@ -272,7 +272,7 @@
return pv;
}

setDelimiter(delimiter: string) {

Check warning on line 275 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Missing return type on function
this.delimiter = delimiter;
}

Expand All @@ -299,7 +299,7 @@
if (this.rawValue.includes("[")) {
// Convert quotes to apostrophes
const unquoted = this.rawValue.replace(/'/g, '"');
return JSON.parse(unquoted);

Check warning on line 302 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unsafe return of an `any` typed value
}

// Continue to handle something like "a,b,c"
Expand Down Expand Up @@ -418,7 +418,7 @@
}
if (paramDefault && !canSatisfyParam(param, paramDefault)) {
throw new FirebaseError(
"Parameter " + param.name + " has default value " + paramDefault + " of wrong type",

Check warning on line 421 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Operands of '+' operation must either be both strings or both numbers. Consider using a template literal
);
}
paramValues[param.name] = await promptParam(param, firebaseConfig.projectId, paramDefault);
Expand Down Expand Up @@ -571,6 +571,7 @@
prompt,
param.input,
resolvedDefault,
param.nonEmpty,

@inlined inlined Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is nonEmpty not part of input?

(res: string[]) => res,
);
} else if (isTextInput(param.input)) {
Expand All @@ -580,15 +581,21 @@
if (param.description) {
prompt += ` \n(${param.description})`;
}
return promptText<string[]>(prompt, param.input, resolvedDefault, (res: string): string[] => {
return res.split(param.delimiter || ",");
});
return promptText<string[]>(
prompt,
param.input,
resolvedDefault,
param.nonEmpty,
(res: string): string[] => {
return res.split(param.delimiter || ",");
},
);
} else if (isResourceInput(param.input)) {
prompt = `Select values for ${param.label || param.name}:`;
if (param.description) {
prompt += ` \n(${param.description})`;
}
return promptResourceStrings(prompt, param.input, projectId);
return promptResourceStrings(prompt, param.input, projectId, param.nonEmpty);
} else {
assertExhaustive(param.input);
}
Expand All @@ -602,7 +609,7 @@
const defaultToText: TextInput<string> = { text: {} };
param.input = defaultToText;
}
const isTruthyInput = (res: string) => ["true", "y", "yes", "1"].includes(res.toLowerCase());

Check warning on line 612 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Missing return type on function
let prompt: string;

if (isSelectInput(param.input)) {
Expand All @@ -619,7 +626,7 @@
if (param.description) {
prompt += ` \n(${param.description})`;
}
return promptText<boolean>(prompt, param.input, resolvedDefault, isTruthyInput);
return promptText<boolean>(prompt, param.input, resolvedDefault, false, isTruthyInput);

@inlined inlined Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please always leave a comment on any boolean literal ever used as a parameter

} else if (isResourceInput(param.input)) {
throw new FirebaseError("Boolean params cannot have Cloud Resource selector inputs");
} else {
Expand All @@ -643,7 +650,7 @@
if (param.description) {
prompt += ` \n(${param.description})`;
}
return promptResourceString(prompt, param.input, projectId, resolvedDefault);
return promptResourceString(prompt, param.input, projectId, param.nonEmpty, resolvedDefault);
} else if (isMultiSelectInput(param.input)) {
throw new FirebaseError("Non-list params cannot have multi selector inputs");
} else if (isSelectInput(param.input)) {
Expand All @@ -658,7 +665,13 @@
if (param.description) {
prompt += ` \n(${param.description})`;
}
return promptText<string>(prompt, param.input, resolvedDefault, (res: string) => res);
return promptText<string>(
prompt,
param.input,
resolvedDefault,
param.nonEmpty,
(res: string) => res,
);
} else {
assertExhaustive(param.input);
}
Expand Down Expand Up @@ -693,7 +706,7 @@
if (param.description) {
prompt += ` \n(${param.description})`;
}
return promptText<number>(prompt, param.input, resolvedDefault, (res: string) => {
return promptText<number>(prompt, param.input, resolvedDefault, false, (res: string) => {
if (isNaN(+res)) {
return { message: `"${res}" could not be converted to a number.` };
}
Expand All @@ -713,16 +726,17 @@
prompt: string,
input: ResourceInput,
projectId: string,
disallowEmpty: boolean | undefined,

@inlined inlined Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why disallowEmpty and nonEmpty? Also, why mandatory but supports undefined?

resolvedDefault?: string,
): Promise<string> {
Comment thread
Berlioz marked this conversation as resolved.
const notFound = new FirebaseError(`No instances of ${input.resource.type} found.`);
switch (input.resource.type) {
case "storage.googleapis.com/Bucket":
const buckets = (await listBuckets(projectId)).map((b) => b.name);

Check warning on line 735 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected lexical declaration in case block
if (buckets.length === 0) {
throw notFound;
}
const forgedInput: SelectInput<string> = {

Check warning on line 739 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected lexical declaration in case block
select: {
options: buckets.map((bucketName: string): SelectOptions<string> => {
return { label: bucketName, value: bucketName };
Expand All @@ -734,40 +748,55 @@
logger.warn(
`Warning: unknown resource type ${input.resource.type}; defaulting to raw text input...`,
);
return promptText<string>(prompt, { text: {} }, resolvedDefault, (res: string) => res);
return promptText<string>(
prompt,
{ text: {} },
resolvedDefault,
disallowEmpty,
(res: string) => res,
);
}
}

async function promptResourceStrings(
prompt: string,
input: ResourceInput,
projectId: string,
disallowEmpty: boolean | undefined,
): Promise<string[]> {
const notFound = new FirebaseError(`No instances of ${input.resource.type} found.`);
switch (input.resource.type) {
case "storage.googleapis.com/Bucket":
const buckets = (await listBuckets(projectId)).map((b) => b.name);

Check warning on line 770 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected lexical declaration in case block
if (buckets.length === 0) {
throw notFound;
}
const forgedInput: MultiSelectInput = {

Check warning on line 774 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected lexical declaration in case block
multiSelect: {
options: buckets.map((bucketName: string): SelectOptions<string> => {
return { label: bucketName, value: bucketName };
}),
},
};
return promptSelectMultiple<string>(prompt, forgedInput, undefined, (res: string[]) => res);
return promptSelectMultiple<string>(
prompt,
forgedInput,
undefined,
disallowEmpty,
(res: string[]) => res,
);
default:
logger.warn(
`Warning: unknown resource type ${input.resource.type}; defaulting to raw text input...`,
);
return promptText<string[]>(prompt, { text: {} }, undefined, (res: string) => res.split(","));
return promptText<string[]>(prompt, { text: {} }, undefined, disallowEmpty, (res: string) =>
res.split(","),
);
}
}

type retryInput = { message: string };
function shouldRetry(obj: any): obj is retryInput {

Check warning on line 799 in src/deploy/functions/params.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected any. Specify a different type
return typeof obj === "object" && (obj as retryInput).message !== undefined;
}

Expand All @@ -775,6 +804,7 @@
prompt: string,
textInput: TextInput<T>,
resolvedDefault: T | undefined,
disallowEmpty: boolean | undefined,
converter: (res: string) => T | retryInput,
): Promise<T> {
const res = await input({
Expand All @@ -788,16 +818,20 @@
textInput.text.validationErrorMessage ||
`Input did not match provided validator ${userRe.toString()}, retrying...`,
);
return promptText<T>(prompt, textInput, resolvedDefault, converter);
return promptText<T>(prompt, textInput, resolvedDefault, disallowEmpty, converter);
}
}
if (disallowEmpty && res === "") {
logger.error(`Input cannot be the empty string, retrying...`);
return promptText<T>(prompt, textInput, resolvedDefault, disallowEmpty, converter);
}
// TODO(vsfan): the toString() is because PromptOnce()'s return type of string
// is wrong--it will return the type of the default if selected. Remove this
// hack once we fix the prompt.ts metaprogramming.
const converted = converter(res.toString());
if (shouldRetry(converted)) {
logger.error(converted.message);
return promptText<T>(prompt, textInput, resolvedDefault, converter);
return promptText<T>(prompt, textInput, resolvedDefault, disallowEmpty, converter);
}
return converted;
}
Expand Down Expand Up @@ -831,6 +865,7 @@
prompt: string,
input: MultiSelectInput,
resolvedDefault: T[] | undefined,
disallowEmpty: boolean | undefined,
converter: (res: string[]) => T[] | retryInput,
): Promise<T[]> {
const response = await checkbox({
Expand All @@ -847,7 +882,11 @@
const converted = converter(response);
if (shouldRetry(converted)) {
logger.error(converted.message);
return promptSelectMultiple<T>(prompt, input, resolvedDefault, converter);
return promptSelectMultiple<T>(prompt, input, resolvedDefault, disallowEmpty, converter);
}
if (disallowEmpty && Array.isArray(converted) && converted.length === 0) {
logger.error(`Input cannot be empty, retrying...`);
return promptSelectMultiple<T>(prompt, input, resolvedDefault, disallowEmpty, converter);
}
Comment thread
Berlioz marked this conversation as resolved.
return converted;
}
Loading