[go_router_builder] Change mixin name #9626

StevenSorial · 2025-07-14T20:28:22Z

This PR fixes workflows that relied on putting child routes in files different than parent files, which resulted in the (private) mixin being generated in a different file than the route itself.

To avoid releasing a new major version in such a short period, this PR makes the mixin public only if needed. Admittedly makes the behavior somewhat unexpected for the user. @chunhtai @hannah-hyj I will leave that decision for you, whether that's ok or I should make it always public and release a major version instead.

Since this change just fixes a use-case that already didn't compile in 3.x.x, this PR doesn't bump the major version.

Pre-Review Checklist

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

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

ManuelRauber · 2025-07-16T13:35:47Z

Works for me for the moment. :)

chunhtai

change makes sense to me. we should also update the readme

StevenSorial · 2025-08-05T22:12:20Z

change makes sense to me. we should also update the readme

@chunhtai, so are you ok with the approach where the mixin will be public /private based on the routes configuration? or does it seem too inconsistent for you?

If you are ok with the current approach, I will add a note in the readme

chunhtai

I think let's just make this a breaking change. I see no reason to not make this a public mixin. WDYT?

chunhtai · 2025-08-07T21:49:31Z

packages/go_router_builder/lib/src/route_config.dart

+  String get _mixinName {
+    // If the routeDataClass is in a different file, we need to make the mixin public
+    final Uri routeUri = routeDataClass.source.uri;
+    return routeUri != targetUri ? '\$$_className' : '_\$$_className';


thinking about it more, let's just do a breaking change and make this a public class. having dealing with cases in both seems counter intuitive since we are maintaining a versioned package.

StevenSorial · 2025-08-07T22:47:07Z

I think let's just make this a breaking change. I see no reason to not make this a public mixin. WDYT?

@chunhtai My ideal scenario would be to improve the Generator to be able track the routes in different files without an annotation in every route file

// parent_route.dart

@TypedGoRoute<ParentRoute>(
  path: '/',
  routes: [TypedGoRoute<SubRoute>(path: 'sub')],
)
class ParentRoute extends GoRouteData with _$ParentRoute {
}

// sub_route.dart

class SubRoute extends GoRouteData with _$SubRoute {}

in my ideal scenario, the generator would be able to generate parent_route.g.dart containing _$ParentRoute mixin, and sub_route.g.dart containing _$SubRoute mixin.

I looked into this, and it seems that it will require so much work and possibly getting rid of SharedPartBuilder. Is that assumption correct?

mustafamasri1 · 2025-08-10T18:37:23Z

any update?

StevenSorial · 2025-08-14T20:29:45Z

@chunhtai could you please answer my last question?

mustafamasri1 · 2025-08-15T11:34:44Z

I think let's just make this a breaking change. I see no reason to not make this a public mixin. WDYT?

@chunhtai My ideal scenario would be to improve the Generator to be able track the routes in different files without an annotation in every route file
// parent_route.dart

@TypedGoRoute<ParentRoute>(
  path: '/',
  routes: [TypedGoRoute<SubRoute>(path: 'sub')],
)
class ParentRoute extends GoRouteData with _$ParentRoute {
}

// sub_route.dart

class SubRoute extends GoRouteData with _$SubRoute {}
in my ideal scenario, the generator would be able to generate parent_route.g.dart containing _$ParentRoute mixin, and sub_route.g.dart containing _$SubRoute mixin.

I looked into this, and it seems that it will require so much work and possibly getting rid of SharedPartBuilder. Is that assumption correct?

This idea is already created, check this comment in #122258

chunhtai · 2025-08-15T22:36:28Z

My concern is that developer need to be aware of 2 option, _$Route and $Route and remember when the _$Route is allow and when it is not. We should just give them one choice, and it seems $Route for all cases?

in my ideal scenario, the generator would be able to generate parent_route.g.dart containing _$ParentRoute mixin, and sub_route.g.dart containing _$SubRoute mixin.

That will work as well

I looked into this, and it seems that it will require so much work and possibly getting rid of SharedPartBuilder. Is that assumption correct?

How so?

mustafamasri1 · 2025-08-23T06:20:26Z

any update?

chunhtai

LGTM, can you write a breaking change doc?

StevenSorial · 2025-08-25T22:59:20Z

LGTM, can you write a breaking change doc?

Sure. How can I do that?

chunhtai · 2025-08-26T16:17:18Z

You can make a doc similar to this one
https://docs.google.com/document/d/1E4B5nMDEvsqQiCo-Ki9LohocLFC88X398-AiLXRQhwU/edit?tab=t.0
share with everyone for view only
Add an golink entry in https://github.com/flutter/website/blob/main/firebase.json
add the breaking change doc to Readme of go_router_builder similar to what we did in go_router

breaking change for go_router_builder v4.0.0 Preparing for [#9626](flutter/packages#9626) ## Presubmit checklist - [x] If you are unwilling, or unable, to sign the CLA, even for a _tiny_, one-word PR, please file an issue instead of a PR. - [x] If this PR is not meant to land until a future stable release, mark it as draft with an explanation. - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style)—for example, it doesn't use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first-person pronouns). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer.

chunhtai

LGTM

auto-submit · 2025-08-27T19:53:54Z

autosubmit label was removed for flutter/packages/9626, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

StevenSorial · 2025-08-28T18:18:55Z

@chunhtai The submit label got removed

chunhtai · 2025-08-28T22:13:37Z

waiting for another review

hannah-hyj

LGTM

flutter/packages@a3f09e5...3db5adc 2025-08-29 [email protected] [go_router_builder] Change mixin name (flutter/packages#9626) 2025-08-29 [email protected] [go_router] Add state restoration topic to documentation (flutter/packages#9867) 2025-08-29 [email protected] [camera_avfoundation] Implementation swift migration - part 12 (flutter/packages#9781) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

github-actions bot added the p: go_router_builder label Jul 14, 2025

StevenSorial mentioned this pull request Jul 14, 2025

[go_router] [go_router_builder] New "mixin" feature in go_router_builder 3.0.0 breaks separated routes logic flutter/flutter#170650

Closed

StevenSorial marked this pull request as ready for review July 19, 2025 20:25

StevenSorial requested a review from chunhtai as a code owner July 19, 2025 20:25

chunhtai reviewed Jul 30, 2025

View reviewed changes

chunhtai reviewed Aug 7, 2025

View reviewed changes

StevenSorial requested a review from chunhtai August 23, 2025 09:35

chunhtai approved these changes Aug 25, 2025

View reviewed changes

chunhtai requested a review from hannah-hyj August 25, 2025 16:48

StevenSorial mentioned this pull request Aug 26, 2025

Add go-router-builder-v4-breaking-changes link flutter/website#12361

Merged

4 tasks

StevenSorial requested a review from chunhtai August 27, 2025 17:35

chunhtai approved these changes Aug 27, 2025

View reviewed changes

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 27, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 27, 2025

StevenSorial added 3 commits August 29, 2025 00:01

change mixin name

179845f

Update example tests

ad93ba4

Update custom tests

fda5942

StevenSorial added 3 commits August 29, 2025 00:01

Update the changelog and pubspec

a5c0ef7

Update README

e31110a

Add breaking change link

e1e5888

hannah-hyj approved these changes Aug 29, 2025

View reviewed changes

hannah-hyj added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 29, 2025

auto-submit bot merged commit 3db5adc into flutter:main Aug 29, 2025
80 checks passed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 1, 2025

3db5adc [go_router_builder] Change mixin name (flutter/packages#9626)

124ad73

engine-flutter-autoroll mentioned this pull request Sep 1, 2025

Roll Packages from a3f09e5705d7 to 3db5adc6a64b (3 revisions) flutter/flutter#174799

Merged

StevenSorial deleted the public-mixin branch September 2, 2025 17:11

[go_router_builder] Change mixin name #9626

[go_router_builder] Change mixin name #9626

Uh oh!

Conversation

StevenSorial commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

ManuelRauber commented Jul 16, 2025

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

StevenSorial commented Aug 5, 2025

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

StevenSorial commented Aug 7, 2025

Uh oh!

mustafamasri1 commented Aug 10, 2025

Uh oh!

StevenSorial commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mustafamasri1 commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunhtai commented Aug 15, 2025

Uh oh!

mustafamasri1 commented Aug 23, 2025

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

StevenSorial commented Aug 25, 2025

Uh oh!

chunhtai commented Aug 26, 2025

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Aug 27, 2025

Uh oh!

StevenSorial commented Aug 28, 2025

Uh oh!

chunhtai commented Aug 28, 2025

Uh oh!

hannah-hyj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

StevenSorial commented Jul 14, 2025 •

edited

Loading

StevenSorial commented Aug 14, 2025 •

edited

Loading

mustafamasri1 commented Aug 15, 2025 •

edited

Loading