diff --git a/integration/test_dynamic_routes.py b/integration/test_dynamic_routes.py index c76993a24..6adb41237 100644 --- a/integration/test_dynamic_routes.py +++ b/integration/test_dynamic_routes.py @@ -169,8 +169,7 @@ def test_on_load_navigate( link = driver.find_element(By.ID, "link_page_next") assert link - exp_order = [f"/page/[page-id]-{ix}" for ix in range(10)] - + exp_order = [f"/page/[page_id]-{ix}" for ix in range(10)] # click the link a few times for ix in range(10): # wait for navigation, then assert on url @@ -190,13 +189,13 @@ def test_on_load_navigate( # manually load the next page to trigger client side routing in prod mode if is_prod: exp_order += ["/404-no page id"] - exp_order += ["/page/[page-id]-10"] + exp_order += ["/page/[page_id]-10"] with poll_for_navigation(driver): driver.get(f"{dynamic_route.frontend_url}/page/10/") poll_for_order(exp_order) # make sure internal nav still hydrates after redirect - exp_order += ["/page/[page-id]-11"] + exp_order += ["/page/[page_id]-11"] link = driver.find_element(By.ID, "link_page_next") with poll_for_navigation(driver): link.click() @@ -205,7 +204,7 @@ def test_on_load_navigate( # load same page with a query param and make sure it passes through if is_prod: exp_order += ["/404-no page id"] - exp_order += ["/page/[page-id]-11"] + exp_order += ["/page/[page_id]-11"] with poll_for_navigation(driver): driver.get(f"{driver.current_url}?foo=bar") poll_for_order(exp_order) @@ -220,7 +219,7 @@ def test_on_load_navigate( # browser nav should still trigger hydration if is_prod: exp_order += ["/404-no page id"] - exp_order += ["/page/[page-id]-11"] + exp_order += ["/page/[page_id]-11"] with poll_for_navigation(driver): driver.back() poll_for_order(exp_order) @@ -235,7 +234,7 @@ def test_on_load_navigate( # hit a page that redirects back to dynamic page if is_prod: exp_order += ["/404-no page id"] - exp_order += ["on_load_redir-{'foo': 'bar', 'page_id': '0'}", "/page/[page-id]-0"] + exp_order += ["on_load_redir-{'foo': 'bar', 'page_id': '0'}", "/page/[page_id]-0"] with poll_for_navigation(driver): driver.get(f"{dynamic_route.frontend_url}/redirect-page/0/?foo=bar") poll_for_order(exp_order) diff --git a/reflex/app.py b/reflex/app.py index 9df27723e..579f4e16e 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -355,7 +355,10 @@ class App(Base): assert isinstance( component, Callable ), "Route must be set if component is not a callable." - route = component.__name__ + # Format the route. + route = format.format_route(component.__name__) + else: + route = format.format_route(route, format_case=False) # Check if the route given is valid verify_route_validity(route) @@ -388,9 +391,6 @@ class App(Base): if script_tags: component.children.extend(script_tags) - # Format the route. - route = format.format_route(route) - # Add the page. self._check_routes_conflict(route) self.pages[route] = component diff --git a/reflex/utils/format.py b/reflex/utils/format.py index 2d3a2f328..e373331cf 100644 --- a/reflex/utils/format.py +++ b/reflex/utils/format.py @@ -164,6 +164,21 @@ def to_title_case(text: str) -> str: return "".join(word.capitalize() for word in text.split("_")) +def to_kebab_case(text: str) -> str: + """Convert a string to kebab case. + + The words in the text are converted to lowercase and + separated by hyphens. + + Args: + text: The string to convert. + + Returns: + The title case string. + """ + return to_snake_case(text).replace("_", "-") + + def format_string(string: str) -> str: """Format the given string as a JS string literal.. @@ -202,18 +217,20 @@ def format_var(var: Var) -> str: return json_dumps(var.full_name) -def format_route(route: str) -> str: +def format_route(route: str, format_case=True) -> str: """Format the given route. Args: route: The route to format. + format_case: whether to format case to kebab case. Returns: The formatted route. """ - # Strip the route. route = route.strip("/") - route = to_snake_case(route).replace("_", "-") + # Strip the route and format casing. + if format_case: + route = to_kebab_case(route) # If the route is empty, return the index route. if route == "": diff --git a/tests/test_route.py b/tests/test_route.py new file mode 100644 index 000000000..782e54218 --- /dev/null +++ b/tests/test_route.py @@ -0,0 +1,71 @@ +import pytest + +from reflex import constants +from reflex.route import catchall_in_route, get_route_args, verify_route_validity + + +@pytest.mark.parametrize( + "route_name, expected", + [ + ("/users/[id]", {"id": constants.RouteArgType.SINGLE}), + ( + "/posts/[postId]/comments/[commentId]", + { + "postId": constants.RouteArgType.SINGLE, + "commentId": constants.RouteArgType.SINGLE, + }, + ), + ], +) +def test_route_args(route_name, expected): + assert get_route_args(route_name) == expected + + +@pytest.mark.parametrize( + "route_name", + [ + "/products/[id]/[id]", + "/posts/[postId]/comments/[postId]", + ], +) +def test_invalid_route_args(route_name): + with pytest.raises(ValueError): + get_route_args(route_name) + + +@pytest.mark.parametrize( + "route_name,expected", + [ + ("/events/[year]/[month]/[...slug]", "[...slug]"), + ("pages/shop/[[...slug]]", "[[...slug]]"), + ], +) +def test_catchall_in_route(route_name, expected): + assert catchall_in_route(route_name) == expected + + +@pytest.mark.parametrize( + "route_name", + [ + "/products", + "/products/[category]/[...]/details/[version]", + "[...]", + "/products/details", + ], +) +def test_verify_valid_routes(route_name): + verify_route_validity(route_name) + + +@pytest.mark.parametrize( + "route_name", + [ + "/products/[...]/details/[category]/latest", + "/blog/[...]/post/[year]/latest", + "/products/[...]/details/[...]/[category]/[...]/latest", + "/products/[...]/details/category", + ], +) +def test_verify_invalid_routes(route_name): + with pytest.raises(ValueError): + verify_route_validity(route_name) diff --git a/tests/test_utils.py b/tests/test_utils.py index 5b16af198..552b046e5 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -250,23 +250,31 @@ def test_is_generic_alias(cls: type, expected: bool): @pytest.mark.parametrize( - "route,expected", + "route,format_case,expected", [ - ("", "index"), - ("/", "index"), - ("custom-route", "custom-route"), - ("custom-route/", "custom-route"), - ("/custom-route", "custom-route"), + ("", True, "index"), + ("/", True, "index"), + ("custom-route", True, "custom-route"), + ("custom-route", False, "custom-route"), + ("custom-route/", True, "custom-route"), + ("custom-route/", False, "custom-route"), + ("/custom-route", True, "custom-route"), + ("/custom-route", False, "custom-route"), + ("/custom_route", True, "custom-route"), + ("/custom_route", False, "custom_route"), + ("/CUSTOM_route", True, "custom-route"), + ("/CUSTOM_route", False, "CUSTOM_route"), ], ) -def test_format_route(route: str, expected: bool): +def test_format_route(route: str, format_case: bool, expected: bool): """Test formatting a route. Args: route: The route to format. + format_case: Whether to change casing to snake_case. expected: The expected formatted route. """ - assert format.format_route(route) == expected + assert format.format_route(route, format_case=format_case) == expected @pytest.mark.parametrize(