-
Notifications
You must be signed in to change notification settings - Fork 76
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 that will be used for reconfiguration.', | ||
| ); | ||
|
russellwheatley marked this conversation as resolved.
|
||
| } | ||
|
|
||
| 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; | ||
|
Comment on lines
+390
to
+393
Member
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. think we can use similar getter used in config command: String get firebaseJsonPath {
return resolveFirebaseJsonPath(argResults![kFirebaseOutFlag] as String?);
} |
||
|
|
||
| final firebaseJson = File(resolveFirebaseJsonPath(rawPath)); | ||
|
|
||
| if (!firebaseJson.existsSync()) { | ||
| throw Exception( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1740,4 +1740,52 @@ void main() { | |
| Duration(minutes: 3), | ||
| ), | ||
| ); | ||
|
|
||
| test('flutterfire configure: --firebase-out flag should dictate where firebase.json is written', () async { | ||
| // Install flutterfire_cli from local path | ||
| final installDevDependency = Process.runSync( | ||
| 'flutter', | ||
| [ | ||
| 'pub', | ||
| 'add', | ||
| '--dev', | ||
| 'flutterfire_cli', | ||
| '--path=${Directory.current.path}', | ||
| ], | ||
| workingDirectory: projectPath, | ||
| ); | ||
|
|
||
| if (installDevDependency.exitCode != 0) { | ||
| fail(installDevDependency.stderr as String); | ||
|
Comment on lines
+1746
to
+1759
Member
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. don't need to install dev, it should be available already. See other tests. |
||
| } | ||
|
|
||
| const customFirebaseJsonPath = 'custom/firebase.json'; | ||
| final result = Process.runSync( | ||
| 'dart', | ||
| [ | ||
| 'run', | ||
| 'flutterfire_cli:flutterfire', | ||
| 'configure', | ||
| '--yes', | ||
| '--project=$firebaseProjectId', | ||
| '--platforms=android', | ||
| '--firebase-out=$customFirebaseJsonPath', | ||
| ], | ||
| workingDirectory: projectPath, | ||
| runInShell: true, | ||
| ); | ||
|
Comment on lines
+1763
to
+1776
Member
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. using dev dependency again, see other tests. |
||
|
|
||
| if (result.exitCode != 0) { | ||
| fail(result.stderr as String); | ||
| } | ||
|
|
||
| // check custom "firebase.json" was created and has correct content | ||
| final firebaseJsonFile = p.join(projectPath!, 'custom', 'firebase.json'); | ||
| expect(File(firebaseJsonFile).existsSync(), true); | ||
|
|
||
| final firebaseJsonFileContent = await File(firebaseJsonFile).readAsString(); | ||
| final decodedFirebaseJson = jsonDecode(firebaseJsonFileContent) as Map<String, dynamic>; | ||
|
|
||
| expect(decodedFirebaseJson[kFlutter], isNotNull); | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -393,4 +393,77 @@ void main() { | |
| Duration(minutes: 2), | ||
| ), | ||
| ); | ||
|
|
||
| test( | ||
| 'flutterfire reconfigure: should use custom firebase.json path specified by --firebase-out', | ||
| () async { | ||
| const customFirebaseJsonPath = 'custom/firebase.json'; | ||
| final scriptPath = p.join(Directory.current.path, 'bin', 'flutterfire.dart'); | ||
|
|
||
| // 1. Run "flutterfire configure" with custom output path | ||
| final result = Process.runSync( | ||
| 'dart', | ||
| [ | ||
| scriptPath, | ||
| 'configure', | ||
| '--yes', | ||
| '--platforms=android', | ||
| '--project=$firebaseProjectId', | ||
| '--firebase-out=$customFirebaseJsonPath', | ||
|
Comment on lines
+401
to
+412
Member
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. same again |
||
| ], | ||
| workingDirectory: projectPath, | ||
| runInShell: true, | ||
| ); | ||
|
|
||
| if (result.exitCode != 0) { | ||
| fail(result.stderr); | ||
| } | ||
|
|
||
| // Verify custom file exists | ||
| final customFile = File(p.join(projectPath!, 'custom', 'firebase.json')); | ||
| expect(customFile.existsSync(), true); | ||
|
|
||
| // Delete generated files to force reconfigure to do work | ||
| final firebaseOptionsPath = p.join(projectPath!, 'lib', 'firebase_options.dart'); | ||
| final androidServiceFilePath = p.join( | ||
| projectPath!, | ||
| 'android', | ||
| 'app', | ||
| androidServiceFileName, | ||
| ); | ||
|
|
||
| if (File(firebaseOptionsPath).existsSync()) { | ||
| await File(firebaseOptionsPath).delete(); | ||
| } | ||
| if (File(androidServiceFilePath).existsSync()) { | ||
| await File(androidServiceFilePath).delete(); | ||
| } | ||
|
|
||
| final accessToken = await generateAccessTokenCI(); | ||
|
|
||
| // 2. Run "flutterfire reconfigure" pointing to the custom path | ||
| final result2 = Process.runSync( | ||
| 'dart', | ||
| [ | ||
| scriptPath, | ||
|
Member
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. same again |
||
| 'reconfigure', | ||
| '--firebase-out=$customFirebaseJsonPath', | ||
| if (accessToken != null) '--ci-access-token=$accessToken', | ||
| ], | ||
| workingDirectory: projectPath, | ||
| runInShell: true, | ||
| ); | ||
|
|
||
| if (result2.exitCode != 0) { | ||
| fail(result2.stderr); | ||
| } | ||
|
|
||
| // Check the files have been recreated | ||
| expect(File(firebaseOptionsPath).existsSync(), true); | ||
| expect(File(androidServiceFilePath).existsSync(), true); | ||
| }, | ||
| timeout: const Timeout( | ||
| Duration(minutes: 2), | ||
| ), | ||
| ); | ||
| } | ||
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.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.
This might need further investigating.