Skip to content

[go_router] Expose full Uri on GoRouterState in GoRouterRedirect #5742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@
## 13.2.0

- Exposes full `Uri` on `GoRouterState` in `GoRouterRedirect`

## 13.1.0

- Adds `topRoute` to `GoRouterState`
Expand Down
52 changes: 25 additions & 27 deletions packages/go_router/lib/src/information_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ import 'package:flutter/widgets.dart';

import 'match.dart';

// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// ignore_for_file: deprecated_member_use

/// The type of the navigation.
///
/// This enum is used by [RouteInformationState] to denote the navigation
Expand Down Expand Up @@ -85,7 +81,7 @@ class GoRouteInformationProvider extends RouteInformationProvider
Listenable? refreshListenable,
}) : _refreshListenable = refreshListenable,
_value = RouteInformation(
location: initialLocation,
uri: Uri.parse(initialLocation),
state: RouteInformationState<void>(
extra: initialExtra, type: NavigatingType.go),
),
Expand All @@ -96,8 +92,8 @@ class GoRouteInformationProvider extends RouteInformationProvider
final Listenable? _refreshListenable;

static WidgetsBinding get _binding => WidgetsBinding.instance;
static const RouteInformation _kEmptyRouteInformation =
RouteInformation(location: '');
static final RouteInformation _kEmptyRouteInformation =
RouteInformation(uri: Uri.parse(''));

@override
void routerReportsNewRouteInformation(RouteInformation routeInformation,
Expand All @@ -109,9 +105,9 @@ class GoRouteInformationProvider extends RouteInformationProvider
final bool replace;
switch (type) {
case RouteInformationReportingType.none:
if (_valueInEngine.location == routeInformation.location &&
const DeepCollectionEquality()
.equals(_valueInEngine.state, routeInformation.state)) {
if (!_valueHasChanged(
newLocationUri: routeInformation.uri,
newState: routeInformation.state)) {
return;
}
replace = _valueInEngine == _kEmptyRouteInformation;
Expand All @@ -122,10 +118,7 @@ class GoRouteInformationProvider extends RouteInformationProvider
}
SystemNavigator.selectMultiEntryHistory();
SystemNavigator.routeInformationUpdated(
// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// ignore: unnecessary_null_checks, unnecessary_non_null_assertion
location: routeInformation.location!,
uri: routeInformation.uri,
state: routeInformation.state,
replace: replace,
);
Expand All @@ -137,17 +130,16 @@ class GoRouteInformationProvider extends RouteInformationProvider
RouteInformation _value;

@override
// TODO(chunhtai): remove this ignore once package minimum dart version is
// above 3.
// ignore: unnecessary_overrides
void notifyListeners() {
super.notifyListeners();
}

void _setValue(String location, Object state) {
final Uri uri = Uri.parse(location);

final bool shouldNotify =
_value.location != location || _value.state != state;
_value = RouteInformation(location: location, state: state);
_valueHasChanged(newLocationUri: uri, newState: state);
_value = RouteInformation(uri: Uri.parse(location), state: state);
if (shouldNotify) {
notifyListeners();
}
Expand Down Expand Up @@ -235,14 +227,27 @@ class GoRouteInformationProvider extends RouteInformationProvider
_value = _valueInEngine = routeInformation;
} else {
_value = RouteInformation(
location: routeInformation.location,
uri: routeInformation.uri,
state: RouteInformationState<void>(type: NavigatingType.go),
);
_valueInEngine = _kEmptyRouteInformation;
}
notifyListeners();
}

bool _valueHasChanged(
{required Uri newLocationUri, required Object? newState}) {
const DeepCollectionEquality deepCollectionEquality =
DeepCollectionEquality();
return !deepCollectionEquality.equals(
_value.uri.path, newLocationUri.path) ||
!deepCollectionEquality.equals(
_value.uri.queryParameters, newLocationUri.queryParameters) ||
!deepCollectionEquality.equals(
_value.uri.fragment, newLocationUri.fragment) ||
!deepCollectionEquality.equals(_value.state, newState);
}

@override
void addListener(VoidCallback listener) {
if (!hasListeners) {
Expand Down Expand Up @@ -274,11 +279,4 @@ class GoRouteInformationProvider extends RouteInformationProvider
_platformReportsNewRouteInformation(routeInformation);
return SynchronousFuture<bool>(true);
}

Copy link
Member

Choose a reason for hiding this comment

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

why remove this?

Copy link
Contributor Author

@ChopinDavid ChopinDavid Jan 13, 2024

Choose a reason for hiding this comment

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

@override
Future<bool> didPushRoute(String route) {
assert(hasListeners);
_platformReportsNewRouteInformation(RouteInformation(location: route));
return SynchronousFuture<bool>(true);
}
}
23 changes: 7 additions & 16 deletions packages/go_router/lib/src/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,13 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
}

late final RouteMatchList initialMatches;
initialMatches =
// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// TODO(chunhtai): After the migration from routeInformation's location
// to uri, empty path check might be required here; see
// https://github.com/flutter/packages/pull/5113#discussion_r1374861070
// ignore: deprecated_member_use, unnecessary_non_null_assertion
configuration.findMatch(routeInformation.location!, extra: state.extra);
initialMatches = configuration.findMatch(
routeInformation.uri.path.isEmpty
? '${routeInformation.uri}/'
: routeInformation.uri.toString(),
extra: state.extra);
if (initialMatches.isError) {
// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// ignore: deprecated_member_use
log('No initial matches: ${routeInformation.location}');
log('No initial matches: ${routeInformation.uri.path}');
}

return debugParserFuture = _redirect(
Expand Down Expand Up @@ -142,10 +136,7 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
location = configuration.uri.toString();
}
return RouteInformation(
// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// ignore: deprecated_member_use
location: location,
uri: Uri.parse(location),
state: _routeMatchListCodec.encode(configuration),
);
}
Expand Down
15 changes: 14 additions & 1 deletion packages/go_router/lib/src/path_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'misc/errors.dart';
import 'route.dart';

final RegExp _parameterRegExp = RegExp(r':(\w+)(\((?:\\.|[^\\()])+\))?');
Expand Down Expand Up @@ -120,6 +121,9 @@ String concatenatePaths(String parentPath, String childPath) {

/// Normalizes the location string.
String canonicalUri(String loc) {
if (loc.isEmpty) {
throw GoException('Location cannot be empty.');
}
String canon = Uri.parse(loc).toString();
canon = canon.endsWith('?') ? canon.substring(0, canon.length - 1) : canon;

Expand All @@ -131,9 +135,18 @@ String canonicalUri(String loc) {
? canon.substring(0, canon.length - 1)
: canon;

// replace '/?', except for first occurrence, from path only
// /login/?from=/ => /login?from=/
// /?from=/ => /?from=/
canon = canon.replaceFirst('/?', '?', 1);
final Uri uri = Uri.parse(canon);
final int pathStartIndex = uri.host.isNotEmpty
? uri.toString().indexOf(uri.host) + uri.host.length
: uri.hasScheme
? uri.toString().indexOf(uri.scheme) + uri.scheme.length
: 0;
if (pathStartIndex < canon.length) {
canon = canon.replaceFirst('/?', '?', pathStartIndex + 1);
}

return canon;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/go_router/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
name: go_router
description: A declarative router for Flutter based on Navigation 2 supporting
deep linking, data-driven routes and more
version: 13.1.0
version: 13.2.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

environment:
sdk: ">=3.0.0 <4.0.0"
flutter: ">=3.10.0"
sdk: ">=3.1.0 <4.0.0"
flutter: ">=3.13.0"

dependencies:
collection: ^1.15.0
Expand Down
99 changes: 91 additions & 8 deletions packages/go_router/test/go_router_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,95 @@ void main() {
expect(find.byType(DummyScreen), findsOneWidget);
});

testWidgets(
'match top level route when location has scheme/host and has trailing /',
(WidgetTester tester) async {
final List<GoRoute> routes = <GoRoute>[
GoRoute(
path: '/',
builder: (BuildContext context, GoRouterState state) =>
const HomeScreen(),
),
];

final GoRouter router = await createRouter(routes, tester);
router.go('https://www.domain.com/?bar=baz');
await tester.pumpAndSettle();
final List<RouteMatchBase> matches =
router.routerDelegate.currentConfiguration.matches;
expect(matches, hasLength(1));
expect(matches.first.matchedLocation, '/');
expect(find.byType(HomeScreen), findsOneWidget);
});

testWidgets(
'match top level route when location has scheme/host and has trailing / (2)',
(WidgetTester tester) async {
final List<GoRoute> routes = <GoRoute>[
GoRoute(
path: '/',
builder: (BuildContext context, GoRouterState state) =>
const HomeScreen(),
),
GoRoute(
path: '/login',
builder: (BuildContext context, GoRouterState state) =>
const LoginScreen(),
),
];

final GoRouter router = await createRouter(routes, tester);
router.go('https://www.domain.com/login/');
await tester.pumpAndSettle();
final List<RouteMatchBase> matches =
router.routerDelegate.currentConfiguration.matches;
expect(matches, hasLength(1));
expect(matches.first.matchedLocation, '/login');
expect(find.byType(LoginScreen), findsOneWidget);
});

testWidgets(
'match top level route when location has scheme/host and has trailing / (3)',
(WidgetTester tester) async {
final List<GoRoute> routes = <GoRoute>[
GoRoute(
path: '/profile',
builder: dummy,
redirect: (_, __) => '/profile/foo'),
GoRoute(path: '/profile/:kind', builder: dummy),
];

final GoRouter router = await createRouter(routes, tester);
router.go('https://www.domain.com/profile/');
await tester.pumpAndSettle();
final List<RouteMatchBase> matches =
router.routerDelegate.currentConfiguration.matches;
expect(matches, hasLength(1));
expect(matches.first.matchedLocation, '/profile/foo');
expect(find.byType(DummyScreen), findsOneWidget);
});

testWidgets(
'match top level route when location has scheme/host and has trailing / (4)',
(WidgetTester tester) async {
final List<GoRoute> routes = <GoRoute>[
GoRoute(
path: '/profile',
builder: dummy,
redirect: (_, __) => '/profile/foo'),
GoRoute(path: '/profile/:kind', builder: dummy),
];

final GoRouter router = await createRouter(routes, tester);
router.go('https://www.domain.com/profile/?bar=baz');
await tester.pumpAndSettle();
final List<RouteMatchBase> matches =
router.routerDelegate.currentConfiguration.matches;
expect(matches, hasLength(1));
expect(matches.first.matchedLocation, '/profile/foo');
expect(find.byType(DummyScreen), findsOneWidget);
});

testWidgets('repeatedly pops imperative route does not crash',
(WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/123369.
Expand Down Expand Up @@ -1301,10 +1390,7 @@ void main() {
expect(find.byKey(const ValueKey<String>('home')), findsOneWidget);

router.routeInformationProvider.didPushRouteInformation(
// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// ignore: deprecated_member_use
RouteInformation(location: location, state: state));
RouteInformation(uri: Uri.parse(location), state: state));
await tester.pumpAndSettle();
// Make sure it has all the imperative routes.
expect(find.byKey(const ValueKey<String>('settings-1')), findsOneWidget);
Expand Down Expand Up @@ -2435,10 +2521,7 @@ void main() {
routes,
tester,
);
// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// ignore: deprecated_member_use
expect(router.routeInformationProvider.value.location, '/dummy');
expect(router.routeInformationProvider.value.uri.path, '/dummy');
TestWidgetsFlutterBinding.instance.platformDispatcher
.clearDefaultRouteNameTestValue();
});
Expand Down
43 changes: 39 additions & 4 deletions packages/go_router/test/information_provider_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,46 @@ void main() {
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
provider.addListener(expectAsync0(() {}));
// TODO(chunhtai): remove this ignore and migrate the code
// https://github.com/flutter/flutter/issues/124045.
// ignore_for_file: deprecated_member_use
provider
.didPushRouteInformation(const RouteInformation(location: newRoute));
.didPushRouteInformation(RouteInformation(uri: Uri.parse(newRoute)));
});

testWidgets('didPushRouteInformation maintains uri scheme and host',
(WidgetTester tester) async {
const String expectedScheme = 'https';
const String expectedHost = 'www.example.com';
const String expectedPath = '/some/path';
const String expectedUriString =
'$expectedScheme://$expectedHost$expectedPath';
late final GoRouteInformationProvider provider =
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
provider.addListener(expectAsync0(() {}));
provider.didPushRouteInformation(
RouteInformation(uri: Uri.parse(expectedUriString)));
expect(provider.value.uri.scheme, 'https');
expect(provider.value.uri.host, 'www.example.com');
expect(provider.value.uri.path, '/some/path');
expect(provider.value.uri.toString(), expectedUriString);
});

testWidgets('didPushRoute maintains uri scheme and host',
(WidgetTester tester) async {
const String expectedScheme = 'https';
const String expectedHost = 'www.example.com';
const String expectedPath = '/some/path';
const String expectedUriString =
'$expectedScheme://$expectedHost$expectedPath';
late final GoRouteInformationProvider provider =
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
provider.addListener(expectAsync0(() {}));
provider.didPushRouteInformation(
RouteInformation(uri: Uri.parse(expectedUriString)));
expect(provider.value.uri.scheme, 'https');
expect(provider.value.uri.host, 'www.example.com');
expect(provider.value.uri.path, '/some/path');
expect(provider.value.uri.toString(), expectedUriString);
});
});
}
Loading