[go_router] Adds an ability to add a custom codec for serializing/des… #5288

chunhtai · 2023-10-31T23:08:57Z

…erializing extra

fixes flutter/flutter#99099
fixes flutter/flutter#137248

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

chunhtai · 2023-10-31T23:19:28Z

packages/go_router/lib/src/router.dart

@@ -121,6 +122,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
  /// The `routes` must not be null and must contain an [GoRouter] to match `/`.
  factory GoRouter({
    required List<RouteBase> routes,
+    Codec<Object?, Object?>? extraCodec,


I have been debating what is the best format to provide codec for multiple class type since GoRouter does not limit how many types can be use as extra during its life cycle.

Things I have tried:

A list
There isn't a way to get what type the codec support, so it is not possible to choose a codec for a given class type

A Map<Type, Codec>
This is fine when encoding the data, but I can't find a good way to get the codec for decoded data.

I end up go with this approach where it would rely on the developer to check the type in their codec and write certain marker in the encoded result to be picked up during decoding

goderbauer

LGTM

packages/go_router/doc/navigation.md

packages/go_router/example/README.md

packages/go_router/example/lib/extra_codec.dart

goderbauer · 2023-11-02T18:22:19Z

packages/go_router/example/lib/extra_codec.dart

+    if (inputAsList[0] == 'ComplexData2') {
+      return ComplexData2(inputAsList[1]! as String);
+    }
+    throw UnimplementedError();


nit: most dart SDK decoders seem to throw a FormatException in this case instead, see for example https://master-api.flutter.dev/flutter/dart-convert/Base64Decoder-class.html

goderbauer · 2023-11-02T18:22:35Z

packages/go_router/example/lib/extra_codec.dart

+      case ComplexData2:
+        return <Object?>['ComplexData2', (input as ComplexData2).data];
+      default:
+        throw UnimplementedError();


goderbauer · 2023-11-02T18:25:22Z

packages/go_router/lib/src/configuration.dart

@@ -231,6 +233,11 @@ class RouteConfiguration {
  /// The global key for top level navigator.
  final GlobalKey<NavigatorState> navigatorKey;

+  /// The codec used to encode and decode extra into a serializable format.


Maybe link the "extra" to GoRoute.extra or wherever users can learn more about what "extra" is.

goderbauer · 2023-11-02T18:32:10Z

packages/go_router/lib/src/match.dart

+// This file deals with json.
+// ignore_for_file: avoid_dynamic_calls


I am generally not a fan of these blanket ignores and would typecast the json where appropriate to make it clear what types you're expecting to deal with.

Not sure what the policy in this repo is for file-level ignores, though.

goderbauer · 2023-11-02T18:34:59Z

packages/go_router/lib/src/match.dart

@@ -420,13 +452,16 @@ class _RouteMatchListDecoder
  RouteMatchList convert(Map<Object?, Object?> input) {
    final String rootLocation =
        input[RouteMatchListCodec._locationKey]! as String;
-    final String? encodedExtra =
-        input[RouteMatchListCodec._extraKey] as String?;
+    final dynamic encodedExtra = input[RouteMatchListCodec._extraKey];


instead of dynamic: Seems like you should know the type here (something like Map<String, Object?> or potentially Map<String, Object?>, maybe?). If it isn't that, line 458 would just crash.

…erializing extra

Co-authored-by: Michael Goderbauer <goderbauer@google.com>

…zing/des… (flutter/packages#5288)

flutter/packages@cccf5d2...49eac1f 2023-11-03 katelovett@google.com [two_dimensional_scrollables] Add borderRadius support to TableSpanDecoration (flutter/packages#5184) 2023-11-03 47866232+chunhtai@users.noreply.github.com [go_router] Adds an ability to add a custom codec for serializing/desâ�¦ (flutter/packages#5288) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

* main: (59 commits) [pointer_interceptor] Move pointer_interceptor to sub folder (flutter#5291) [webview_flutter] Update iOS to Pigeon 13 (flutter#5271) [repo] Adjust error message layout for repo checks (flutter#5241) [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase/example/android/app (flutter#5153) Roll Flutter from f5a9835 to 5a6a322 (15 revisions) (flutter#5345) [google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 (flutter#5240) Roll Flutter from 29b2516 to f5a9835 (101 revisions) (flutter#5341) [pigeon]: Bump org.jetbrains.kotlin:kotlin-gradle-plugin from 1.9.10 to 1.9.20 in /packages/pigeon/platform_tests/test_plugin/android (flutter#5334) [flutter_markdown] Strip leading whitespace from list items (flutter#5294) [two_dimensional_scrollables] Add borderRadius support to TableSpanDecoration (flutter#5184) [go_router] Adds an ability to add a custom codec for serializing/des… (flutter#5288) [go_router] Fixes crashes when dynamically updates routing tables wit… (flutter#5242) [camerax] Fix `_getResolutionSelectorFromPreset` NPE (flutter#5287) [go_router]Fixes the problem what pathParameters is null in redirect when the Router is recreated. (flutter#5258) [google_sign_in] Fix Obj-C formatting bug (flutter#5310) Fix mounted checks (flutter#5305) [flutter_image] [flutter_markdown] Do not use dynamic in an == operator override (flutter#5298) [google_sign_in] Enable FedCM for web. Use token expiration. (flutter#5225) [video_player_web] Listen to loadedmetadata event from video element. (flutter#5289) [local_auth] Update iOS to Pigeon 13 (flutter#5269) ... # Conflicts: # packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFWebViewHostApi.m

flutter#5288) â�¦erializing extra fixes flutter/flutter#99099 fixes flutter/flutter#137248

github-actions bot added p: go_router platform-ios labels Oct 31, 2023

chunhtai commented Oct 31, 2023

View reviewed changes

chunhtai requested a review from goderbauer November 1, 2023 17:36

goderbauer approved these changes Nov 2, 2023

View reviewed changes

chunhtai and others added 4 commits November 3, 2023 12:51

[go_router] Adds an ability to add a custom codec for serializing/des…

1d62ff8

…erializing extra

update

7f00820

Apply suggestions from code review

ce35b85

Co-authored-by: Michael Goderbauer <goderbauer@google.com>

update

e0f0eae

chunhtai force-pushed the issues/137248-alt branch from 2ede2cc to e0f0eae Compare November 3, 2023 19:51

update

39412b5

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2023

auto-submit bot merged commit e890f6b into flutter:main Nov 3, 2023
79 checks passed

engine-flutter-autoroll mentioned this pull request Nov 6, 2023

Roll Packages from cccf5d24d703 to 49eac1fec6c7 (2 revisions) flutter/flutter#137943

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2023

e890f6b [go_router] Adds an ability to add a custom codec for seriali…

820966c

…zing/des… (flutter/packages#5288)

HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023

[go_router] Adds an ability to add a custom codec for serializing/des… (

5ca3e17

flutter#5288) â�¦erializing extra fixes flutter/flutter#99099 fixes flutter/flutter#137248

reidbaker mentioned this pull request Jan 12, 2024

Dialog Debug #5881

Closed

reidbaker mentioned this pull request Jan 19, 2024

[in_app_purchase] Add play country code api #5941

Merged

12 tasks

reidbaker mentioned this pull request Feb 19, 2024

[in_app_purchase_android] Add UserChoiceBilling mode. #6162

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[go_router] Adds an ability to add a custom codec for serializing/des… #5288

[go_router] Adds an ability to add a custom codec for serializing/des… #5288

chunhtai commented Oct 31, 2023 •

edited

Loading

chunhtai Oct 31, 2023

goderbauer left a comment

goderbauer Nov 2, 2023

goderbauer Nov 2, 2023

goderbauer Nov 2, 2023

goderbauer Nov 2, 2023

goderbauer Nov 2, 2023

		// This file deals with json.
		// ignore_for_file: avoid_dynamic_calls

[go_router] Adds an ability to add a custom codec for serializing/des… #5288

[go_router] Adds an ability to add a custom codec for serializing/des… #5288

Conversation

chunhtai commented Oct 31, 2023 • edited Loading

Pre-launch Checklist

chunhtai Oct 31, 2023

Choose a reason for hiding this comment

goderbauer left a comment

Choose a reason for hiding this comment

goderbauer Nov 2, 2023

Choose a reason for hiding this comment

goderbauer Nov 2, 2023

Choose a reason for hiding this comment

goderbauer Nov 2, 2023

Choose a reason for hiding this comment

goderbauer Nov 2, 2023

Choose a reason for hiding this comment

goderbauer Nov 2, 2023

Choose a reason for hiding this comment

chunhtai commented Oct 31, 2023 •

edited

Loading