[go_router_builder] Fixes broken enhanced enums.
Fixes #105876
Added support to enhanced enums introduced in dart 2.17
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [x] I listed at least one issue that this PR fixes in the description above.
- [x] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style. - [ ] I updated/added relevant documentation (doc comments with
///). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
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.
Thanks for sending this pull request. Could you explain what scenarios you'd want to use an enhanced enum instead of a regular enum for your routes? It's not clear to me that we should allow enhanced enums since we can't map all of their fields cleanly to parameters. In my mind it would be better to produce an error if an enhanced enum is used that recommends using a regular enum instead.
Thanks for sending this pull request. Could you explain what scenarios you'd want to use an enhanced enum instead of a regular enum for your routes? It's not clear to me that we should allow enhanced enums since we can't map all of their fields cleanly to parameters. In my mind it would be better to produce an error if an enhanced enum is used that recommends using a regular enum instead.
In my production use case I switched from adding functionalities to an enum with extensions to enhanced enums that allow to write less code and achieve the same result with a more concise and straight forward way, especially for json serialization/deserialization.
Mapping an enhanced enum fields should not be done since according to dart specification they must be final, unmutable and can only be initialised by each enum value calling a const constructor.
To declare an enhanced enum, follow a syntax similar to normal classes, but with a few extra requirements:
Instance variables must be final, including those added by mixins. All generative constructors must be constant. Factory constructors can only return one of the fixed, known enum instances. No other class can be extended as Enum is automatically extended. There cannot be overrides for index, hashCode, the equality operator ==. A member named values cannot be declared in an enum, as it would conflict with the automatically generated static values getter. All instances of the enum must be declared in the beginning of the declaration, and there must be at least one instance declared.
An example of use is pairing each enum value with additional useful data:
Enum definition
enum SportEnum {
volleyball(
imageUrl: 'https://i.cbc.ca/1.6477752.1654316355!/fileImage/httpImage/image.jpg_gen/derivatives/16x9_940/canada-vs-usa-volleyball.jpg',
playerPerTeam: 6,
accessory: null,
hasNet: true,
),
football(
imageUrl: 'https://upload.wikimedia.org/wikipedia/commons/b/b9/Football_iu_1996.jpg',
playerPerTeam: 11,
accessory: null,
hasNet: true,
),
tennis(
imageUrl: 'https://upload.wikimedia.org/wikipedia/commons/9/94/2013_Australian_Open_-_Guillaume_Rufin.jpg',
playerPerTeam: 2,
accessory: 'Rackets',
hasNet: true,
),
hockey(
imageUrl: 'https://upload.wikimedia.org/wikipedia/commons/3/39/Pittsburgh_Penguins%2C_Washington_Capitals%2C_Bryan_Rust_%2833744033514%29.jpg',
playerPerTeam: 6,
accessory: 'Hockey sticks',
hasNet: true,
),
;
final String imageUrl;
final int playerPerTeam;
final String? accessory;
final bool hasNet;
const SportEnum({
required this.accessory,
required this.hasNet,
required this.imageUrl,
required this.playerPerTeam,
});
}
Route declaration
@TypedGoRoute<SportDetailPageData>(path: '/sport/:sport')
class SportDetailPageData extends GoRouteData {
SportDetailPageData(this.sport);
final SportEnum sport;
@override
Widget build(BuildContext) => SportDetailPage(
name: sport.name,
image: sport.imageUrl,
playerPerTeam: sport.playerPerTeam,
hasNet: sport.hasNet,
accessory: sport.accessory,
);
}
Generated code
GoRoute get $sportDetailPageData => GoRouteData.$route(
path: '/sport/:sport',
factory: $SportDetailPageDataExtension._fromState,
);
extension $SportDetailPageDataExtension on SportDetailPageData {
static SportDetailPageData _fromState(GoRouterState state) =>
SportDetailPageData(
_$SportEnumEnumMap._$fromName(state.params['sport']!),
);
String get location => GoRouteData.$location(
'/sport/${Uri.encodeComponent(_$SportEnumEnumMap[sport]!)}',
);
void go(BuildContext context) => context.go(location, extra: this);
void push(BuildContext context) => context.push(location, extra: this);
}
const _$SportEnumEnumMap = {
SportEnum.volleyball: 'volleyball',
SportEnum.football: 'football',
SportEnum.tennis: 'tennis',
SportEnum.hockey: 'hockey',
};
extension<T extends Enum> on Map<T, String> {
T _$fromName(String value) =>
entries.singleWhere((element) => element.value == value).key;
}
@chunhtai i add an enhanced enum field to all_types.dart example as requested and I also fixed a bug in it (routeInformationProvider was missing and initial route '/' wouldn't be found resulting in an error page).
Regarding widget_test.dart is a test against the app written in main.dart with a complete navigation tree, while on the contrary all_types.dart has only a single path with all the possible data combination.
Should I put an example in main.dart and then its test in widget_test.dart or a complete separate example app with its test suite?
@chunhtai i add an enhanced enum field to
all_types.dartexample as requested and I also fixed a bug in it (routeInformationProvider was missing and initial route '/' wouldn't be found resulting in an error page).Regarding
widget_test.dartis a test against the app written inmain.dartwith a complete navigation tree, while on the contraryall_types.darthas only a single path with all the possible data combination.Should I put an example in
main.dartand then its test inwidget_test.dartor a complete separate example app with its test suite?
Is it possible to modify the all_types to add a new route so it can navigate in between?
@chunhtai instead of just running a single route with all types I could refactor the all_types app to make it more structured with a dedicated route per type and run a complete test against the entire app. Would it be ok?
@Skogsfrae
Yes that is a good idea, please go ahead.
(triage) Hi @Skogsfrae, are you still planning to come back to this pr?
Hi @chunhtai Yes, I've been on holiday and came back on track in these days 💪🏽 I've already rewritten the all types app and tomorrow I'll write the test suite, then it'll be ready to go
@chunhtai I rebased my branch to upstream/main and updated release info. You can review the change in example/lib/all_types.dart and the tests in example/test/all_types_test.dart
Let me know if more changes need to be done.
@Skogsfrae could you fix the merge conflicts?