From 4d1dd1a3f6b2a29cfbaba8cc08d76a3040b41a2f Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Fri, 19 May 2023 16:14:15 +0800 Subject: [PATCH 1/5] revamp: Accept required parameters not in path --- .../lib/src/route_config.dart | 14 ++---- .../lib/src/type_helpers.dart | 46 ++++++++++--------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/packages/go_router_builder/lib/src/route_config.dart b/packages/go_router_builder/lib/src/route_config.dart index a67f33bab05..3e53cb1c427 100644 --- a/packages/go_router_builder/lib/src/route_config.dart +++ b/packages/go_router_builder/lib/src/route_config.dart @@ -369,21 +369,15 @@ GoRouteData.\$route( String _decodeFor(ParameterElement element) { if (element.isRequired) { - if (element.type.nullabilitySuffix == NullabilitySuffix.question) { + if (element.type.nullabilitySuffix == NullabilitySuffix.question && + _pathParams.contains(element.name)) { throw InvalidGenerationSourceError( - 'Required parameters cannot be nullable.', - element: element, - ); - } - - if (!_pathParams.contains(element.name) && !element.isExtraField) { - throw InvalidGenerationSourceError( - 'Missing param `${element.name}` in path.', + 'Required parameters in the path cannot be nullable.', element: element, ); } } - final String fromStateExpression = decodeParameter(element); + final String fromStateExpression = decodeParameter(element, _pathParams); if (element.isPositional) { return '$fromStateExpression,'; diff --git a/packages/go_router_builder/lib/src/type_helpers.dart b/packages/go_router_builder/lib/src/type_helpers.dart index 1ed85072465..908b56d7a22 100644 --- a/packages/go_router_builder/lib/src/type_helpers.dart +++ b/packages/go_router_builder/lib/src/type_helpers.dart @@ -44,15 +44,15 @@ const List<_TypeHelper> _helpers = <_TypeHelper>[ /// Returns the decoded [String] value for [element], if its type is supported. /// /// Otherwise, throws an [InvalidGenerationSourceError]. -String decodeParameter(ParameterElement element) { +String decodeParameter(ParameterElement element, Set pathParameters) { if (element.isExtraField) { - return 'state.${_stateValueAccess(element)}'; + return 'state.${_stateValueAccess(element, pathParameters)}'; } final DartType paramType = element.type; for (final _TypeHelper helper in _helpers) { if (helper._matchesType(paramType)) { - String decoded = helper._decode(element); + String decoded = helper._decode(element, pathParameters); if (element.isOptional && element.hasDefaultValue) { if (element.type.isNullableType) { throw NullableDefaultValueError(element); @@ -92,30 +92,29 @@ String encodeField(PropertyAccessorElement element) { // ignore: deprecated_member_use String enumMapName(InterfaceType type) => '_\$${type.element.name}EnumMap'; -String _stateValueAccess(ParameterElement element) { +String _stateValueAccess(ParameterElement element, Set pathParameters) { if (element.isExtraField) { return 'extra as ${element.type.getDisplayString(withNullability: element.isOptional)}'; } - if (element.isRequired) { - return 'pathParameters[${escapeDartString(element.name)}]!'; + late String access; + if (pathParameters.contains(element.name)) { + access = 'pathParameters[${escapeDartString(element.name)}]'; + } else { + access = 'queryParameters[${escapeDartString(element.name.kebab)}]'; } - - if (element.isOptional) { - return 'queryParameters[${escapeDartString(element.name.kebab)}]'; + if (!element.type.isNullableType) { + access += '!'; } - throw InvalidGenerationSourceError( - '$likelyIssueMessage (param not required or optional)', - element: element, - ); + return access; } abstract class _TypeHelper { const _TypeHelper(); /// Decodes the value from its string representation in the URL. - String _decode(ParameterElement parameterElement); + String _decode(ParameterElement parameterElement, Set pathParameters); /// Encodes the value from its string representation in the URL. String _encode(String fieldName, DartType type); @@ -228,8 +227,9 @@ class _TypeHelperString extends _TypeHelper { const _TypeHelperString(); @override - String _decode(ParameterElement parameterElement) => - 'state.${_stateValueAccess(parameterElement)}'; + String _decode( + ParameterElement parameterElement, Set pathParameters) => + 'state.${_stateValueAccess(parameterElement, pathParameters)}'; @override String _encode(String fieldName, DartType type) => fieldName; @@ -257,7 +257,8 @@ class _TypeHelperIterable extends _TypeHelper { const _TypeHelperIterable(); @override - String _decode(ParameterElement parameterElement) { + String _decode( + ParameterElement parameterElement, Set pathParameters) { if (parameterElement.type is ParameterizedType) { final DartType iterableType = (parameterElement.type as ParameterizedType).typeArguments.first; @@ -323,17 +324,20 @@ abstract class _TypeHelperWithHelper extends _TypeHelper { String helperName(DartType paramType); @override - String _decode(ParameterElement parameterElement) { + String _decode( + ParameterElement parameterElement, Set pathParameters) { final DartType paramType = parameterElement.type; + final String parameterName = parameterElement.name; - if (!parameterElement.isRequired) { + if (!pathParameters.contains(parameterName) && + (paramType.isNullableType || parameterElement.hasDefaultValue)) { return '$convertMapValueHelperName(' - '${escapeDartString(parameterElement.name.kebab)}, ' + '${escapeDartString(parameterName.kebab)}, ' 'state.queryParameters, ' '${helperName(paramType)})'; } return '${helperName(paramType)}' - '(state.${_stateValueAccess(parameterElement)})'; + '(state.${_stateValueAccess(parameterElement, pathParameters)})'; } } From 38745b0b59626303eae6a101de324f058e103710 Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Fri, 19 May 2023 16:14:58 +0800 Subject: [PATCH 2/5] build: Regenerate the code --- packages/go_router_builder/example/lib/all_types.g.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router_builder/example/lib/all_types.g.dart b/packages/go_router_builder/example/lib/all_types.g.dart index 8f3add393eb..36213697357 100644 --- a/packages/go_router_builder/example/lib/all_types.g.dart +++ b/packages/go_router_builder/example/lib/all_types.g.dart @@ -323,7 +323,7 @@ extension $StringRouteExtension on StringRoute { requiredStringField: state.pathParameters['requiredStringField']!, stringField: state.queryParameters['string-field'], stringFieldWithDefaultValue: - state.queryParameters['string-field-with-default-value'] ?? + state.queryParameters['string-field-with-default-value']! ?? 'defaultValue', ); From b8d3d164397324b754d55130aaef05df463996ea Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Fri, 19 May 2023 16:15:12 +0800 Subject: [PATCH 3/5] test: Update the tests --- .../go_router_builder/test/builder_test.dart | 5 +- .../_go_router_builder_test_input.dart | 84 ++++++++++++++++--- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/packages/go_router_builder/test/builder_test.dart b/packages/go_router_builder/test/builder_test.dart index 056d854a4c0..53df1f0cf88 100644 --- a/packages/go_router_builder/test/builder_test.dart +++ b/packages/go_router_builder/test/builder_test.dart @@ -26,10 +26,11 @@ const Set _expectedAnnotatedTests = { 'BadPathParam', 'ExtraValueRoute', 'RequiredExtraValueRoute', - 'MissingPathParam', 'MissingPathValue', 'MissingTypeAnnotation', - 'NullableRequiredParam', + 'NullableRequiredParamInPath', + 'NullableRequiredParamNotInPath', + 'NonNullableRequiredParamNotInPath', 'UnsupportedType', 'theAnswer', 'EnumParam', diff --git a/packages/go_router_builder/test/test_inputs/_go_router_builder_test_input.dart b/packages/go_router_builder/test/test_inputs/_go_router_builder_test_input.dart index 678a5286e08..016d2c86518 100644 --- a/packages/go_router_builder/test/test_inputs/_go_router_builder_test_input.dart +++ b/packages/go_router_builder/test/test_inputs/_go_router_builder_test_input.dart @@ -45,21 +45,83 @@ class UnsupportedType extends GoRouteData { } @ShouldThrow( - 'Required parameters cannot be nullable.', + 'Required parameters in the path cannot be nullable.', ) -@TypedGoRoute(path: 'bob/:id') -class NullableRequiredParam extends GoRouteData { - NullableRequiredParam({required this.id}); +@TypedGoRoute(path: 'bob/:id') +class NullableRequiredParamInPath extends GoRouteData { + NullableRequiredParamInPath({required this.id}); final int? id; } -@ShouldThrow( - 'Missing param `id` in path.', -) -@TypedGoRoute(path: 'bob/') -class MissingPathParam extends GoRouteData { - MissingPathParam({required this.id}); - final String id; +@ShouldGenerate(r''' +RouteBase get $nullableRequiredParamNotInPath => GoRouteData.$route( + path: 'bob', + factory: $NullableRequiredParamNotInPathExtension._fromState, + ); + +extension $NullableRequiredParamNotInPathExtension + on NullableRequiredParamNotInPath { + static NullableRequiredParamNotInPath _fromState(GoRouterState state) => + NullableRequiredParamNotInPath( + id: _$convertMapValue('id', state.queryParameters, int.parse), + ); + + String get location => GoRouteData.$location( + 'bob', + ); + + void go(BuildContext context) => context.go(location); + + Future push(BuildContext context) => context.push(location); + + void pushReplacement(BuildContext context) => + context.pushReplacement(location); +} + +T? _$convertMapValue( + String key, + Map map, + T Function(String) converter, +) { + final value = map[key]; + return value == null ? null : converter(value); +} +''') +@TypedGoRoute(path: 'bob') +class NullableRequiredParamNotInPath extends GoRouteData { + NullableRequiredParamNotInPath({required this.id}); + final int? id; +} + +@ShouldGenerate(r''' +RouteBase get $nonNullableRequiredParamNotInPath => GoRouteData.$route( + path: 'bob', + factory: $NonNullableRequiredParamNotInPathExtension._fromState, + ); + +extension $NonNullableRequiredParamNotInPathExtension + on NonNullableRequiredParamNotInPath { + static NonNullableRequiredParamNotInPath _fromState(GoRouterState state) => + NonNullableRequiredParamNotInPath( + id: int.parse(state.queryParameters['id']!), + ); + + String get location => GoRouteData.$location( + 'bob', + ); + + void go(BuildContext context) => context.go(location); + + Future push(BuildContext context) => context.push(location); + + void pushReplacement(BuildContext context) => + context.pushReplacement(location); +} +''') +@TypedGoRoute(path: 'bob') +class NonNullableRequiredParamNotInPath extends GoRouteData { + NonNullableRequiredParamNotInPath({required this.id}); + final int id; } @ShouldGenerate(r''' From 0cad3916fbf44f3000429ca2028440581829eb68 Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Fri, 19 May 2023 16:22:55 +0800 Subject: [PATCH 4/5] doc: Update documentation --- packages/go_router_builder/CHANGELOG.md | 3 ++- packages/go_router_builder/README.md | 8 ++++---- packages/go_router_builder/pubspec.yaml | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/go_router_builder/CHANGELOG.md b/packages/go_router_builder/CHANGELOG.md index bdab45ea957..697de341899 100644 --- a/packages/go_router_builder/CHANGELOG.md +++ b/packages/go_router_builder/CHANGELOG.md @@ -1,6 +1,7 @@ -## NEXT +## 2.1.0 * Updates minimum supported SDK version to Flutter 3.3/Dart 2.18. +* Supports required/positional parameters that are not in the path. ## 2.0.1 diff --git a/packages/go_router_builder/README.md b/packages/go_router_builder/README.md index 18a4ea0c941..0eff1f07c6a 100644 --- a/packages/go_router_builder/README.md +++ b/packages/go_router_builder/README.md @@ -89,9 +89,6 @@ class HomeRoute extends GoRouteData { } ``` -Required parameters are pulled from the route's `path` defined in the route -tree. - ## Route tree The tree of routes is defined as an attribute on each of the top-level routes: @@ -178,9 +175,10 @@ void _tap() async { ## Query parameters -Optional parameters (named or positional) indicate query parameters: +Parameters (named or positional) not listed in the path of `TypedGoRoute` indicate query parameters: ```dart +@TypedGoRoute(path: '/login') class LoginRoute extends GoRouteData { LoginRoute({this.from}); final String? from; @@ -195,6 +193,7 @@ class LoginRoute extends GoRouteData { For query parameters with a **non-nullable** type, you can define a default value: ```dart +@TypedGoRoute(path: '/my-route') class MyRoute extends GoRouteData { MyRoute({this.queryParameter = 'defaultValue'}); final String queryParameter; @@ -237,6 +236,7 @@ recommended when targeting Flutter web. You can, of course, combine the use of path, query and $extra parameters: ```dart +@TypedGoRoute(path: '/:ketchup') class HotdogRouteWithEverything extends GoRouteData { HotdogRouteWithEverything(this.ketchup, this.mustard, this.$extra); final bool ketchup; // required path parameter diff --git a/packages/go_router_builder/pubspec.yaml b/packages/go_router_builder/pubspec.yaml index a8ae680e97a..027b79ea2c6 100644 --- a/packages/go_router_builder/pubspec.yaml +++ b/packages/go_router_builder/pubspec.yaml @@ -2,7 +2,7 @@ name: go_router_builder description: >- A builder that supports generated strongly-typed route helpers for package:go_router -version: 2.0.1 +version: 2.1.0 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 From 65dec727bb543148144a23cd563c027ee191a90a Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Mon, 22 May 2023 09:52:44 +0800 Subject: [PATCH 5/5] :bug: Don't use null check operator when there is a default value --- packages/go_router_builder/example/lib/all_types.g.dart | 2 +- packages/go_router_builder/lib/src/type_helpers.dart | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/go_router_builder/example/lib/all_types.g.dart b/packages/go_router_builder/example/lib/all_types.g.dart index 36213697357..8f3add393eb 100644 --- a/packages/go_router_builder/example/lib/all_types.g.dart +++ b/packages/go_router_builder/example/lib/all_types.g.dart @@ -323,7 +323,7 @@ extension $StringRouteExtension on StringRoute { requiredStringField: state.pathParameters['requiredStringField']!, stringField: state.queryParameters['string-field'], stringFieldWithDefaultValue: - state.queryParameters['string-field-with-default-value']! ?? + state.queryParameters['string-field-with-default-value'] ?? 'defaultValue', ); diff --git a/packages/go_router_builder/lib/src/type_helpers.dart b/packages/go_router_builder/lib/src/type_helpers.dart index 908b56d7a22..cea332a0001 100644 --- a/packages/go_router_builder/lib/src/type_helpers.dart +++ b/packages/go_router_builder/lib/src/type_helpers.dart @@ -103,7 +103,8 @@ String _stateValueAccess(ParameterElement element, Set pathParameters) { } else { access = 'queryParameters[${escapeDartString(element.name.kebab)}]'; } - if (!element.type.isNullableType) { + if (pathParameters.contains(element.name) || + (!element.type.isNullableType && !element.hasDefaultValue)) { access += '!'; }