Milad-Akarie / auto_route_library

Flutter route generator
MIT License
1.57k stars 394 forks source link

StackRouter.isPathActive returns false if path argument contains regex characters #1710

Open Adam-Langley opened 1 year ago

Adam-Langley commented 1 year ago

StackRouter.isPathActive incorrectly returns false if the path we are searching for contains regex special characters.

For example:

Given the currently active path:

/Home/Items/Item/123+456

context.router.isPathActive('Item/123+456') should return true - however it returns false, because the implementation is using regex matches instead of simple 'contains' testing.

(taken from auto_router/navigation_history_base.dart)

  /// Whether this path-pattern is in the visible urlState-segments
  bool isPathActive(String pattern) {
    return RegExp(pattern).hasMatch(urlState.path);
  }

I am not prescribing that the implementation should or shouldnt use regular expression testing, just that as currently implemented, it's not weakest-link obvious :) that the developer needs to know to regex escape any paths before passing to 'isPathActive' to work correctly.

Thank you

Milad-Akarie commented 1 year ago

Hey @Adam-Langley so what do you suggest? Pass a flag to whether look up paths as string segments?

Adam-Langley commented 1 year ago

Hey @Milad-Akarie Maybe I'm being pedantic.

The signature in RoutingController is isPathActive(String path)

which calls NavigationHistory, whose signature is isPathActive(String pattern)

perhaps just naming both arguments 'pattern' might make it more obvious?

Adam-Langley commented 1 year ago

To get more specific, what I was doing was testing to see if a particular path with ID was active - e.g /customers/5

and that Route has the path template "/customers/:id"

I feel like perhaps I should be using the 'isRouteDataActive' method, because I want to not only test the activation of a route, but also specific data on that route...

I can't for the life of me see how to correctly create a RouteData object to pass to it.

I would prefer auto_route to template my argument into the path template (i.e, to the regex replace), rather than duplicate that, and have multiple places to change my code if I change the route paths at any time down the road...

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions