Repository navigation
[go_router_builder] Adds support for enhanced enums. - #2395
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. |
|
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.
An example of use is pairing each enum value with additional useful data: Enum definitionenum 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 codeGoRoute 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
left a comment
There was a problem hiding this comment.
Can you add the enum to the all_types.dart example so the test can on the example?
also would be good to write a test in example/test similar to widget_test.dart to verify the generated content can run without issue.
|
@chunhtai i add an enhanced enum field to Regarding Should I put an example in |
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 |
|
Yes that is a good idea, please go ahead. |
|
(triage) Hi @Skogsfrae, are you still planning to come back to this pr? |
|
Hi @chunhtai |
|
@chunhtai I rebased my branch to upstream/main and updated release info. You can review the change in Let me know if more changes need to be done. |
chunhtai
left a comment
There was a problem hiding this comment.
code LGTM, it looks like there are some formatting issue
https://github.com/flutter/packages/pull/2395/checks?check_run_id=8372456407
There was a problem hiding this comment.
Do these need to be real links? This could cause security issues...
There was a problem hiding this comment.
Why are these all named *FieldField now instead of just *Field?
There was a problem hiding this comment.
My fault, I use a find and replace all 😬
There was a problem hiding this comment.
| * Fixes broken code generation for enhanced enums. [#105876](https://github.com/flutter/flutter/issues/105876). | |
| * Adds support for enhanced enums. [#105876](https://github.com/flutter/flutter/issues/105876). |
johnpryan
left a comment
There was a problem hiding this comment.
LGTM overall - my comments are mostly minor
|
@Skogsfrae could you fix the merge conflicts? |
Adds build-time detection of routes that resolve to the same URL pattern. `go_router` matches the first route that fits, so a duplicated path silently makes the later route's page unreachable, and `LaterRoute().go(context)` navigates you to `EarlierRoute`'s page instead. Today nothing surfaces that until you run into the issue at runtime. Reported as a warning by default, and new `duplicate_route_paths` builder option accepts `warning` (the default), `error`, and `ignore`, so teams whose duplicates are mistakes can fail the build instead. **Before**, `go_router_builder`'s own example app built clean despite two duplicate registrations: ``` $ dart run build_runner build [INFO] Succeeded after 14s with 36 outputs ``` **After**: ``` $ dart run build_runner build W go_router_builder on lib/all_types.dart: Duplicate route path detected: DoubleRoute is declared more than once at "double-route/:requiredDoubleField". ``` That duplicate was real. `DoubleRoute` had been registered twice since #2395 (2022) and `DoubleExtensionRoute` since #9458 (2025). Both were unreachable, and this PR removes them. The only change to the generated output is the dead route entries. What competes for a URL namespace: - Sibling routes, with path parameters normalized, so `product/:id` and `product/:productId` are one pattern. - Routes inside shell routes and `StatefulShellRoute` branches, since those own no path of their own and their children compete with the routes around them. - Relative routes, which own their path like any other route, so `/home/details/edit` and `/home/edit` stay distinct. - Routes from separate annotations, since the generated `$appRoutes` collects them into one list. Scope is the library, `part` files included. **Testing:** five new golden inputs in `test_inputs/` cover sibling duplicates, parameter-name-only differences, `StatefulShellRoute` branches, relative routes, cross-annotation duplicates, and the sound same-class grouping case. The golden harness gained two optional companion files per input, documented in the README: `<name>.dart.options` supplies JSON builder options so each severity is exercised through the real option-parsing path, and `<name>.dart.warnings` asserts logged warnings, with an empty file asserting silence. Fixes flutter/flutter#190741 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
…er#12399) Adds build-time detection of routes that resolve to the same URL pattern. `go_router` matches the first route that fits, so a duplicated path silently makes the later route's page unreachable, and `LaterRoute().go(context)` navigates you to `EarlierRoute`'s page instead. Today nothing surfaces that until you run into the issue at runtime. Reported as a warning by default, and new `duplicate_route_paths` builder option accepts `warning` (the default), `error`, and `ignore`, so teams whose duplicates are mistakes can fail the build instead. **Before**, `go_router_builder`'s own example app built clean despite two duplicate registrations: ``` $ dart run build_runner build [INFO] Succeeded after 14s with 36 outputs ``` **After**: ``` $ dart run build_runner build W go_router_builder on lib/all_types.dart: Duplicate route path detected: DoubleRoute is declared more than once at "double-route/:requiredDoubleField". ``` That duplicate was real. `DoubleRoute` had been registered twice since flutter#2395 (2022) and `DoubleExtensionRoute` since flutter#9458 (2025). Both were unreachable, and this PR removes them. The only change to the generated output is the dead route entries. What competes for a URL namespace: - Sibling routes, with path parameters normalized, so `product/:id` and `product/:productId` are one pattern. - Routes inside shell routes and `StatefulShellRoute` branches, since those own no path of their own and their children compete with the routes around them. - Relative routes, which own their path like any other route, so `/home/details/edit` and `/home/edit` stay distinct. - Routes from separate annotations, since the generated `$appRoutes` collects them into one list. Scope is the library, `part` files included. **Testing:** five new golden inputs in `test_inputs/` cover sibling duplicates, parameter-name-only differences, `StatefulShellRoute` branches, relative routes, cross-annotation duplicates, and the sound same-class grouping case. The golden harness gained two optional companion files per input, documented in the README: `<name>.dart.options` supplies JSON builder options so each severity is exercised through the real option-parsing path, and `<name>.dart.warnings` asserts logged warnings, with an empty file asserting silence. Fixes flutter/flutter#190741 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
…er#12399) Adds build-time detection of routes that resolve to the same URL pattern. `go_router` matches the first route that fits, so a duplicated path silently makes the later route's page unreachable, and `LaterRoute().go(context)` navigates you to `EarlierRoute`'s page instead. Today nothing surfaces that until you run into the issue at runtime. Reported as a warning by default, and new `duplicate_route_paths` builder option accepts `warning` (the default), `error`, and `ignore`, so teams whose duplicates are mistakes can fail the build instead. **Before**, `go_router_builder`'s own example app built clean despite two duplicate registrations: ``` $ dart run build_runner build [INFO] Succeeded after 14s with 36 outputs ``` **After**: ``` $ dart run build_runner build W go_router_builder on lib/all_types.dart: Duplicate route path detected: DoubleRoute is declared more than once at "double-route/:requiredDoubleField". ``` That duplicate was real. `DoubleRoute` had been registered twice since flutter#2395 (2022) and `DoubleExtensionRoute` since flutter#9458 (2025). Both were unreachable, and this PR removes them. The only change to the generated output is the dead route entries. What competes for a URL namespace: - Sibling routes, with path parameters normalized, so `product/:id` and `product/:productId` are one pattern. - Routes inside shell routes and `StatefulShellRoute` branches, since those own no path of their own and their children compete with the routes around them. - Relative routes, which own their path like any other route, so `/home/details/edit` and `/home/edit` stay distinct. - Routes from separate annotations, since the generated `$appRoutes` collects them into one list. Scope is the library, `part` files included. **Testing:** five new golden inputs in `test_inputs/` cover sibling duplicates, parameter-name-only differences, `StatefulShellRoute` branches, relative routes, cross-annotation duplicates, and the sound same-class grouping case. The golden harness gained two optional companion files per input, documented in the README: `<name>.dart.options` supplies JSON builder options so each severity is exercised through the real option-parsing path, and `<name>.dart.warnings` asserts logged warnings, with an empty file asserting silence. Fixes flutter/flutter#190741 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Fixes #105876
Added support to enhanced enums introduced in dart 2.17
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.