feat: implement progressive CLI help disclosure and dynamic deploy help#10772
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces progressive help for CLI commands, dynamically registering intermediate namespaces and filtering subcommands. It also refactors deploy targets to centralize their metadata, enabling dynamic help generation and detailed target-specific help via firebase help deploy:<target>. The review feedback highlights several opportunities to align with the repository style guide, such as replacing console.log with the central logger, avoiding any type escapes with proper interfaces, correcting an inaccurate reference to database indexes, and improving type safety in dynamic imports to prevent potential prototype pollution.
|
Could you provide some example about how CLI help looks like now? It is a bit hard to comprehend by reading your code. Please provide some CLI example and their output. Thank you |
Done - added a bunch of example output to the description. |
| "Configuration format in firebase.json:\n" + | ||
| "{\n" + | ||
| ' "extensions": {\n' + | ||
| ' "storage-resize-images": "extensions/storage-resize-images.env"\n' + |
There was a problem hiding this comment.
This is wrong. The expected format is "my-extension-instance-id": "firebase/storage-resize-images@^0.1.0"
…ord deploy help text
…te setup from command runner
| .before(requireConfig) | ||
| .before((options: Options) => { | ||
| options.filteredTargets = filterTargets(options, VALID_DEPLOY_TARGETS); | ||
| options.filteredTargets = filterTargets(options, [...VALID_DEPLOY_TARGETS]); |
There was a problem hiding this comment.
Super nitty thing that jetski caught - mutating this const array was a potential future footgun
There was a problem hiding this comment.
I don't think filterTargets mutates the input. The right way is to fix the typing for filterTargets so it takes a const array, to signify that it doesn't mutate the input. This should fix Jetski warnings and also improve the documentation and may even prevent future TypeScript warnings / errors. Anyway this is a super nit and we can wait and see if we will touch this code again
Description
Revamping the --help and firebase help command to use progressive disclosure. Instead of dumping a massive list of commands all at once, you can navigate this layer by layer so that there is not a ton of irrelevant info flooding the terminal (or context).
I also cleaned up some of our Winston logging transport initialization logic - it was spread our over a few places (and was happening late enough to miss some errors early in the flow). Now, it lives at the main entry point for the cli binary.
I also ran our eval suite before and after this change - in cases where the --help command was run, we saw significant performance improvements:
Averages Across Help-Executing Cases
Testing
Before this change,
firebase --helpwould print every single command, andfirebase <command> helpwould print the help text for just that command. There was no support forfirebase <namespace> --help- doing so would just give you a 'command not found error'Some examples of the new behavior - at the top level,
--helpjust returns all of the top level namespaces:To dig deeper you request help on the hosting namespace. It prints only the hosting subcommands and sub-namespaces (like channel and sites), filtering out all other unrelated CLI commands:
This applies for all namespace levels - for example, running help on
hosting:channellists all leaf commands scoped strictly under that namespace:I also improved the flow for deploy - that is one of our most complicated comands, and it previously offered no meaningful help:
Now, it offers details about all the targets:
And, you can view specific details about how to use a target: