Skip to content

feat(cli): add --firebase-out flag for custom firebase.json path#435

Open
kevmoo wants to merge 2 commits intoinvertase:mainfrom
kevmoo:i277_firebase_json_output
Open

feat(cli): add --firebase-out flag for custom firebase.json path#435
kevmoo wants to merge 2 commits intoinvertase:mainfrom
kevmoo:i277_firebase_json_output

Conversation

@kevmoo
Copy link
Copy Markdown

@kevmoo kevmoo commented Apr 16, 2026

Fixes #277

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a --firebase-out flag to the configure and reconfigure commands, allowing users to specify a custom path for the firebase.json file. The changes involve updating argument parsing, path resolution logic, and adding a corresponding test case. Reviewer feedback suggests that custom paths should be resolved relative to the project root to ensure consistent behavior across different execution environments.

Comment thread packages/flutterfire_cli/lib/src/commands/config.dart
Comment thread packages/flutterfire_cli/lib/src/commands/reconfigure.dart Outdated
@kevmoo
Copy link
Copy Markdown
Author

kevmoo commented Apr 16, 2026

/gemini review

@github-actions github-actions bot added the Needs Attention OP created or responded to issue and it needs attention. label Apr 16, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new --firebase-out flag to the configure and reconfigure commands, enabling users to specify a custom path for the firebase.json file. The path resolution logic is now centralized in the FlutterFireCommand base class, and the changes are verified with new integration tests. Feedback highlights the need for defensive programming to handle cases where the Flutter app context might be null and recommends consistent help text for the new flag across different commands.

Comment on lines +51 to +63
String resolveFirebaseJsonPath(String? customPath) {
if (customPath != null) {
// If a custom path was provided, check if it is relative.
if (path.isRelative(customPath)) {
// Resolve relative paths relative to the project root to ensure consistent behavior.
return path.join(flutterApp!.package.path, customPath);
}
// Use absolute paths as provided.
return customPath;
}
// Default to the standard firebase.json in the project root if no custom path is specified.
return path.join(flutterApp!.package.path, 'firebase.json');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The resolveFirebaseJsonPath method uses the null-check operator (!) on flutterApp multiple times. While most commands in this CLI require a Flutter app, FlutterFireCommand allows flutterApp to be null. If this method is called when flutterApp is null, it will throw a runtime error. Consider adding a defensive check or using a fallback like Directory.current.path to determine the project root.

Suggested change
String resolveFirebaseJsonPath(String? customPath) {
if (customPath != null) {
// If a custom path was provided, check if it is relative.
if (path.isRelative(customPath)) {
// Resolve relative paths relative to the project root to ensure consistent behavior.
return path.join(flutterApp!.package.path, customPath);
}
// Use absolute paths as provided.
return customPath;
}
// Default to the standard firebase.json in the project root if no custom path is specified.
return path.join(flutterApp!.package.path, 'firebase.json');
}
String resolveFirebaseJsonPath(String? customPath) {
final projectRoot = flutterApp?.package.path ?? Directory.current.path;
if (customPath != null) {
// If a custom path was provided, check if it is relative.
if (path.isRelative(customPath)) {
// Resolve relative paths relative to the project root to ensure consistent behavior.
return path.join(projectRoot, customPath);
}
// Use absolute paths as provided.
return customPath;
}
// Default to the standard firebase.json in the project root if no custom path is specified.
return path.join(projectRoot, 'firebase.json');
}

Comment on lines +71 to +75
argParser.addOption(
kFirebaseOutFlag,
valueHelp: 'filePath',
help: 'The path to the `firebase.json` file.',
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The help text for the firebase-out flag in the Reconfigure command is slightly inconsistent with the one in ConfigCommand. In ConfigCommand, it mentions that the file will be "generated or updated", whereas here it is just "The path to the firebase.json file.". For better user experience, consider using a consistent and descriptive help message across all commands.

Suggested change
argParser.addOption(
kFirebaseOutFlag,
valueHelp: 'filePath',
help: 'The path to the `firebase.json` file.',
);
argParser.addOption(
kFirebaseOutFlag,
valueHelp: 'filePath',
help: 'The path to the `firebase.json` file that will be used for reconfiguration.',
);

@karnemanideep
Copy link
Copy Markdown

"Hi! I noticed the CI tests are failing.
I'd love to help fix them if you need
an extra hand!"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Attention OP created or responded to issue and it needs attention.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

request: Configurable firebase.json output directory ("--firebase-out=<PATH>")

2 participants