refactor: improve pubspec_overrides handling

Author: felangel

bricks/dart_frog_prod_server/hooks/lib/src/create_external_packages_folder.dart

@@ -67,7 +67,7 @@ Future<List<String>> createExternalPackagesFolder({
     pathResolver.join(buildDirectory.path, 'pubspec_overrides.yaml'),
   ).writeAsStringSync(
     '''
-
+resolution: null
 dependency_overrides:
 ${copiedExternalPathDependencies.map(
       (dependency) {
@@ -78,7 +78,6 @@ ${copiedExternalPathDependencies.map(
       },
     ).join('\n')}
 ''',
-    mode: FileMode.append,
   );
 
   return copiedExternalPathDependencies

bricks/dart_frog_prod_server/hooks/lib/src/disable_workspace_resolution.dart

@@ -1,35 +1,46 @@
 import 'dart:io';
 import 'package:mason/mason.dart';
 import 'package:path/path.dart' as path;
+import 'package:yaml/yaml.dart';
+
+/// A void callback function (e.g. `void Function()`).
+typedef VoidCallback = void Function();
 
 /// Opts out of dart workspaces until we can generate per package lockfiles.
 /// https://github.com/dart-lang/pub/issues/4594
-void disableWorkspaceResolution(
+VoidCallback disableWorkspaceResolution(
   HookContext context, {
   required String projectDirectory,
   required void Function(int exitCode) exit,
 }) {
   try {
-    overrideResolutionInPubspecOverrides(projectDirectory);
+    return overrideResolutionInPubspecOverrides(projectDirectory);
   } on Exception catch (e) {
     context.logger.err('$e');
     exit(1);
+    return () {};
   }
 }
 
-void overrideResolutionInPubspecOverrides(String projectDirectory) {
-  final pubspecOverrides = File(
+void Function() overrideResolutionInPubspecOverrides(String projectDirectory) {
+  final pubspecOverridesFile = File(
     path.join(projectDirectory, 'pubspec_overrides.yaml'),
   );
 
-  if (pubspecOverrides.existsSync()) {
-    return pubspecOverrides.writeAsStringSync(
-      '\nresolution: null\n',
-      mode: FileMode.append,
-    );
+  if (!pubspecOverridesFile.existsSync()) {
+    pubspecOverridesFile.writeAsStringSync('resolution: null');
+    return pubspecOverridesFile.deleteSync;
   }
 
-  pubspecOverrides
-    ..createSync(recursive: true)
-    ..writeAsStringSync('resolution: null');
+  final contents = pubspecOverridesFile.readAsStringSync();
+  final pubspecOverrides = loadYaml(contents) as YamlMap?;
+
+  if (pubspecOverrides?['resolution'] == 'null') return () {};
+  pubspecOverridesFile.writeAsStringSync(
+    '''
+resolution: null
+$contents''',
+  );
+
+  return () => pubspecOverridesFile.writeAsStringSync(contents);
 }

bricks/dart_frog_prod_server/hooks/lib/src/uses_workspace_resolution.dart

@@ -1,7 +1,7 @@
 import 'dart:io';
 import 'package:mason/mason.dart';
 import 'package:path/path.dart' as path;
-import 'package:pubspec_parse/pubspec_parse.dart';
+import 'package:yaml/yaml.dart';
 
 /// Determines whether the project in the provided [workingDirectory]
 /// is configured to use `resolution: workspace`.
@@ -13,14 +13,18 @@ bool usesWorkspaceResolution(
   final pubspecFile = File(path.join(workingDirectory, 'pubspec.yaml'));
   if (!pubspecFile.existsSync()) return false;
 
-  final Pubspec pubspec;
+  final YamlMap pubspec;
   try {
-    pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
+    final yaml = loadYaml(pubspecFile.readAsStringSync());
+    if (yaml is! YamlMap) {
+      throw Exception('Unable to parse ${pubspecFile.path}');
+    }
+    pubspec = yaml;
   } on Exception catch (e) {
     context.logger.err('$e');
     exit(1);
     return false;
   }
 
-  return pubspec.resolution == 'workspace';
+  return pubspec['resolution'] == 'workspace';
 }

bricks/dart_frog_prod_server/hooks/pre_gen.dart

@@ -29,10 +29,12 @@ Future<void> preGen(
     exit: exit,
   );
 
+  VoidCallback? restoreWorkspaceResolution;
+
   if (usesWorkspaces) {
     // Disable workspace resolution until we can generate per-package lockfiles.
     // https://github.com/dart-lang/pub/issues/4594
-    disableWorkspaceResolution(
+    restoreWorkspaceResolution = disableWorkspaceResolution(
       context,
       projectDirectory: projectDirectory.path,
       exit: exit,
@@ -58,6 +60,8 @@ Future<void> preGen(
     exit: exit,
   );
 
+  restoreWorkspaceResolution?.call();
+
   final RouteConfiguration configuration;
   try {
     configuration = buildConfiguration(projectDirectory);

bricks/dart_frog_prod_server/hooks/pubspec.yaml

@@ -10,7 +10,6 @@ dependencies:
   io: ^1.0.3
   mason: ^0.1.0
   path: ^1.8.1
-  pubspec_parse: ^1.5.0
   yaml: ^3.1.2
 
 dev_dependencies:

bricks/dart_frog_prod_server/hooks/test/pre_gen_test.dart

@@ -230,7 +230,11 @@ packages:
         exit: exitCalls.add,
         directory: directory,
         runProcess: successRunProcess,
-        copyPath: (_, __) async {},
+        copyPath: (from, to) async {
+          File(
+            path.join(to, 'pubspec_overrides.yaml'),
+          ).createSync(recursive: true);
+        },
       );
 
       expect(exitCalls, isEmpty);

bricks/dart_frog_prod_server/hooks/test/src/create_external_packages_folder_test.dart

@@ -17,9 +17,11 @@ void main() {
       await createExternalPackagesFolder(
         projectDirectory: projectDirectory,
         buildDirectory: Directory(path.join(projectDirectory.path, 'build')),
-        copyPath: (from, to) {
+        copyPath: (from, to) async {
           copyCalls.add('$from -> $to');
-          return Future.value();
+          File(
+            path.join(to, 'pubspec_overrides.yaml'),
+          ).createSync(recursive: true);
         },
       );
 
@@ -56,9 +58,11 @@ void main() {
       await createExternalPackagesFolder(
         projectDirectory: projectDirectory,
         buildDirectory: Directory(path.join(projectDirectory.path, 'build')),
-        copyPath: (from, to) {
+        copyPath: (from, to) async {
           copyCalls.add('$from -> $to');
-          return Future.value();
+          File(
+            path.join(to, 'pubspec_overrides.yaml'),
+          ).createSync(recursive: true);
         },
       );
 

bricks/dart_frog_prod_server/hooks/test/src/disable_workspace_resolution_test.dart

@@ -70,9 +70,8 @@ dependency_overrides:
           pubspecOverrides.readAsStringSync(),
           equals(
             '''
-$originalPubspecOverridesContent
 resolution: null
-''',
+$originalPubspecOverridesContent''',
           ),
         );
       });

bricks/dart_frog_prod_server/hooks/test/src/uses_workspace_resolution_test.dart

@@ -47,10 +47,11 @@ void main() {
     });
 
     group('when pubspec.yaml is malformed', () {
+      late File pubspecFile;
       setUp(() {
-        File(
+        pubspecFile = File(
           path.join(workingDirectory.path, 'pubspec.yaml'),
-        ).writeAsStringSync('invalid pubspec.yaml');
+        )..writeAsStringSync('invalid pubspec.yaml');
       });
 
       test('returns false', () {
@@ -63,7 +64,11 @@ void main() {
           isFalse,
         );
         expect(exitCalls, equals([1]));
-        verify(() => logger.err(any(that: contains('ParsedYamlException'))));
+        verify(
+          () => logger.err(
+            any(that: contains('Unable to parse ${pubspecFile.path}')),
+          ),
+        );
       });
     });