Lazy load commands #9519
Lazy load commands #9519
Conversation
- Updated `src/commands/index.ts` to make `loadCommand` return a lazy runner that only `require`s and registers the command when executed or explicitly loaded. - Updated `src/index.ts` to handle lazy loading in the catch-all command handler by traversing the `client` object and loading the matching command before re-parsing arguments. - Updated `src/bin/cli.ts` to load all commands when no arguments are provided (global help), ensuring the help text is fully populated. - Added cycle detection to the `loadAll` logic to handle circular references in the `client` object.
- Updated `src/commands/index.ts` to make `loadCommand` return a lazy runner that only `require`s and registers the command when executed or explicitly loaded. - Updated `src/index.ts` to handle lazy loading in the catch-all command handler by traversing the `client` object and loading the matching command before re-parsing arguments. - Updated `src/bin/cli.ts` to load all commands when no arguments are provided (global help), ensuring the help text is fully populated. - Added cycle detection to the `loadAll` logic to handle circular references in the `client` object. - Added `scripts/benchmark_load.ts` to measure command load time.
Summary of ChangesHello @joehan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant performance optimization by implementing lazy loading for Firebase CLI commands. Previously, all commands were loaded at startup, leading to noticeable delays. With this change, commands are now loaded on demand, dramatically reducing the initial overhead and improving the responsiveness of the CLI for most operations. The core logic involves refactoring how commands are registered and executed, with special handling for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces lazy loading for commands to significantly improve CLI startup time. The approach is solid, but there are a couple of issues to address. First, the implementation in src/commands/index.ts can lead to commands being registered multiple times, which is inefficient and could cause subtle bugs. I've suggested a refactor to prevent this. Second, the handling of the help command in src/bin/cli.ts has a regression that breaks firebase help <command>. I've provided a fix for this as well. Addressing these points will make this performance improvement robust.
1 participant
Description
Lazy load commands as needed, instead of 'require'ing them all at once at the start of every command
Scenarios Tested
Manually validated that commands still work, and check that 'firebase' and 'firebase help' still work. Also tested out flags, both that expected ones are allowed and unexpected ones as not:
Also tested out that nested commands still work - ie
firebase targetandfirebase target:applyAdded some timer code, and found that previously, we would do about 800ms of loading before every command - now it is <10ms.
Before this change - every commands would take about 800ms upfront:
After - different commands take differing amounts of time, but range from >10ms (for smaller ones) to ~300ms (for the ones with the largest dep trees)
Finally, I also validated that
require('firebase-tools').getCommand("commandName")still works. IDK if anyone in the world uses this, but we technically export it (and use it in the barely usedfirebase extcommand)