diff --git a/news/+static-dynamic-route-conflict.bugfix.md b/news/+static-dynamic-route-conflict.bugfix.md new file mode 100644 index 00000000000..605d8e5cbe3 --- /dev/null +++ b/news/+static-dynamic-route-conflict.bugfix.md @@ -0,0 +1 @@ +Adding a page no longer raises a spurious `RouteValueError` when a static segment lines up with another route's dynamic segment (e.g. `/posts/all/[x]` alongside `/posts/[id]`). React Router resolves such siblings in favor of the static one, so only two differently named dynamic segments at the same position conflict. The check was also order-dependent: it only tripped when the bracket-carrying route was added second. diff --git a/reflex/app.py b/reflex/app.py index e7c88222b79..a24116d42b0 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -1388,22 +1388,29 @@ def _find_route_conflict( constants.RouteRegex.DOUBLE_SEGMENT, constants.RouteRegex.DOUBLE_CATCHALL_SEGMENT, ) + replaced_new_route = replace_brackets_with_keywords(new_route) for route in existing_routes: replaced_route = replace_brackets_with_keywords(route) - for rw, r, nr in zip( + for rw, nrw, r, nr in zip( replaced_route.split("/"), + replaced_new_route.split("/"), route.split("/"), new_route.split("/"), strict=False, ): - if rw in segments and r != nr: + if r == nr: + continue + if rw in segments and nrw in segments: + # Two dynamic segments with different names cannot share + # the same position in the route tree. return route, r, nr - if rw not in segments and r != nr: - # if the section being compared in both routes is not a dynamic segment(i.e not wrapped in brackets) - # then we are guaranteed that the route is valid and there's no need checking the rest. - # eg. /posts/[id]/info/[slug1] and /posts/[id]/info1/[slug1] is always going to be valid since - # info1 will break away into its own tree. - break + # A static segment differing from the other route's segment + # (static or dynamic) splits into its own subtree, so the rest + # of the route cannot conflict. e.g. /posts/[id]/info/[slug1] + # and /posts/[id]/info1/[slug1] is always going to be valid + # since info1 will break away into its own tree; likewise + # /posts/all is a legal static sibling of /posts/[id]. + break return None def _setup_admin_dash(self): diff --git a/tests/units/test_route.py b/tests/units/test_route.py index ae1b81e89cb..1102211b320 100644 --- a/tests/units/test_route.py +++ b/tests/units/test_route.py @@ -104,6 +104,11 @@ def test_check_routes_conflict_invalid( ("/posts/[slug]/info/[[...splat]]", "/posts/[slug]/info1/[[...splat]]"), ("/posts/[slug]/info/[...slug1]", "/posts/[slug]/info1/[...slug1]"), ("/posts/[slug]/info/[...slug1]", "/posts/[slug]/info1/[...slug2]"), + # static siblings of dynamic segments are legal (static wins in React Router) + ("/posts/[slug]", "/posts/all/[x]"), + ("/posts/all/[x]", "/posts/[slug]"), + ("/[org]/dashboard", "/admin/devices/[pk]"), + ("/admin/devices/[pk]", "/[org]/dashboard"), ], ) def test_check_routes_conflict_valid(mocker: MockerFixture, app, route1, route2):