[go_router_builder] Support custom types

[ValentinVignal]

Closes flutter/flutter#112152
Closes flutter/flutter#110781

Pre-Review Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter.
• 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 linked to 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 I have commented below to indicate which version change exemption this PR falls under¹.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under¹.
• I updated/added any relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under¹.
• All existing and new tests are passing.

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

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. 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.

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. ↩ ↩² ↩³

[ValentinVignal]

It needs #11067 to be merged first

[gemini-code-assist]

Code Review

This pull request adds support for custom types as query parameters by allowing custom encoder, decoder, and compare functions in the TypedQueryParameter annotation. The changes are well-implemented across the builder logic, tests, and example code. I have a few suggestions to improve robustness in the example and enhance correctness and clarity in the builder logic.

[gemini-code-assist]

The check if (name == 'new') for an unnamed constructor seems incorrect. According to the analyzer package documentation, the name of an unnamed constructor is an empty string. Using 'new' might be unreliable and could fail if a class has a named constructor called new.

Please change this to check for an empty string to correctly identify the default constructor.

Suggested change

	if (name == 'new') {
	if (name.isEmpty) {

[gemini-code-assist]

The decode method is not robust against malformed input and could lead to runtime exceptions. For example, if the value string doesn't contain a comma, accessing parts[1] will throw a RangeError. If the second part is not a valid integer, int.parse will throw a FormatException.

Since this is an example file, it's a good opportunity to demonstrate best practices for parsing. Consider adding checks for the number of parts and using int.tryParse for safer integer conversion.

  static CustomParameter? decode(String? value) {
    if (value == null) {
      return null;
    }
    final List<String> parts = value.split(',');
    if (parts.length != 2) {
      return null;
    }
    final int? valueInt = int.tryParse(parts[1]);
    if (valueInt == null) {
      return null;
    }
    return CustomParameter(
      valueString: parts[0],
      valueInt: valueInt,
    );
  }

[ValentinVignal]

This is true. But that's only an example. So I'd think the current code is okay. Let me know if you think otherwise

[gemini-code-assist]

This part of the code seems to contain some redundant logic, likely from a refactoring.

• The condition annotatedDecoder != null on line 113 will always be false because of the early return on lines 99-109 if annotatedDecoder is not null.
• Consequently, var decoder = annotatedDecoder; on line 114 will always assign null to decoder.
• This makes the if (decoder != null) block on line 116 dead code.

For clarity and maintainability, this logic could be simplified by removing the redundant checks and the dead code path.

[chunhtai]

seems like this may be a breaking change?

[ValentinVignal]

I considered it a non-breaking change because existing usages of @TypedQueryParameter(name: 'my-name') will now be interpreted as @TypedQueryParameter<dynamic>S(name: 'my-name') which is okay. <T> only matters for compare, decode, encode.

But yes, ultimately, if the linter rules inference_failure_on_instance_creation is enabled, this will now create a lint that needs to be fixed.

What do you think? Should we flag it as a breaking change, and is it an acceptable one? Or is it okay to not flag it as a breaking change?

[chunhtai]

nit: use a for loop for readability

[ValentinVignal]

Sure, I've changed it into a for loop in Use loop for readability

[chunhtai]

Is there a guard to ensure encode, decode, compare must all be provided or none should be provided

[ValentinVignal]

I didn’t add a guard here because I don’t think it’s strictly necessary:

• If a custom type isn’t supported, the builder already fails and suggests adding encode/decode.
• compare only matters when there’s a default value on a non‑supported type.
• Users can annotate supported types (e.g. String) to override encode while still relying on the default decode from go_router_builder. I’m not sure what the practical use case for that would be, but it’s technically valid.

So there are scenarios where encode, decode, and compare don’t all need to be provided together. Given that, do you still want me to enforce a guard requiring encode and decode to always be paired?