-
-
Notifications
You must be signed in to change notification settings - Fork 7
Rewrite ReactPy-Router #30
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
Conversation
…ctoring, and better resolver base class
While the tests may have passed on this PR, the tests are still extremely flakey. The tests being flakey appears to already be a problem on Will need a follow up PR to resolve this. |
Will take a look this weekend. |
Fixed the flakey tests by adding some click delays. I saw this same problem on This is technically a problem with ReactPy core, but I don't see a way we could fix this. As a note, this is probably the same reason why tests are flakey on core's CI as well. |
# FIXME: This component currently works in a "dumb" way by trusting that ReactPy's script tag \ | ||
# properly sets the location due to bugs in ReactPy rendering. | ||
# https://github.com/reactive-python/reactpy/pull/1224 | ||
current_path = use_connection().location.pathname | ||
|
||
@event(prevent_default=True) | ||
def on_click(_event: dict[str, Any]) -> None: | ||
pathname, search = to.split("?", 1) if "?" in to else (to, "") | ||
if search: | ||
search = f"?{search}" | ||
|
||
# Resolve relative paths that match `../foo` | ||
if pathname.startswith("../"): | ||
pathname = urljoin(current_path, pathname) | ||
|
||
# Resolve relative paths that match `foo` | ||
if not pathname.startswith("/"): | ||
pathname = urljoin(current_path, pathname) | ||
|
||
# Resolve relative paths that match `/foo/../bar` | ||
while "/../" in pathname: | ||
part_1, part_2 = pathname.split("/../", 1) | ||
pathname = urljoin(f"{part_1}/", f"../{part_2}") | ||
|
||
# Resolve relative paths that match `foo/./bar` | ||
pathname = pathname.replace("/./", "/") | ||
|
||
set_location(Location(pathname, search)) | ||
|
||
attrs["onClick"] = on_click | ||
|
||
return html._(html.a(attrs, *children), html.script(link_js_content.replace("UUID", uuid_string))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this solution isn't ideal, but I'd rather let this PR move forward with this short-term solution so I can continue tackling the other issues in reactpy-router
.
I'll need to do some fixes on ReactPy core to allow me to uncomment the client-side approach from def link()
.
Description
Fixes a boatload of rendering bugs and aligns the API with react router.
Changed
use_query
touse_search_params
.simple.router
tobrowser_router
.SimpleResolver
toStarletteResolver
.CONVERSION_TYPES
toCONVERTERS
.*
to{name:any}
.reactpy_router.link
to be a server-side component.reactpy_router
.Added
Resolver
base class.Fixed
link
elements could not have@component
type children.Checklist
Please update this checklist as you complete each item:
By submitting this pull request I agree that all contributions comply with this project's open source license(s).