add validation for the app directory prompt#10618
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds input validation to ensure the specified App Hosting root directory exists. The reviewer noted that the path resolution should use config.path(input) instead of process.cwd() to correctly resolve relative to the firebase.json directory, and suggested removing the unnecessary async keyword.
| validate: async (input: string) => { | ||
| const absPath = path.join(process.cwd(), input); | ||
| if (!existsSync(absPath)) { | ||
| return `Directory ${absPath} does not exist. Please enter a valid directory.`; | ||
| } | ||
| return true; | ||
| }, |
There was a problem hiding this comment.
The prompt asks for the app's root directory relative to the firebase.json directory. However, the validation logic resolves the path using process.cwd(), which may differ from the directory containing firebase.json (e.g., if the CLI is run from a subdirectory).
To ensure correctness, resolve the path using config.path(input) instead of path.join(process.cwd(), input). Additionally, since the validation function does not perform any asynchronous operations, the async keyword can be removed.
| validate: async (input: string) => { | |
| const absPath = path.join(process.cwd(), input); | |
| if (!existsSync(absPath)) { | |
| return `Directory ${absPath} does not exist. Please enter a valid directory.`; | |
| } | |
| return true; | |
| }, | |
| validate: (input: string) => { | |
| const absPath = config.path(input); | |
| if (!existsSync(absPath)) { | |
| return `Directory ${absPath} does not exist. Please enter a valid directory.`; | |
| } | |
| return true; | |
| }, |
There was a problem hiding this comment.
the absPath code was copied from over
Although, aynsc should be removed since there are no promises or awaits inside the function. Good call out!
| default: "/", | ||
| message: "Specify your app's root directory relative to your firebase.json directory", | ||
| validate: (input: string) => { | ||
| const absPath = path.join(process.cwd(), input); |
There was a problem hiding this comment.
This logic assumes that process.cwd() == the firebase directory. However, if you run init from a child of the firebase directory, that will not be true. We've got some utils that resolve the firebase project dir - lets use those instead here.
There was a problem hiding this comment.
Good catch! Running the init command on a sub-directory does break the prompt. We do set a config early on in
Line 52 in 2e3b10d
switching process.cwd() to config.projectDir
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
Description
When prompted for the app hosting app directory and you provide a non-existent path the CLI will error out:
Updated the behavior to validate the path during prompting:
Scenarios Tested
Sample Commands