[go_router_builder] Support default values for route parameters #2826
Conversation
| final String parameterName = param.name; | ||
| String condition = '$parameterName != null'; | ||
| if (param.hasDefaultValue) { | ||
| condition += '&& $parameterName != ${param.defaultValueCode!}'; |
There was a problem hiding this comment.
How do you tell the difference between a parameter with default value but explicitly set to null? in that case the URL it generates will always represent parameter with default value
There was a problem hiding this comment.
I was considering the type being non-nullable when a default value is set. But you are right, this is not always true. I assume we could allow / need ?param=null in the URL
But this might conflict if param is a String?. Does ?param=null mean String? param = null or String? param = 'null' ?
I believe we can only support default parameters when the type is not nullable?
What do you think?
There was a problem hiding this comment.
yes, I can't think of other way to support nullable field without som caveats. Only allow non-nullable field to have default value SGTM
There was a problem hiding this comment.
So what do you think about the current implementation? Do you want me to change something?
There was a problem hiding this comment.
The more I think about this the more I think we probably shouldn't add this api. cc @johnpryan WDYT
There was a problem hiding this comment.
you can handle the default value in build method.
If we really want to do this, we need some kind of warning or even linting error if developer assign null to the nullable parameter.
There was a problem hiding this comment.
If we really want to do this, we need some kind of warning or even linting error if developer assign null to the nullable parameter.
But didn't we say the parameter would be non-nullable? (#2826 (comment))?
There was a problem hiding this comment.
yes but what i meant was that we need something to warn developer about it.
There was a problem hiding this comment.
I can try to implement a warning or an error if the type is nullable
There was a problem hiding this comment.
What do you think about 🔊 Throw an error when a default value is used with a nullable type?
Now it throws an error if a default value is used with a nullable type
|
@chunhtai Is there anything else you want me to do about this? |
…route-parameters # Conflicts: # packages/go_router_builder/CHANGELOG.md # packages/go_router_builder/pubspec.yaml
…upport-default-values-in-route-parameters
|
@chunhtai Ping from triage: what's the status of this review? |
chunhtai
left a comment
chunhtai
left a comment
There was a problem hiding this comment.
sorry for the late reply, just come back from vacation. I think this looks good now. Can you add update the readme to include this new functionality?
|
@chunhtai I've modified the README in Update README. Tell me if that's good enough for you |
packages/go_router_builder/README.md
Outdated
| } | ||
| ``` | ||
|
|
||
| A query parameter equal to its default value is not included in the location. |
There was a problem hiding this comment.
| A query parameter equal to its default value is not included in the location. | |
| A query parameter equals to its default value is not included in the location. |
There was a problem hiding this comment.
Actually "equal" is not the verb in the sentence, the verb is "is"
"A query parameter equals to its default value" is the subject of the sentence.
So I guess it should be "equal" ?
There was a problem hiding this comment.
ah sorry, should it be this then? "A query parameter that equals to its default value"
There was a problem hiding this comment.
Sure, I've changed it
chunhtai
added
the
autosubmit
4 participants
Closes flutter/flutter#114406
This PR adds the support of default values in the route constructors.
If the URL/location doesn't have a set value for the field, it uses the default value. If the value of the field is equal to the default value, it doesn't include it in the location/URL.
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.