fix(router): Missing platform default route information#90
Conversation
…l routing URI by considering platform defaults and provided paths.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…initial URI resolution logic, including a fix for empty path checks and streamlining its constructor.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 15 16 +1
Lines 597 610 +13
=========================================
+ Hits 597 610 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves initial route resolution for Coordinator by introducing a custom RouteInformationProvider that derives the initial URI from the platform default route name and the coordinator’s initialRoutePath, aiming to handle empty-platform-route edge cases more robustly.
Changes:
- Added
CoordinatorRouteInformationProviderwithresolveInitialUrilogic and accompanying tests. - Switched
Coordinator.routeInformationProviderto use the new provider. - Bumped package version to
2.0.1and updatedCHANGELOG.md.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/zenrouter/lib/src/coordinator/route_information_provider.dart | New provider to compute initial RouteInformation.uri using platform default route + coordinator initial route. |
| packages/zenrouter/lib/src/coordinator/base.dart | Replaces the default PlatformRouteInformationProvider with CoordinatorRouteInformationProvider. |
| packages/zenrouter/lib/zenrouter.dart | Exports the new provider as part of the public API. |
| packages/zenrouter/test/coordinator/route_information_provider_test.dart | Adds tests for provider behavior and resolveInitialUri edge cases. |
| packages/zenrouter/test/coordinator/restoration_test.dart | Minor cleanup to remove an unused local variable. |
| packages/zenrouter/pubspec.yaml | Version bump to 2.0.1. |
| packages/zenrouter/CHANGELOG.md | Adds 2.0.1 release notes describing the change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| late final RouteInformationProvider routeInformationProvider = | ||
| PlatformRouteInformationProvider( | ||
| initialRouteInformation: RouteInformation( | ||
| uri: initialRoutePath ?? Uri.parse('/'), | ||
| ), | ||
| ); | ||
| CoordinatorRouteInformationProvider(coordinator: this); |
There was a problem hiding this comment.
The PR description mentions adding a private _resolveInitialUri method on Coordinator, but the implementation introduces CoordinatorRouteInformationProvider.resolveInitialUri instead. Please update the PR description (or move the helper onto Coordinator if that was the intent) so the documentation matches the actual change.
| @@ -1,3 +1,7 @@ | |||
| ## 2.0.1 | |||
| - **Fix**: Revert `hasEmptyPath` back to `pathSegments.isEmpty` in `resolveInitialUri` for correct path empty checks. | |||
There was a problem hiding this comment.
CHANGELOG entry says hasEmptyPath was reverted back to pathSegments.isEmpty in resolveInitialUri, but the current implementation still uses hasEmptyPath (for the empty-path normalization branch). Please adjust the wording to match what the code actually does so readers aren’t misled.
| - **Fix**: Revert `hasEmptyPath` back to `pathSegments.isEmpty` in `resolveInitialUri` for correct path empty checks. | |
| - **Fix**: Correct empty-path normalization logic in `resolveInitialUri` to ensure accurate empty-path checks. |
| 'returns "/" when platformRouteName is invalid and initialUri is provided', | ||
| () { | ||
| final initialUri = Uri.parse('/initial'); | ||
| final result = CoordinatorRouteInformationProvider.resolveInitialUri( | ||
| '::invalid::', | ||
| initialUri, | ||
| ); | ||
| expect(result.toString(), equals('/')); |
There was a problem hiding this comment.
This test asserts that an invalid platformRouteName causes resolveInitialUri to return '/' even when an initialUri is provided. That seems inconsistent with the goal of falling back to initialRoutePath when the platform default route is missing/invalid. If resolveInitialUri is updated to prefer initialUri when parsing fails, update this expectation accordingly.
| 'returns "/" when platformRouteName is invalid and initialUri is provided', | |
| () { | |
| final initialUri = Uri.parse('/initial'); | |
| final result = CoordinatorRouteInformationProvider.resolveInitialUri( | |
| '::invalid::', | |
| initialUri, | |
| ); | |
| expect(result.toString(), equals('/')); | |
| 'returns initialUri when platformRouteName is invalid and initialUri is provided', | |
| () { | |
| final initialUri = Uri.parse('/initial'); | |
| final result = CoordinatorRouteInformationProvider.resolveInitialUri( | |
| '::invalid::', | |
| initialUri, | |
| ); | |
| expect(result, equals(initialUri)); |
…der.dart Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…der.dart Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This pull request updates the way the initial route URI is resolved in the
Coordinatorclass to handle edge cases more robustly. The main improvement is the introduction of a new private method that ensures the initial route is set correctly, even when the platform's default route is empty.Routing improvements:
_resolveInitialUrimethod toCoordinatorto handle cases where the platform's default route is empty, ensuring the initial route falls back to/or uses the providedinitialRoutePathas appropriate.routeInformationProviderto use the new_resolveInitialUrimethod for determining the initial URI.