[go_router_builder] Add go_router StatefulShellRoute support to go_router_builder by hannah-hyj · Pull Request #4238 · flutter/packages

hannah-hyj · 2023-06-16T20:44:47Z

This reverts commit 68f99cb4ac66ca445b37bdc8619875f163b739f3.

issue: [#127371](flutter/flutter#127371) go_router_builder_pr #4238

…er-stateful

chunhtai · 2023-07-13T17:00:39Z

packages/go_router_builder/lib/src/route_config.dart

+    required this.navigatorKey,
+    required super.routeDataClass,
+    required super.parent,
+    required super.parentNavigatorKey,


StatefulShellBranch doesn't support parent navigator key

I think we should have another more generic base class for RouteBaseConfig so that it can be extended by StatefulShellBranchConfig without the unused logic or property

Just to confirm, do you mean like

RouteOrBranchBaseConfig -> RouteBaseConfig -> (GoRouteConfig, StatefulShellRouteConfig, etc)
RouteOrBranchBaseConfig -> StatefulShellBranchConfig

packages/go_router_builder/lib/src/route_config.dart

chunhtai · 2023-07-13T17:02:59Z

packages/go_router_builder/lib/src/route_config.dart

+
+  @override
+  String get routeConstructorParameters =>
+      navigatorKey == null ? '' : 'navigatorKey: $navigatorKey,';


no navigator key, but we need to have a way to pass in
navigatorContainerBuilder and restorationScopeId

chunhtai · 2023-07-13T17:13:57Z

packages/go_router_builder/lib/src/route_config.dart

+          ),
+        );
+        break;
+      default:


why not use case 'TypedGoRoute':?
it should throw if it is anything else.

chunhtai · 2023-07-13T17:16:08Z

packages/go_router_builder/lib/src/route_config.dart

-    value._children.addAll(reader.read('routes').listValue.map<RouteBaseConfig>(
-        (DartObject e) => RouteBaseConfig._fromAnnotation(
+    value._children.addAll(reader
+        .read(typeName == 'TypedStatefulShellRoute' ? 'branches' : 'routes')


can this be a getter in the RouteBaseConfig? In general we should try to have less check like this in this method as possible.

chunhtai · 2023-07-13T17:16:39Z

packages/go_router_builder/lib/src/route_config.dart

        ? ''
        : '''
-routes: [${_children.map((RouteBaseConfig e) => '${e._invokesRouteConstructor()},').join()}],
+${routeDataClassName == 'StatefulShellRouteData' ? 'branches' : 'routes'}: [${_children.map((RouteBaseConfig e) => '${e._invokesRouteConstructor()},').join()}],


chunhtai · 2023-07-13T17:17:07Z

packages/go_router_builder/lib/src/route_config.dart

        ? ''
        : 'parentNavigatorKey: $parentNavigatorKey,';
+
+    if (routeDataClassName == 'StatefulShellBranchData') {


chunhtai · 2023-07-27T20:02:02Z

packages/go_router_builder/lib/src/route_config.dart

+class StatefulShellRouteConfig extends RouteBaseConfig {
+  StatefulShellRouteConfig._({
+    required super.routeDataClass,
+    required super.parent,


parent navigator key?

chunhtai · 2023-07-27T20:04:40Z

packages/go_router_builder/lib/src/route_config.dart

+            classElement,
+            parameterName: r'$parentNavigatorKey',
+          ),
        );


can you add a default and throws? it will be useful to remind people when they add a new typed route

chunhtai · 2023-07-27T20:25:04Z

packages/go_router_builder/lib/src/route_config.dart

-  /// The parent navigator key string that is used for initialize the
-  /// `RouteBase` class this config generates.
-  final String? parentNavigatorKey;
+  static String _generateChildrenGetterName(String name) {


This should probably exposed as a getter for subclass to override.

it's used in the factory function factory RouteBaseConfig._fromAnnotation and Instance members can’t be accessed from a factory constructor.

…er-stateful

chunhtai

LGTM, nice work!

auto-submit · 2023-08-02T21:21:47Z

auto label is removed for flutter/packages, pr: 4238, due to Pull request flutter/packages/4238 is not in a mergeable state.

…to go_router_builder (flutter/packages#4238)

flutter/packages@4e4961a...d00c1f9 2023-08-03 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.1 to 2.21.2 (flutter/packages#4637) 2023-08-03 engine-flutter-autoroll@skia.org Roll Flutter from b3f99ff to c00d241 (12 revisions) (flutter/packages#4640) 2023-08-03 jpnurmi@gmail.com [path_provider] Add getApplicationCachePath() - implementations (flutter/packages#4619) 2023-08-03 109253501+pdblasi-google@users.noreply.github.com [various] Removed references to deprecated `TestWindow` APIs (flutter/packages#4558) 2023-08-02 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 4.1.0 to 4.3.0 (flutter/packages#4432) 2023-08-02 jhy03261997@gmail.com [go_router_builder] Add go_router StatefulShellRoute support to go_router_builder (flutter/packages#4238) 2023-08-02 reidbaker@google.com [Tooling] Add google owned cache for dependencies as an option in ci (flutter/packages#4567) 2023-08-02 engine-flutter-autoroll@skia.org Roll Flutter from 1d59196 to b3f99ff (32 revisions) (flutter/packages#4634) 2023-08-02 32538273+ValentinVignal@users.noreply.github.com [go_router_builder] Support `ShellRouteData` without `const` constructor (flutter/packages#4627) 2023-08-02 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.0 to 2.21.1 (flutter/packages#4573) 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

hannah-hyj and others added 6 commits June 15, 2023 18:17

implemented helpers for StatefulShellRoute

990c293

Merge branch 'main' into statefulshellroute

4918014

lint

c6bde6d

Update route_data.dart

52ce037

lint

b9c8080

Update route_data.dart

9005a8c

github-actions bot added p: go_router p: go_router_builder labels Jun 16, 2023

hannah-hyj mentioned this pull request Jun 16, 2023

[go_router] implemented helpers for StatefulShellRoute #4228

Merged

11 tasks

hannah-hyj added 6 commits June 26, 2023 11:53

update

8cf69b6

resolve comments

5963020

1

4f60cd6

Revert "1"

e4fc526

This reverts commit 68f99cb4ac66ca445b37bdc8619875f163b739f3.

update

d328021

update tests

bc19f53

hannah-hyj force-pushed the builder-stateful branch from 855cce7 to bc19f53 Compare June 28, 2023 23:38

auto-submit bot pushed a commit that referenced this pull request Jul 7, 2023

[go_router] implemented helpers for StatefulShellRoute (#4228)

1c78359

issue: [#127371](flutter/flutter#127371) go_router_builder_pr #4238

Merge branch 'main' of https://github.com/flutter/packages into build…

f827680

…er-stateful

github-actions bot removed the p: go_router label Jul 10, 2023

hannah-hyj added 2 commits July 9, 2023 17:37

update

6a09020

bump version

bb6064b

hannah-hyj changed the title ~~[draft][go_router_builder] Add go_router StatefulShellRoute support to go_router_builder~~ [go_router_builder] Add go_router StatefulShellRoute support to go_router_builder Jul 10, 2023

hannah-hyj marked this pull request as ready for review July 10, 2023 01:03

hannah-hyj requested a review from chunhtai as a code owner July 10, 2023 01:03

hannah-hyj added 4 commits July 9, 2023 18:25

Update route_config.dart

441eb78

Update stateful_shell_route_test.dart

8b4e2de

Merge branch 'main' of https://github.com/flutter/packages into build…

a2ef5d2

…er-stateful

update route_config

13865ac

chunhtai reviewed Jul 13, 2023

View reviewed changes

Update route_config.dart

e3d109d

hannah-hyj mentioned this pull request Jul 26, 2023

[go_router_builder] Add go_router StatefulShellRoute support to go_router_builder flutter/flutter#127371

Closed

chunhtai reviewed Jul 27, 2023

View reviewed changes

hannah-hyj added 5 commits July 31, 2023 13:33

update

ecbc5f5

Merge branch 'main' of https://github.com/flutter/packages into build…

9808f62

…er-stateful

Update pubspec.yaml

6044e32

lint

3c2ed3b

Update stateful_shell_route_example.dart

1e78def

hannah-hyj requested a review from chunhtai July 31, 2023 21:40

hannah-hyj added 3 commits July 31, 2023 14:49

Update pubspec.yaml

eecb67e

Update pubspec.yaml

2e63519

Update stateful_shell_route_example.dart

cfb1de8

chunhtai approved these changes Aug 1, 2023

View reviewed changes

hannah-hyj added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 2, 2023

Merge branch 'main' into builder-stateful

7fa8523

hannah-hyj added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 2, 2023

auto-submit bot merged commit c3a5fb9 into flutter:main Aug 2, 2023

engine-flutter-autoroll mentioned this pull request Aug 3, 2023

Roll Packages from 4e4961a24b3d to 9323e33ed9d8 (8 revisions) flutter/flutter#131852

Closed

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

c3a5fb9 [go_router_builder] Add go_router StatefulShellRoute support …

c72cebc

…to go_router_builder (flutter/packages#4238)

engine-flutter-autoroll mentioned this pull request Aug 3, 2023

Roll Packages from 4e4961a24b3d to 94ec57fe2583 (9 revisions) flutter/flutter#131855

Closed

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

c3a5fb9 [go_router_builder] Add go_router StatefulShellRoute support …

f87363c

…to go_router_builder (flutter/packages#4238)

engine-flutter-autoroll mentioned this pull request Aug 3, 2023

Roll Packages from 4e4961a24b3d to d00c1f9eb01c (10 revisions) flutter/flutter#131866

Merged

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

c3a5fb9 [go_router_builder] Add go_router StatefulShellRoute support …

2af091f

…to go_router_builder (flutter/packages#4238)

reidbaker mentioned this pull request Aug 4, 2023

[All] Expand artifact hub to all plugins #4645

Merged

11 tasks

This comment was marked as off-topic.

Sign in to view

Conversation

hannah-hyj commented Jun 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Aug 2, 2023

Uh oh!

This comment was marked as off-topic.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hannah-hyj commented Jun 16, 2023 •

edited

Loading