Skip to content

[go_router_builder] Adds support for enhanced enums. - #2395

Merged
auto-submit[bot] merged 15 commits into
flutter:mainfrom
Skogsfrae:main
Sep 16, 2022
Merged

auto-submit[bot] merged 15 commits into
flutter:mainfrom
Skogsfrae:main

Conversation

@Skogsfrae

@Skogsfrae Skogsfrae commented Jul 28, 2022 •

Copy link
Copy Markdown
Contributor

Fixes #105876

Added support to enhanced enums introduced in dart 2.17

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • 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.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard

Copy link
Copy Markdown

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.

@Skogsfrae Skogsfrae changed the title [WIP] fixes broken enhanced enums. Fixes broken enhanced enums. Jul 28, 2022
@Skogsfrae Skogsfrae changed the title Fixes broken enhanced enums. [go_router_builder] Fixes broken enhanced enums. Jul 28, 2022
Comment thread packages/go_router_builder/CHANGELOG.md Outdated
@loic-sharma

Copy link
Copy Markdown
Member

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.

@Skogsfrae

Copy link
Copy Markdown
Contributor Author

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.

Dart documentation

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 chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@Skogsfrae

Copy link
Copy Markdown
Contributor Author

@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

chunhtai commented Aug 9, 2022

Copy link
Copy Markdown
Contributor

@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?

Is it possible to modify the all_types to add a new route so it can navigate in between?

@Skogsfrae

Skogsfrae commented Aug 11, 2022 •

Copy link
Copy Markdown
Contributor Author

@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?

@chunhtai

chunhtai commented Aug 25, 2022 •

Copy link
Copy Markdown
Contributor

@Skogsfrae

Yes that is a good idea, please go ahead.

@chunhtai

chunhtai commented Sep 8, 2022

Copy link
Copy Markdown
Contributor

(triage) Hi @Skogsfrae, are you still planning to come back to this pr?

@Skogsfrae

Copy link
Copy Markdown
Contributor Author

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

@Skogsfrae

Copy link
Copy Markdown
Contributor Author

@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.

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

code LGTM, it looks like there are some formatting issue
https://github.com/flutter/packages/pull/2395/checks?check_run_id=8372456407

Comment thread packages/go_router_builder/CHANGELOG.md Outdated

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do these need to be real links? This could cause security issues...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are these all named *FieldField now instead of just *Field?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My fault, I use a find and replace all 😬

Comment thread packages/go_router_builder/CHANGELOG.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 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 johnpryan changed the title [go_router_builder] Fixes broken enhanced enums. [go_router_builder] Adds support for enhanced enums. Sep 15, 2022

@johnpryan johnpryan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM overall - my comments are mostly minor

@johnpryan

Copy link
Copy Markdown
Contributor

@Skogsfrae could you fix the merge conflicts?

@johnpryan johnpryan added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 16, 2022
@auto-submit
auto-submit Bot merged commit dfa1e65 into flutter:main Sep 16, 2022
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
bisor0627 pushed a commit to bisor0627/packages that referenced this pull request Jun 19, 2026
auto-submit Bot pushed a commit that referenced this pull request Sep 2, 2026
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.
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
…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.
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App needs tests p: go_router_builder

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[go_router_builder] Support Enhanced enums

4 participants