Skip to content
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.

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

[go_router_builder]: Handle invaild params #8405

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

Conversation

CaoGiaHieu-dev
Copy link

@CaoGiaHieu-dev CaoGiaHieu-dev commented Jan 10, 2025

Fixed handling of invalid parameters

Fix: flutter/flutter#160894 (comment)

Pre-launch Checklist

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

Copy link

google-cla bot commented Jan 10, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@CaoGiaHieu-dev CaoGiaHieu-dev changed the title Handle invaild params [go_router_builder]: Handle invaild params Jan 10, 2025
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.

I think the solution sounds good to me, left some comments

packages/go_router_builder/example/lib/all_types.dart Outdated Show resolved Hide resolved
@@ -79,7 +79,7 @@ extension $FamilyRouteExtension on FamilyRoute {
extension $PersonRouteExtension on PersonRoute {
static PersonRoute _fromState(GoRouterState state) => PersonRoute(
state.pathParameters['fid']!,
int.parse(state.pathParameters['pid']!),
int.parse(state.pathParameters['pid']!.toString())!,
Copy link
Contributor

Choose a reason for hiding this comment

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

can you revert the format change?

Copy link
Author

@CaoGiaHieu-dev CaoGiaHieu-dev Jan 11, 2025

Choose a reason for hiding this comment

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

currently i cannot revert it because it was using same code gen with tryPrase case .
i will look around and try to resolve this

@chunhtai chunhtai requested a review from hannah-hyj January 10, 2025 19:45
@@ -345,7 +389,7 @@ abstract class _TypeHelperWithHelper extends _TypeHelper {
'${helperName(paramType)})';
}
return '${helperName(paramType)}'
'(state.${_stateValueAccess(parameterElement, pathParameters)})';
'(state.${_stateValueAccess(parameterElement, pathParameters)}.toString())!';
Copy link
Contributor

Choose a reason for hiding this comment

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

is this possible to not be a string?

Copy link
Author

@CaoGiaHieu-dev CaoGiaHieu-dev Jan 18, 2025

Choose a reason for hiding this comment

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

@chunhtai i dont think it can because _stateValueAccess will get data from a map <String,String> that will be a nullable string . i have to make to .toString() to make sure it a not null string to able using tryParse or parse

param: state.uri.queryParametersAll['param']?.map(int.parse) ??
param: (state.uri.queryParametersAll['param']
?.map(int.parse)
?.where((e) => e != null)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ? after the first ?.map may cause lint error as it is not needed?

Copy link
Author

@CaoGiaHieu-dev CaoGiaHieu-dev Jan 18, 2025

Choose a reason for hiding this comment

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

it wont because it not different with previous code . the only thing different is i wrap it to () and add ?.where((e) => e != null)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[go_router_builder] Cannot handle query parameter when it cannot parse
2 participants