[go_router_builder] Add support for Iterable, List and Set to TypedGoRoute #2679
Conversation
There was a problem hiding this comment.
This seems to be incorrect?
There was a problem hiding this comment.
Seems a bit weird to hardcoded the query parameter name, I think this may make it less useful if a company care about the query parameter names. Would it make more sense to fix flutter/flutter#110781 and provide an API that people can set it up for iterable object with minimum code?
There was a problem hiding this comment.
this seems not addressed yet?
|
@chunhtai is there something else I can do to make this PR advance? I'm currently using a workaround to pass lists of query params and I'd really appreciate to be able to use normal lists as we can do without typed navigation. |
chunhtai
left a comment
chunhtai
left a comment
There was a problem hiding this comment.
I think we should provide a more generic solution than harding the query parameter names
There was a problem hiding this comment.
this seems not addressed yet?
|
@chunhtai I answered to your last comment in packages/go_router_builder/example/test/all_types_test.dart, could you please give more clarification so I can address it and finalise this PR? |
There was a problem hiding this comment.
Would be good to modulalize this to be a class. I imagine someone would like to do this
class IterableRoute extends GoRouteData {
IterableRoute({
this.myField
})
final MyCustomField? myField;
}
abstract class CustomField<T> {
Iterable<T> value;
bool get isQueryParameter;
String get queryParameterName;
}
class MyCustomField extend CustomField<List<int>> {
MyCustomField();
@override
bool get isQueryParameter => true;
@override
String get queryParameterName => 'some-name' ;
}
The idea is to be able to choose the query parameter name.
There was a problem hiding this comment.
Hi @Skogsfrae WDYT about this? The main concern is the query parameter names are hardcoded which I imagine people would ask for customized query parameter names in the future. Although this is not something we have to fix now, but the current API, as it is written, may not be able the extend to support this feature.
|
please fix the ci, and this needs to wait for #2698 to be merged first |
Skogsfrae
force-pushed
the
feature/#108437_support-for-iterables
branch
from
8272974 to
a4a6772
Compare
Skogsfrae
force-pushed
the
feature/#108437_support-for-iterables
branch
from
a4a6772 to
c0adb7b
Compare
|
@Skogsfrae this should be ready to land since the other pr is merged |
Skogsfrae
force-pushed
the
feature/#108437_support-for-iterables
branch
from
d8de275 to
8d6cc1b
Compare
|
@chunhtai I rebased my branch to upstream and now it's ready to go 🚀 |
Skogsfrae
force-pushed
the
feature/#108437_support-for-iterables
branch
from
8d6cc1b to
1d74193
Compare
chunhtai
added
the
autosubmit
* ad48ee5 [go_router] Fix some broken links in doc (flutter/packages#3288) * 58ac45e [go_router_builder] Add support for Iterable, List and Set to TypedGoRoute (flutter/packages#2679) * 0edae25 Export super types in route_data.dart library (flutter/packages#3286) * 8ad3fde [go_router] Add `GoRouter.maybeOf` (flutter/packages#3216) * 13ee644 [flutter_migrate] Skip slow tests (flutter/packages#3270) * 3cc754a Update .gitignore with missing values from flutter/plugins (flutter/packages#3265) * c0f0a22 [ci] Re-enable pathified unit tests (flutter/packages#3268) * 5834b4c [go_router]: implemented helpers for ShellRoute (flutter/packages#2730) * af5906b [extension_gsi] Update extension to support gsi 5 and 6. (flutter/packages#3235) * 195f4e8 Merge in plugin README and CONTRIBUTING (flutter/packages#3252) * fab47af [go_router] Disable logging in tests (flutter/packages#3263) * 25f0f70 [various] Update flutter/plugins links (flutter/packages#3256) * 2e16733 Merge flutter/plugins (flutter/packages#3233) * 324a7f2 Exclude more tests on Windows * 334b58e Adjust test configs * 69e6dac [go_router_builder] Generate replace method in RouteExtension (flutter/packages#2838) * 193e454 Merge repository metadata * 18715d7 Merge remote-tracking branch 'plugins-packages/main' into merge-flutter-plugins * f2d802d Roll Flutter from ae8d051 to 7175de4 (4 revisions) (flutter/packages#3232) * 6f1b1e8 Roll Flutter from 33e4d21 to ae8d051 (6 revisions) (flutter/packages#3229) * a162a98 Roll Flutter from 0be7c3f to 33e4d21 (5 revisions) (flutter/packages#3227) * ab5a8c0 [tool] Allow importing packages with NEXT (flutter/packages#3215) * ce9c61b Roll Flutter from 170539f to 0be7c3f (38 revisions) (flutter/packages#3225) * 9747469 Fix deprecation message for GoRouterState.namedLocation (flutter/packages#3092) * 6e4431f [go_router] Bump example `compileSdkVersion` and `package_info_plus` dependency version (flutter/packages#3219) * 925bea8 [pigeon] Validate generated files in CI (flutter/packages#3224) * 3094867 Move iOS Swift unit tests back to Cirrus (flutter/packages#3221) * 763d025 [pigeon] Eliminate some of the test pigeons (flutter/packages#3213)
…Route (flutter#2679) * fixes #108437 support for iterable, list and set * fixes #108437 url encoding for iterable, list and set * fixes #108437 tests * format and analysis fix * fix string encoding * format * removed unused helper name * fix nullability checks for query params encoding * fix all_types.dart for new go router version * rebased to upstream * version * missing file regeneration
3 participants
Add support for Iterable, List and Set to TypedGoRoute
Resolves #108437
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.mdto add a description of the change, [following repository CHANGELOG style].///).