Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f74ad0d
Return value when pop
NazarenoCavazzon Mar 3, 2023
72db2d6
Merge branch 'main' into feat/return-value-when-pop
NazarenoCavazzon Mar 3, 2023
ba08b81
Fixes
NazarenoCavazzon Mar 3, 2023
845dfbd
Removed unnecessary completer
NazarenoCavazzon Mar 3, 2023
0105de8
removed addPostFrameCallback from docs
NazarenoCavazzon Mar 3, 2023
4a6352c
Moved implementation to ImperativeRouteMatch
NazarenoCavazzon Mar 3, 2023
ecee639
Documented complete() method
NazarenoCavazzon Mar 3, 2023
98e1216
Changes
NazarenoCavazzon Mar 3, 2023
1b5e37b
Update delegate.dart
NazarenoCavazzon Mar 3, 2023
b734714
Update go_router_test.dart
NazarenoCavazzon Mar 3, 2023
ba99cc5
Merge branch 'main' into feat/return-value-when-pop
NazarenoCavazzon Mar 3, 2023
9b556a3
Last fixes
NazarenoCavazzon Mar 3, 2023
8d34c15
Update match.dart
NazarenoCavazzon Mar 3, 2023
0f0eafc
Update packages/go_router/CHANGELOG.md
NazarenoCavazzon Mar 4, 2023
77f6ab9
Merge branch 'main' into feat/return-value-when-pop
NazarenoCavazzon Mar 6, 2023
251966a
Merge branch 'main' into feat/return-value-when-pop
NazarenoCavazzon Mar 9, 2023
ed87847
Merge branch 'main' into feat/return-value-when-pop
NazarenoCavazzon Mar 20, 2023
1661569
Merge branch 'main' into feat/return-value-when-pop
NazarenoCavazzon Mar 23, 2023
c3ed204
Update version to 6.5.0
NazarenoCavazzon Mar 23, 2023
02a4f5c
Updated from main
NazarenoCavazzon Mar 23, 2023
a44a5cc
Merge branch 'main' into feat/return-value-when-pop
NazarenoCavazzon Mar 23, 2023
801c7c6
Type annotations
NazarenoCavazzon Mar 23, 2023
5d88336
Update router.dart
NazarenoCavazzon Mar 23, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/go_router/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 6.3.0

- Supports for returning values on pop.

## NEXT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the ##Next section should be merged in 6.3.0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

## Next meant whoever publish the next version will need to publish the changes in##NEXTas well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AHHHHH, got it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems not address yet?


- Updates compileSdkVersion to 33.
Expand Down
17 changes: 17 additions & 0 deletions packages/go_router/doc/navigation.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,21 @@ Navigator.of(context).push(
);
```

## Returning values
Waiting for a value to be returned:

```dart
onTap: () {
final bool? result = await context.push<bool>('/page2');
if(result ?? false)...
}
```

Returning a value:

```dart
onTap: () => context.pop(true)
```


[Named routes]: https://pub.dev/documentation/go_router/latest/topics/Named%20routes-topic.html
28 changes: 23 additions & 5 deletions packages/go_router/lib/src/delegate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
}

/// Pushes the given location onto the page stack
void push(RouteMatchList matches) {
Future<T?> push<T extends Object?>(RouteMatchList matches) async {
assert(matches.last.route is! ShellRoute);

// Remap the pageKey to allow any number of the same page on the stack
final int count = (_pushCounts[matches.fullpath] ?? 0) + 1;
_pushCounts[matches.fullpath] = count;
final ValueKey<String> pageKey =
ValueKey<String>('${matches.fullpath}-p$count');
final ImperativeRouteMatch newPageKeyMatch = ImperativeRouteMatch(
final ImperativeRouteMatch<T> newPageKeyMatch = ImperativeRouteMatch<T>(
route: matches.last.route,
subloc: matches.last.subloc,
extra: matches.last.extra,
Expand All @@ -95,6 +95,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>

_matchList.push(newPageKeyMatch);
notifyListeners();
return newPageKeyMatch.future;
}

/// Returns `true` if the active Navigator can pop.
Expand All @@ -113,6 +114,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
final _NavigatorStateIterator iterator = _createNavigatorStateIterator();
while (iterator.moveNext()) {
if (iterator.current.canPop()) {
iterator.matchList.last.complete(result);
iterator.current.pop<T>(result);
return;
}
Expand Down Expand Up @@ -268,8 +270,7 @@ class _NavigatorStateIterator extends Iterator<NavigatorState> {
}

/// The route match that represent route pushed through [GoRouter.push].
// TODO(chunhtai): Removes this once imperative API no longer insert route match.
class ImperativeRouteMatch extends RouteMatch {
class ImperativeRouteMatch<T> extends RouteMatch {
/// Constructor for [ImperativeRouteMatch].
ImperativeRouteMatch({
required super.route,
Expand All @@ -278,8 +279,25 @@ class ImperativeRouteMatch extends RouteMatch {
required super.error,
required super.pageKey,
required this.matches,
});
}) : _completer = Completer<T?>();

/// The matches that produces this route match.
final RouteMatchList matches;

/// The completer for the promise returned by [GoRouter.push].
final Completer<T?> _completer;

/// Completes the promise returned by [GoRouter.push].
@override
void complete([dynamic value]) {
_completer.complete(value as T?);
}

/// Returns `true` if the promise returned by [GoRouter.push] has been completed.
@override
bool didComplete() => _completer.isCompleted;

/// The future of the [RouteMatch] completer. When the future completes, this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The future of the [RouteMatch] completer. When the future completes, this
/// The future of the [RouteMatch] completer.
///
/// When the future completes, this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know about private methods being accessed if in the same file, really cool!

/// will return the value passed to [complete].
Future<T?> get future => _completer.future;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be private

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is we need to return this completer future in the push function, if we make it private we cannot return it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can access private method as long as the class is in the same file, so it should be fine

}
23 changes: 23 additions & 0 deletions packages/go_router/lib/src/match.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,29 @@ class RouteMatch {
throw MatcherError('Unexpected route type: $route', restLoc);
}

/// Completes the promise returned by [GoRouter.push], allowing comunication
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

Called when the corresponding [Route] associated with this route match is completed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// between pages.
///
/// If the promise has already been completed, this method does nothing.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

promise is not a keyword in dart. I think you meant future. Eitherway, this is implementation detail, we should focus on when/why this method is called.

///
/// E.g.:
/// ```dart
/// final bool? result = await context.push<bool>('/page2');
/// if(result ?? false){
/// // do something
/// }
/// ```
/// When the page is popped, the promise is completed with the value passed,
/// and the push method returns.
/// to [Navigator.pop].
/// ```dart
/// context.pop(true);
/// ```
void complete([dynamic value]) {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chunhtai what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why optional? I think it can be Object? value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant this method should be call didComplete.

void didComplete(Object? value) {}


/// Returns `true` if the promise returned by [GoRouter.push] has been completed.
bool? didComplete() => null;

/// The matched route.
final RouteBase route;

Expand Down
6 changes: 3 additions & 3 deletions packages/go_router/lib/src/misc/extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,17 @@ extension GoRouterHelper on BuildContext {
);

/// Push a location onto the page stack.
void push(String location, {Object? extra}) =>
Future<T?> push<T extends Object?>(String location, {Object? extra}) =>
GoRouter.of(this).push(location, extra: extra);

/// Navigate to a named route onto the page stack.
void pushNamed(
Future<T?> pushNamed<T extends Object?>(
String name, {
Map<String, String> params = const <String, String>{},
Map<String, dynamic> queryParams = const <String, dynamic>{},
Object? extra,
}) =>
GoRouter.of(this).pushNamed(
GoRouter.of(this).pushNamed<T>(
name,
params: params,
queryParams: queryParams,
Expand Down
2 changes: 1 addition & 1 deletion packages/go_router/lib/src/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
}
if (configuration.matches.last is ImperativeRouteMatch) {
configuration =
(configuration.matches.last as ImperativeRouteMatch).matches;
(configuration.matches.last as ImperativeRouteMatch<dynamic>).matches;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be Object? ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should avoid using dynamic as much as possible unless you are handling json.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
return RouteInformation(
location: configuration.uri.toString(),
Expand Down
18 changes: 8 additions & 10 deletions packages/go_router/lib/src/router.dart
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class GoRouter extends ChangeNotifier implements RouterConfig<RouteMatchList> {
routerDelegate.currentConfiguration.matches.last
is ImperativeRouteMatch) {
newLocation = (routerDelegate.currentConfiguration.matches.last
as ImperativeRouteMatch)
as ImperativeRouteMatch<dynamic>)
.matches
.uri
.toString();
Expand Down Expand Up @@ -204,32 +204,30 @@ class GoRouter extends ChangeNotifier implements RouterConfig<RouteMatchList> {

/// Push a URI location onto the page stack w/ optional query parameters, e.g.
/// `/family/f2/person/p1?color=blue`
void push(String location, {Object? extra}) {
Future<T?> push<T extends Object?>(String location, {Object? extra}) async {
assert(() {
log.info('pushing $location');
return true;
}());
_routeInformationParser
.parseRouteInformationWithDependencies(
final RouteMatchList matches =
await _routeInformationParser.parseRouteInformationWithDependencies(
RouteInformation(location: location, state: extra),
// TODO(chunhtai): avoid accessing the context directly through global key.
// https://github.com/flutter/flutter/issues/99112
_routerDelegate.navigatorKey.currentContext!,
)
.then<void>((RouteMatchList matches) {
_routerDelegate.push(matches);
});
);
return _routerDelegate.push<T>(matches);
}

/// Push a named route onto the page stack w/ optional parameters, e.g.
/// `name='person', params={'fid': 'f2', 'pid': 'p1'}`
void pushNamed(
Future<T?> pushNamed<T extends Object?>(
String name, {
Map<String, String> params = const <String, String>{},
Map<String, dynamic> queryParams = const <String, dynamic>{},
Object? extra,
}) =>
push(
push<T>(
namedLocation(name, params: params, queryParams: queryParams),
extra: extra,
);
Expand Down
2 changes: 1 addition & 1 deletion packages/go_router/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: go_router
description: A declarative router for Flutter based on Navigation 2 supporting
deep linking, data-driven routes and more
version: 6.2.0
version: 6.3.0
repository: https://github.com/flutter/packages/tree/main/packages/go_router
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22

Expand Down
2 changes: 1 addition & 1 deletion packages/go_router/test/delegate_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ void main() {
reason: 'The last match should have been removed',
);
expect(
(goRouter.routerDelegate.matches.last as ImperativeRouteMatch)
(goRouter.routerDelegate.matches.last as ImperativeRouteMatch<dynamic>)
.matches
.uri
.toString(),
Expand Down
48 changes: 46 additions & 2 deletions packages/go_router/test/go_router_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2396,8 +2396,8 @@ void main() {
expect(router.location, loc);
expect(matches.matches, hasLength(2));
expect(find.byType(PersonScreen), findsOneWidget);
final ImperativeRouteMatch imperativeRouteMatch =
matches.matches.last as ImperativeRouteMatch;
final ImperativeRouteMatch<dynamic> imperativeRouteMatch =
matches.matches.last as ImperativeRouteMatch<dynamic>;
expect(imperativeRouteMatch.matches.pathParameters['fid'], fid);
expect(imperativeRouteMatch.matches.pathParameters['pid'], pid);
});
Expand Down Expand Up @@ -2655,6 +2655,26 @@ void main() {
expect(router.extra, extra);
});

testWidgets('calls [push] on closest GoRouter with a promise',
(WidgetTester tester) async {
final GoRouterPushSpy router = GoRouterPushSpy(routes: routes);
await tester.pumpWidget(
MaterialApp.router(
routeInformationProvider: router.routeInformationProvider,
routeInformationParser: router.routeInformationParser,
routerDelegate: router.routerDelegate,
title: 'GoRouter Example',
),
);
final String? result = await router.push<String>(
location,
extra: extra,
);
expect(result, extra);
expect(router.myLocation, location);
expect(router.extra, extra);
});

testWidgets('calls [pushNamed] on closest GoRouter',
(WidgetTester tester) async {
final GoRouterPushNamedSpy router = GoRouterPushNamedSpy(routes: routes);
Expand All @@ -2676,6 +2696,30 @@ void main() {
expect(router.extra, extra);
});

testWidgets('calls [pushNamed] on closest GoRouter with a promise',
(WidgetTester tester) async {
final GoRouterPushNamedSpy router = GoRouterPushNamedSpy(routes: routes);
await tester.pumpWidget(
MaterialApp.router(
routeInformationProvider: router.routeInformationProvider,
routeInformationParser: router.routeInformationParser,
routerDelegate: router.routerDelegate,
title: 'GoRouter Example',
),
);
final String? result = await router.pushNamed<String>(
name,
params: params,
queryParams: queryParams,
extra: extra,
);
expect(result, extra);
expect(router.extra, extra);
expect(router.name, name);
expect(router.params, params);
expect(router.queryParams, queryParams);
});

testWidgets('calls [pop] on closest GoRouter', (WidgetTester tester) async {
final GoRouterPopSpy router = GoRouterPopSpy(routes: routes);
await tester.pumpWidget(
Expand Down
3 changes: 2 additions & 1 deletion packages/go_router/test/inherited_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,12 @@ class MockGoRouter extends GoRouter {
late String latestPushedName;

@override
void pushNamed(String name,
Future<T?> pushNamed<T extends Object?>(String name,
{Map<String, String> params = const <String, String>{},
Map<String, dynamic> queryParams = const <String, dynamic>{},
Object? extra}) {
latestPushedName = name;
return Future<T?>.value();
}

@override
Expand Down
6 changes: 4 additions & 2 deletions packages/go_router/test/test_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ class GoRouterPushSpy extends GoRouter {
Object? extra;

@override
void push(String location, {Object? extra}) {
Future<T?> push<T extends Object?>(String location, {Object? extra}) {
myLocation = location;
this.extra = extra;
return Future<T?>.value(extra as T?);
}
}

Expand All @@ -110,7 +111,7 @@ class GoRouterPushNamedSpy extends GoRouter {
Object? extra;

@override
void pushNamed(
Future<T?> pushNamed<T extends Object?>(
String name, {
Map<String, String> params = const <String, String>{},
Map<String, dynamic> queryParams = const <String, dynamic>{},
Expand All @@ -120,6 +121,7 @@ class GoRouterPushNamedSpy extends GoRouter {
this.params = params;
this.queryParams = queryParams;
this.extra = extra;
return Future<T?>.value(extra as T?);
}
}

Expand Down