Skip to content

Commit

Permalink
[go_router_builder] Fixes trailing ? by comparing iterables (#8521)
Browse files Browse the repository at this point in the history
  • Loading branch information
ValentinVignal authored Jan 29, 2025
1 parent ef7e0d5 commit dc57079
Show file tree
Hide file tree
Showing 9 changed files with 181 additions and 23 deletions.
4 changes: 4 additions & 0 deletions packages/go_router_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.7.5

- Fixes trailing `?` in the location when a go route has an empty default value.

## 2.7.4

- Fixes an issue by removing unnecessary `const` in StatefulShellRouteData generation.
Expand Down
51 changes: 34 additions & 17 deletions packages/go_router_builder/example/lib/all_types.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 49 additions & 0 deletions packages/go_router_builder/example/test/location_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter_test/flutter_test.dart';
import 'package:go_router_builder_example/all_types.dart';

void main() {
test('IterableRouteWithDefaultValues', () {
expect(
const IterableRouteWithDefaultValues().location,
'/iterable-route-with-default-values',
);

// Needs to not be a const to test
// https://github.com/flutter/flutter/issues/127825.
final Set<double> doubleSetField = <double>{};
expect(
IterableRouteWithDefaultValues(
doubleSetField: doubleSetField,
).location,
'/iterable-route-with-default-values',
);

expect(
IterableRouteWithDefaultValues(
doubleSetField: <double>{0.0, 1.0},
).location,
'/iterable-route-with-default-values?double-set-field=0.0&double-set-field=1.0',
);

// Needs to not be a const to test
// https://github.com/flutter/flutter/issues/127825.
final Set<int> intSetField = <int>{0, 1};
expect(
IterableRouteWithDefaultValues(
intSetField: intSetField,
).location,
'/iterable-route-with-default-values',
);

expect(
const IterableRouteWithDefaultValues(
intSetField: <int>{0, 1, 2},
).location,
'/iterable-route-with-default-values?int-set-field=0&int-set-field=1&int-set-field=2',
);
});
}
20 changes: 19 additions & 1 deletion packages/go_router_builder/lib/src/route_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,9 @@ class GoRouteConfig extends RouteBaseConfig {
if (param.type.isNullableType) {
throw NullableDefaultValueError(param);
}
conditions.add('$parameterName != ${param.defaultValueCode!}');
conditions.add(
compareField(param, parameterName, param.defaultValueCode!),
);
} else if (param.type.isNullableType) {
conditions.add('$parameterName != null');
}
Expand Down Expand Up @@ -734,6 +736,7 @@ const Map<String, String> helperNames = <String, String>{
convertMapValueHelperName: _convertMapValueHelper,
boolConverterHelperName: _boolConverterHelper,
enumExtensionHelperName: _enumConverterHelper,
iterablesEqualHelperName: _iterableEqualsHelper,
};

const String _convertMapValueHelper = '''
Expand Down Expand Up @@ -765,3 +768,18 @@ extension<T extends Enum> on Map<T, String> {
T $enumExtensionHelperName(String value) =>
entries.singleWhere((element) => element.value == value).key;
}''';

const String _iterableEqualsHelper = '''
bool $iterablesEqualHelperName<T>(Iterable<T>? iterable1, Iterable<T>? iterable2) {
if (identical(iterable1, iterable2)) return true;
if (iterable1 == null || iterable2 == null) return false;
final iterator1 = iterable1.iterator;
final iterator2 = iterable2.iterator;
while (true) {
final hasNext1 = iterator1.moveNext();
final hasNext2 = iterator2.moveNext();
if (hasNext1 != hasNext2) return false;
if (!hasNext1) return true;
if (iterator1.current != iterator2.current) return false;
}
}''';
30 changes: 29 additions & 1 deletion packages/go_router_builder/lib/src/type_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const String extraFieldName = r'$extra';
/// Shared start of error message related to a likely code issue.
const String likelyIssueMessage = 'Should never get here! File an issue!';

/// The name of the generated, private helper for comparing iterables.
const String iterablesEqualHelperName = r'_$iterablesEqual';

const List<_TypeHelper> _helpers = <_TypeHelper>[
_TypeHelperBigInt(),
_TypeHelperBool(),
Expand Down Expand Up @@ -86,6 +89,22 @@ String encodeField(PropertyAccessorElement element) {
);
}

/// Returns the comparison of a parameter with its default value.
///
/// Otherwise, throws an [InvalidGenerationSourceError].
String compareField(ParameterElement param, String value1, String value2) {
for (final _TypeHelper helper in _helpers) {
if (helper._matchesType(param.type)) {
return helper._compare(param.name, param.defaultValueCode!);
}
}

throw InvalidGenerationSourceError(
'The type `${param.type}` is not supported.',
element: param,
);
}

/// Gets the name of the `const` map generated to help encode [Enum] types.
String enumMapName(InterfaceType type) => '_\$${type.element.name}EnumMap';

Expand Down Expand Up @@ -119,6 +138,8 @@ abstract class _TypeHelper {
String _encode(String fieldName, DartType type);

bool _matchesType(DartType type);

String _compare(String value1, String value2) => '$value1 != $value2';
}

class _TypeHelperBigInt extends _TypeHelperWithHelper {
Expand Down Expand Up @@ -252,9 +273,12 @@ class _TypeHelperUri extends _TypeHelperWithHelper {
const TypeChecker.fromRuntime(Uri).isAssignableFromType(type);
}

class _TypeHelperIterable extends _TypeHelper {
class _TypeHelperIterable extends _TypeHelperWithHelper {
const _TypeHelperIterable();

@override
String helperName(DartType paramType) => iterablesEqualHelperName;

@override
String _decode(
ParameterElement parameterElement, Set<String> pathParameters) {
Expand Down Expand Up @@ -324,6 +348,10 @@ $fieldName$nullAwareAccess.map((e) => e.toString()).toList()''';
@override
bool _matchesType(DartType type) =>
const TypeChecker.fromRuntime(Iterable).isAssignableFromType(type);

@override
String _compare(String value1, String value2) =>
'!$iterablesEqualHelperName($value1, $value2)';
}

abstract class _TypeHelperWithHelper extends _TypeHelper {
Expand Down
2 changes: 1 addition & 1 deletion packages/go_router_builder/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: go_router_builder
description: >-
A builder that supports generated strongly-typed route helpers for
package:go_router
version: 2.7.4
version: 2.7.5
repository: https://github.com/flutter/packages/tree/main/packages/go_router_builder
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router_builder%22

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ extension $IterableDefaultValueRouteExtension on IterableDefaultValueRoute {
String get location => GoRouteData.$location(
'/iterable-default-value-route',
queryParams: {
if (param != const <int>[0])
if (!_$iterablesEqual(param, const <int>[0]))
'param': param.map((e) => e.toString()).toList(),
},
);
Expand All @@ -27,3 +27,17 @@ extension $IterableDefaultValueRouteExtension on IterableDefaultValueRoute {

void replace(BuildContext context) => context.replace(location);
}

bool _$iterablesEqual<T>(Iterable<T>? iterable1, Iterable<T>? iterable2) {
if (identical(iterable1, iterable2)) return true;
if (iterable1 == null || iterable2 == null) return false;
final iterator1 = iterable1.iterator;
final iterator2 = iterable2.iterator;
while (true) {
final hasNext1 = iterator1.moveNext();
final hasNext2 = iterator2.moveNext();
if (hasNext1 != hasNext2) return false;
if (!hasNext1) return true;
if (iterator1.current != iterator2.current) return false;
}
}
16 changes: 15 additions & 1 deletion packages/go_router_builder/test_inputs/list.dart.expect
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extension $ListRouteExtension on ListRoute {
'ids': ids.map((e) => e.toString()).toList(),
if (nullableIds != null)
'nullable-ids': nullableIds?.map((e) => e.toString()).toList(),
if (idsWithDefaultValue != const <int>[0])
if (!_$iterablesEqual(idsWithDefaultValue, const <int>[0]))
'ids-with-default-value':
idsWithDefaultValue.map((e) => e.toString()).toList(),
},
Expand All @@ -38,3 +38,17 @@ extension $ListRouteExtension on ListRoute {

void replace(BuildContext context) => context.replace(location);
}

bool _$iterablesEqual<T>(Iterable<T>? iterable1, Iterable<T>? iterable2) {
if (identical(iterable1, iterable2)) return true;
if (iterable1 == null || iterable2 == null) return false;
final iterator1 = iterable1.iterator;
final iterator2 = iterable2.iterator;
while (true) {
final hasNext1 = iterator1.moveNext();
final hasNext2 = iterator2.moveNext();
if (hasNext1 != hasNext2) return false;
if (!hasNext1) return true;
if (iterator1.current != iterator2.current) return false;
}
}
16 changes: 15 additions & 1 deletion packages/go_router_builder/test_inputs/set.dart.expect
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extension $SetRouteExtension on SetRoute {
'ids': ids.map((e) => e.toString()).toList(),
if (nullableIds != null)
'nullable-ids': nullableIds?.map((e) => e.toString()).toList(),
if (idsWithDefaultValue != const <int>{0})
if (!_$iterablesEqual(idsWithDefaultValue, const <int>{0}))
'ids-with-default-value':
idsWithDefaultValue.map((e) => e.toString()).toList(),
},
Expand All @@ -38,3 +38,17 @@ extension $SetRouteExtension on SetRoute {

void replace(BuildContext context) => context.replace(location);
}

bool _$iterablesEqual<T>(Iterable<T>? iterable1, Iterable<T>? iterable2) {
if (identical(iterable1, iterable2)) return true;
if (iterable1 == null || iterable2 == null) return false;
final iterator1 = iterable1.iterator;
final iterator2 = iterable2.iterator;
while (true) {
final hasNext1 = iterator1.moveNext();
final hasNext2 = iterator2.moveNext();
if (hasNext1 != hasNext2) return false;
if (!hasNext1) return true;
if (iterator1.current != iterator2.current) return false;
}
}

0 comments on commit dc57079

Please sign in to comment.