Merge pull request #438 from Adam-Langley/fix/nondeterministic-depend…

…ency-ordering Change usage of 'Set' to 'List' to retain collection ordering
Milad-Akarie · Mar 18, 2024 · 566ce20 · 566ce20
2 parents b6f6cb4 + aca5015
commit 566ce20
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 13 deletions.
diff --git a/injectable_generator/lib/code_builder/builder_utils.dart b/injectable_generator/lib/code_builder/builder_utils.dart
@@ -92,18 +92,18 @@ extension _InjectedDependencyX on InjectedDependency {
 
 @visibleForTesting
 Set<DependencyConfig> sortDependencies(Iterable<DependencyConfig> it) {
-  // sort dependencies alphabetically
-  final deps = it.toList()..sortBy((e) => e.type.name);
+  // sort dependencies alphabetically by all the various attributes that may make them unique
+  final deps = it.toList()..sortBy<num>((e) => e.identityHash);
   // sort dependencies by their register order
-  final Set<DependencyConfig> sorted = {};
-  _sortByDependents(deps.toSet(), sorted);
+  final List<DependencyConfig> sorted = [];
+  _sortByDependents(deps, sorted);
   // sort dependencies by their orderPosition
   final s = sorted.sortedBy<num>((e) => e.orderPosition).toSet();
   return s;
 }
 
 void _sortByDependents(
-    Set<DependencyConfig> unSorted, Set<DependencyConfig> sorted) {
+    List<DependencyConfig> unSorted, List<DependencyConfig> sorted) {
   for (var dep in unSorted) {
     if (dep.dependencies.every(
       (iDep) {
@@ -125,20 +125,22 @@ void _sortByDependents(
     }
   }
   if (unSorted.isNotEmpty) {
-    _sortByDependents(unSorted.difference(sorted), sorted);
+    var difference = unSorted.where((element) => !sorted.contains(element)).toList();
+
+    _sortByDependents(difference, sorted);
   }
 }
 
 DependencyConfig? lookupDependency(
-    InjectedDependency iDep, Set<DependencyConfig> allDeps) {
+    InjectedDependency iDep, List<DependencyConfig> allDeps) {
   return allDeps.firstWhereOrNull(
     (d) => d.type == iDep.type && d.instanceName == iDep.instanceName,
   );
 }
 
 DependencyConfig? lookupDependencyWithNoEnvOrHasAny(
   InjectedDependency iDep,
-  Set<DependencyConfig> allDeps,
+  List<DependencyConfig> allDeps,
   List<String> envs,
 ) {
   return allDeps.firstWhereOrNull(

diff --git a/injectable_generator/lib/models/dependency_config.dart b/injectable_generator/lib/models/dependency_config.dart
@@ -32,7 +32,7 @@ class DependencyConfig {
   final int orderPosition;
   final String? scope;
 
-  const DependencyConfig({
+  DependencyConfig({
     required this.type,
     required this.typeImpl,
     this.injectableType = InjectableType.factory,
@@ -144,6 +144,16 @@ class DependencyConfig {
       postConstructReturnsSelf.hashCode ^
       scope.hashCode;
 
+ late final int identityHash = type.identity.hashCode ^
+     typeImpl.identity.hashCode ^
+     injectableType.hashCode ^
+     instanceName.hashCode ^
+     orderPosition.hashCode ^
+     scope.hashCode ^
+     const ListEquality().hash(dependencies) ^
+     const ListEquality().hash(dependsOn) ^
+     const ListEquality().hash(environments);
+
   factory DependencyConfig.fromJson(Map<dynamic, dynamic> json) {
     ModuleConfig? moduleConfig;
     DisposeFunctionConfig? disposeFunction;

diff --git a/injectable_generator/test/code_builder/builder_utils_test.dart b/injectable_generator/test/code_builder/builder_utils_test.dart
@@ -382,7 +382,7 @@ void main() {
         type: ImportableType(name: 'Fizz'),
         paramName: 'fizz',
       );
-      final allDeps = <DependencyConfig>{};
+      final allDeps = <DependencyConfig>[];
       expect(lookupDependency(iDep, allDeps), isNull);
     });
 
@@ -396,7 +396,7 @@ void main() {
         typeImpl: ImportableType(name: 'Fizz'),
         injectableType: InjectableType.factory,
       );
-      final allDeps = {dep};
+      final allDeps = [dep];
       expect(lookupDependency(iDep, allDeps), same(dep));
     });
 
@@ -412,7 +412,7 @@ void main() {
         injectableType: InjectableType.factory,
         instanceName: 'fizzBuzz',
       );
-      final allDeps = {dep};
+      final allDeps = [dep];
       expect(lookupDependency(iDep, allDeps), isNull);
     });
 
@@ -428,7 +428,7 @@ void main() {
         injectableType: InjectableType.factory,
         instanceName: 'fizzImpl',
       );
-      final allDeps = {dep};
+      final allDeps = [dep];
       expect(lookupDependency(iDep, allDeps), same(dep));
     });
   });