-
Notifications
You must be signed in to change notification settings - Fork 75
feat(cli): add --firebase-out flag for custom firebase.json path #435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -56,18 +56,27 @@ class ConfigFileWrite { | |||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| class Reconfigure extends FlutterFireCommand { | ||||||||||||||||||||||
| Reconfigure(FlutterApp? flutterApp, {String? token}) : super(flutterApp) { | ||||||||||||||||||||||
| Reconfigure(FlutterApp? flutterApp, {String? token, String? firebaseJsonPath}) | ||||||||||||||||||||||
| : super(flutterApp) { | ||||||||||||||||||||||
| setupDefaultFirebaseCliOptions(); | ||||||||||||||||||||||
| _accessToken = token; | ||||||||||||||||||||||
| _firebaseJsonPath = firebaseJsonPath; | ||||||||||||||||||||||
| argParser.addOption( | ||||||||||||||||||||||
| 'ci-access-token', | ||||||||||||||||||||||
| valueHelp: 'ciAccessToken', | ||||||||||||||||||||||
| hide: true, | ||||||||||||||||||||||
| help: | ||||||||||||||||||||||
| 'Set the access token for making Firebase API requests. Required for CI environment.', | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| argParser.addOption( | ||||||||||||||||||||||
| kFirebaseOutFlag, | ||||||||||||||||||||||
| valueHelp: 'filePath', | ||||||||||||||||||||||
| help: 'The path to the `firebase.json` file.', | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
Comment on lines
+71
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The help text for the
Suggested change
|
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| String? _firebaseJsonPath; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @override | ||||||||||||||||||||||
| final String description = | ||||||||||||||||||||||
| 'Updates the configurations for all build variants included in the "firebase.json" added by running `flutterfire configure`.'; | ||||||||||||||||||||||
|
|
@@ -378,12 +387,12 @@ class Reconfigure extends FlutterFireCommand { | |||||||||||||||||||||
| Future<void> run() async { | ||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| commandRequiresFlutterApp(); | ||||||||||||||||||||||
| final firebaseJson = File( | ||||||||||||||||||||||
| path.join( | ||||||||||||||||||||||
| flutterApp!.package.path, | ||||||||||||||||||||||
| 'firebase.json', | ||||||||||||||||||||||
| ), | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| final customPath = | ||||||||||||||||||||||
| argResults != null ? argResults![kFirebaseOutFlag] as String? : null; | ||||||||||||||||||||||
| // Determine the raw path, prioritizing the programmatic path passed from the configure command. | ||||||||||||||||||||||
| final rawPath = _firebaseJsonPath ?? customPath; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| final firebaseJson = File(resolveFirebaseJsonPath(rawPath)); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (!firebaseJson.existsSync()) { | ||||||||||||||||||||||
| throw Exception( | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
resolveFirebaseJsonPathmethod uses the null-check operator (!) onflutterAppmultiple times. While most commands in this CLI require a Flutter app,FlutterFireCommandallowsflutterAppto be null. If this method is called whenflutterAppis null, it will throw a runtime error. Consider adding a defensive check or using a fallback likeDirectory.current.pathto determine the project root.