Skip to content

[go_router_builder] Support extension types #9458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[go_router_builder] Support extension types #9458

wants to merge 11 commits into from

Conversation

koji-1009
Copy link
Contributor

@koji-1009 koji-1009 commented Jun 20, 2025 •
edited
Loading

fix flutter/flutter#170870

Extended type_helpers.dart to support Dart extension types.

Pre-Review Checklist

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

Footnotes

  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. ↩ ↩2 ↩3

@koji-1009 koji-1009 marked this pull request as ready for review June 20, 2025 04:07
@koji-1009 koji-1009 requested a review from chunhtai as a code owner June 20, 2025 04:07
chunhtai
chunhtai reviewed Jun 23, 2025
View reviewed changes
packages/go_router_builder/lib/src/type_helpers.dart Outdated
return '$stateValue as $castType';
}

final String parseTypeName =
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we guard against no primitive type? since this is following by a hardcode tryParse, may as well only allow certain numeric types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you are right in pointing this out. I restricted the type and handled it correctly. 61901a3

@koji-1009 koji-1009 requested a review from chunhtai June 26, 2025 13:09
chunhtai
chunhtai reviewed Jul 2, 2025
View reviewed changes
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

code lgtm, just need an example

packages/go_router_builder/README.md
@@ -334,7 +334,7 @@ class RedirectRoute extends GoRouteData {

## Type conversions

The code generator can convert simple types like `int` and `enum` to/from the
The code generator can convert simple types like `int`, `enum`, and `extension type` to/from the
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a new example in example/ folder

@chunhtai chunhtai requested a review from hannah-hyj July 2, 2025 18:36
hannah-hyj
hannah-hyj approved these changes Jul 7, 2025
View reviewed changes
Copy link
Member

@hannah-hyj hannah-hyj left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for writing the examples!

packages/go_router_builder/example/lib/all_extension_typs.dart Outdated
@@ -0,0 +1,408 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

typo in the file name: typs-> types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry! I fixed. c57f3d0

packages/go_router_builder/lib/src/type_helpers.dart Outdated
@@ -263,6 +264,94 @@ class _TypeHelperEnum extends _TypeHelperWithHelper {
bool _matchesType(DartType type) => type.isEnum;
}

/// A type helper for extension types.
/// Suppoted extension types are:
Copy link
Member

Choose a reason for hiding this comment

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

nit: Suppoted-> Supported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I fixed! c57f3d0

@koji-1009 koji-1009 requested a review from chunhtai July 10, 2025 01:39
@justinmc
Copy link
Contributor

@koji-1009 You had a failure that I just reran. If it still fails, can you take a look? Maybe you just need to push a merge commit / rebase.

@koji-1009
Copy link
Contributor Author

@justinmc
Thanks for mention. The error message is following.

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8708712639707483713/+/u/Run_package_tests/CHANGELOG_and_version_validation/stdout?format=raw

--packages-for-branch: running on branch "HEAD".
Running for all packages that have diffs relative to "d9d3191ea6cf5b2a040b3368171112c1da736483"

Changed packages: go_router_builder
[0:00] Running for packages/go_router_builder...
  3.0.1 -> 3.1.0
  When bumping the version for release, the NEXT section should be incorporated into the new version's release notes.


The following packages had errors:
  packages/go_router_builder:
    CHANGELOG.md failed validation.
See above for full details.

This “NEXT” section was described at #9453. Because of this, go_router_builder is in a difficult situation for minor version upgrades until the build issue is resolved.

I believe that the problem of build could have been solved at #9649.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hannah-hyj hannah-hyj hannah-hyj approved these changes

@chunhtai chunhtai Awaiting requested review from chunhtai chunhtai is a code owner

Assignees
No one assigned
Labels
p: go_router_builder
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[go_router_builder] Support extension types as route arguments
4 participants
@koji-1009 @justinmc @chunhtai @hannah-hyj