Use correct path to typed routing library #2381
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
I think the tests are failing because this expects go_router 4.2.1 (#2317), but the latest version on Pub is 4.2.0. |
|
@stuartmorgan what's the best way to proceed? |
|
Capturing from issue and Discord comments: 4.2.1 isn't available because I retracted it, as the issue this PR is intended to fix caused out-of-band tree breakage. Retracting 4.2.1 acted as an immediate quasi-revert to fix the tree (and avoided more people hitting flutter/flutter#108274) The next step is to revert the breaking changes in go_router, or fix them forward once I publish the tooling update (in a few minutes hopefully) so that the PR will be tested correctly. |
|
Okay, version 0.8.9 of the tooling is published with the change from flutter/plugins#6146. So the next step is to make a PR that rolls to 0.8.9 (see #2300 for the two files you need to change; unfortunately there's not a single source of truth for the version right now) to make sure the PR is being tested correctly, and also either:
Importantly, that PR should not change No other changes should be landed in |
| @@ -18,7 +18,7 @@ class GoRouterGenerator extends GeneratorForAnnotation<void> { | |||
|
|
|||
| @override | |||
| TypeChecker get typeChecker => const TypeChecker.fromUrl( | |||
| 'package:go_router/src/route_data.dart#TypedGoRoute', | |||
| 'package:go_router/src/typed_routing.dart#TypedGoRoute', | |||
There was a problem hiding this comment.
@kevmoo Do you know if we change this to package:go_router/go_router.dart#TypedGoRoute instead? go_router.dart exports this library.
There was a problem hiding this comment.
Doesn't seem to work...
There was a problem hiding this comment.
Note also that changing go_router_builder can't be a complete fix for the issue created by 4.2.1; a complete fix must be in go_router.
Versions of go_router_builder that have this src dependency have already been published and are in use in the wild, and they depend on go_router: ^4.0.0. People using those existing versions can get new versions of go_router without getting new versions of go_router_builder, so unless you are comfortable breaking those people by violating semver you need to revert or otherwise fix go_router before publishing it again as 4.x.x.
| @@ -18,7 +18,7 @@ class GoRouterGenerator extends GeneratorForAnnotation<void> { | |||
|
|
|||
| @override | |||
| TypeChecker get typeChecker => const TypeChecker.fromUrl( | |||
| 'package:go_router/src/route_data.dart#TypedGoRoute', | |||
| 'package:go_router/src/typed_routing.dart#TypedGoRoute', | |||
There was a problem hiding this comment.
typedgoroute is exposed as a public class, this should be 'package:go_router/go_router.dart#TypedGoRoute`
There was a problem hiding this comment.
Hmm, this doesn't seem to work
|
Reverting the change to go_router here: #2385 |
I'm not sure why the unit tests aren't catching this, but we changed the location of the GoRouteData and TypedRoute classes in #2317 so these strings need to be updated to match.
Fixes flutter/flutter#108274