Fix misleading typing for options.json. #9275

yuchenshi · 2025-10-07T18:56:26Z

Description

options.json has always been unset since at least 5 years ago but the typing
in options.ts says that it exists. This led to two kinds of mistakes.

Using if (options.json) in command handlers. This effectively creates a
dead branch. I've tried my best to remove those -- should be noop.
Providing json: false when Options are required. It's a not a
noop strictly-speaking but the tests should still pass since it is not
actually accessed in commands. (These are mostly found in tests.)

Alternatives considered: Setting options.json for real seems appealing but
branching on options.json is almost always a mistake. Unless in some obscure
use cases (not found in the code base), it is best to let the global handling
deal with --json the flag.

Things taken care of by global handling:

Suppressing normal output:

firebase-tools/src/command.ts

Lines 315 to 319 in a952fb4

    
           if (getInheritedOption(options, "json")) { 
        
             options.nonInteractive = true; 
        
           } else if (!options.isMCP) { 
        
             useConsoleLoggers(); 
        
           }

Serializing the return value to JSON:

firebase-tools/src/command.ts

Line 211 in a952fb4

if (getInheritedOption(options, "json")) {

Note that the last step wraps the return value in a wrapper object:
{status: "success", result: ...}. The dead branches I've removed all
failed to account for this. So if we had made options.json a real boolean,
it'd cause quite a few breaking changes unless we fix those one by one.

Scenarios Tested

Existing unit tests. I've also logged options.json in commands and made sure
it is always unset regardless of flags.

Sample Commands

N/A

Created using spr 1.3.6-beta.1

gemini-code-assist · 2025-10-07T18:57:01Z

Summary of Changes

Hello @yuchenshi, I'm Gemini Code Assist¹! 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 resolves a long-standing inconsistency in the Options interface where the json property was typed as a boolean but was never actually set by commands. This led to two main issues: dead code branches that checked if (options.json) and incorrect assumptions in tests providing json: false. The changes remove the json property from the BaseOptions interface, add a clarifying JSDoc comment to guide future development, and refactor all affected command handlers to remove explicit options.json checks. This ensures that JSON output is handled consistently and globally via the --json flag, simplifying command logic and preventing future misinterpretations of the json option.

Highlights

Typing Correction: The json property has been removed from the BaseOptions interface and defaultOptions in options.ts, correcting a long-standing misleading type definition.
Code Simplification: All conditional checks for options.json within command handlers have been removed, streamlining the code and eliminating dead branches.
Consistent JSON Output Handling: The system now exclusively relies on the global --json flag handling, ensuring that commands consistently produce JSON output when requested, without requiring explicit checks in individual command logic.
Non-Interactive Mode Enforcement: When the --json flag is detected, options.interactive is explicitly set to false in command.ts, reinforcing non-interactive behavior for JSON output.
Test Refinements: json: false has been removed from various test option configurations, aligning tests with the corrected understanding that options.json is not directly set by commands.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This is an excellent pull request that addresses a long-standing issue with the misleading options.json typing. The changes are thorough and systematically remove the incorrect usage pattern across the codebase, replacing it with the intended framework-level handling of JSON output. The update to options.ts to explicitly type json as undefined and include a detailed comment explaining the correct approach is a fantastic improvement for future maintainability. The removal of dead and buggy code branches in various command files simplifies the logic and improves correctness. Overall, this is a high-quality cleanup and refactoring effort.

yuchenshi · 2025-10-07T18:58:16Z

firebase-vscode/src/options.ts

  projectNumber: "",
  projectRoot: "",
  account: "",
-  json: true,


The VSCE right now doesn't call Firestore commands. So I believe this should be a noop.

src/command.ts

yuchenshi · 2025-10-07T19:00:30Z

src/commands/firestore-backups-list.ts

    const backups: Backup[] = listBackupsResponse.backups || [];

-    if (options.json) {
-      logger.info(JSON.stringify(listBackupsResponse, undefined, 2));


This branch isn't consistent with the return backups. I believe the intention was to print listBackupsResponse when --json is used. But fulfilling that intention will cause a breaking change. So I kept it WAI (working as implemented) and left a TODO instead

That was indeed the intention. It's possible that I didn't notice this when this was implemented the first time... sorry.

No problem. The next major (breaking) release of the CLI should be coming up soon and feel free to make it return listBackupsResponse by then

src/requireConfig.spec.ts

joehan

This looks good to me once you remove the now unused logger imports that are making the linter mad.

joehan · 2025-10-07T19:08:22Z

@rwhogg FYI, for future commands

Created using spr 1.3.6-beta.1

* Remove overrides that diverge the test configuration from the build configuration. (#9300) Co-authored-by: Jamie Rothfeder <[email protected]> * Fix misleading typing for options.json. (#9275) * feat(dataconnect): Add confirmation for Gemini schema generation (#9282) * feat(dataconnect): add confirmation for Gemini schema generation Instead of directly asking for an app description to generate a schema with Gemini, this change first asks the user to confirm if they want to use Gemini. If the user confirms, it then prompts for the app description with a default value of "an app for ${setup.projectId}". * prompts * changelog * m * feedback * typo * metrics * Update index.ts --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> --------- Co-authored-by: Jamie Rothfeder <[email protected]> Co-authored-by: Jamie Rothfeder <[email protected]> Co-authored-by: Yuchen Shi <[email protected]> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

[spr] initial version

81a3962

Created using spr 1.3.6-beta.1

yuchenshi requested a review from joehan October 7, 2025 18:56

github-project-automation bot added this to [Cloud] Extensions + Functions Oct 7, 2025

gemini-code-assist bot reviewed Oct 7, 2025

View reviewed changes

yuchenshi commented Oct 7, 2025

View reviewed changes

src/command.ts Show resolved Hide resolved

yuchenshi commented Oct 7, 2025

View reviewed changes

src/requireConfig.spec.ts Show resolved Hide resolved

joehan reviewed Oct 7, 2025

View reviewed changes

joehan approved these changes Oct 7, 2025

View reviewed changes

github-project-automation bot moved this to Approved [PR] in [Cloud] Extensions + Functions Oct 7, 2025

joehan requested a review from rwhogg October 7, 2025 19:07

rwhogg approved these changes Oct 7, 2025

View reviewed changes

rebase and remove unused vars

9586645

Created using spr 1.3.6-beta.1

yuchenshi enabled auto-merge (squash) October 7, 2025 21:57

yuchenshi added 2 commits October 7, 2025 15:01

rebase

30fa19b

Created using spr 1.3.6-beta.1

remove more unused vars

ad4aa52

Created using spr 1.3.6-beta.1

yuchenshi disabled auto-merge October 13, 2025 19:32

remove test changes

818f28a

Created using spr 1.3.6-beta.1

yuchenshi enabled auto-merge (squash) October 13, 2025 19:34

update tests with --json

c20e771

Created using spr 1.3.6-beta.1

yuchenshi merged commit 1157731 into master Oct 13, 2025
71 of 74 checks passed

yuchenshi deleted the spr/yuchenshi/fix-misleading-typing-for-optionsjson branch October 13, 2025 20:35

github-project-automation bot moved this from Approved [PR] to Done in [Cloud] Extensions + Functions Oct 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix misleading typing for options.json. #9275

Fix misleading typing for options.json. #9275

Uh oh!

yuchenshi commented Oct 7, 2025

Uh oh!

gemini-code-assist bot commented Oct 7, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

yuchenshi Oct 7, 2025

Uh oh!

Uh oh!

yuchenshi Oct 7, 2025

Uh oh!

rwhogg Oct 7, 2025

Uh oh!

yuchenshi Oct 7, 2025

Uh oh!

Uh oh!

joehan left a comment

Uh oh!

joehan commented Oct 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (getInheritedOption(options, "json")) {
	options.nonInteractive = true;
	} else if (!options.isMCP) {
	useConsoleLoggers();
	}

Fix misleading typing for options.json. #9275

Fix misleading typing for options.json. #9275

Uh oh!

Conversation

yuchenshi commented Oct 7, 2025

Description

Scenarios Tested

Sample Commands

Uh oh!

gemini-code-assist bot commented Oct 7, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

yuchenshi Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yuchenshi Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

rwhogg Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

yuchenshi Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joehan left a comment

Choose a reason for hiding this comment

Uh oh!

joehan commented Oct 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants