-
-
Notifications
You must be signed in to change notification settings - Fork 823
Drop auto-index pages in favour of streamlined usage of SPAs via positional root parameter in ui.run #5005
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
which is evaluated via default 404 handler to implement late-catch-all-routing
This doesn't have to be mutually exclusive with the auto-index page (I think), if we have the default root to serve the auto-index page. Then, when you pass the root that auto-index page will be ignored (add a warning). Also, it may be a bit of a pity to lose the shared client of the auto-index page. We may want to look at #4729 with a fresh pairs of eyes and keep NiceGUI competitive in the shared-client web development framework aspect as well. |
I disagree. It would be a relief for the code base. Let's move the discussion to #4472 (comment) and focus on the implementation of this PR here. |
I don't think |
@falkoschindler I do not see the point in keeping the auto-index client. It will make the code -- and behavior more complicated. As @evnchn said,
This warning will be gone or change significantly when dropping auto-index client in 3.0. |
@falkoschindler I experimented with the test infrastructure to support SPAs in d6532f8. But wonder if there is a better way... lets discuss this in person later. |
### Motivation As @alexzaech reported in #5085, opening a nested sub page which is inside a async sub page shows the 404 error page. ### Implementation Currently this PR only reproduces the problem in a screen test. A minimal reproduction: ```py from nicegui import ui @ui.page('/') @ui.page('/{_:path}') def index(): ui.sub_pages({'/': main}) async def main(): ui.sub_pages({ '/': sub_main, '/sub': sub_sub_page, }) def sub_main(): ui.link('Go to sub page', '/sub') def sub_sub_page(): ui.label('sub page') ui.run() ``` If you access `/` everything works fine. And clicking on "Go to sub page" also works. But if you directly open `/sub` you see a 404 page. The problem is this: due to the `async` keyword the execution of `main` is a coroutine which is not executed until the event loop ticks it forward. Hence the sub pages element in main is not available and https://github.com/zauberzeug/nicegui/blob/78bf3151b645573834e46aecd8e2c50977d4ac26/nicegui/elements/sub_pages.py#L111-L115 determines that the 404 should be shown. It works for the `/` because `match.remaining_path` is empty. ### Progress - [x] I chose a meaningful title that completes the sentence: "If applied, this PR will..." - [x] The implementation is complete. - [x] try to find a way to allow 404 http errors **and** evaluate async builder functions properly - [ ] verify that the approach will still work with 3.0 (especially #5005, where SPA root page is processed in 404 handler) - [x] Pytests have been added. - [x] Documentation is not necessary. --------- Co-authored-by: Falko Schindler <[email protected]>
### Motivation With the introduction of the root page concept, we need to rethink our pytest infrastructure (he `module_under_test` marker and the modular setup with a startup function) ### Implementation `main.py` is loaded via `runpy.run_path` in the fixture setup. ### Progress - [x] I chose a meaningful title that completes the sentence: "If applied, this PR will..." - [x] The implementation is complete. - [x] Pytests have been added. - [x] Documentation has been updated. --------- Co-authored-by: Falko Schindler <[email protected]>
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.
Looks good so far!
…he code (#5107) ### Motivation While working on PR #5005, we noticed that some projects like our ROS2 example might depend on the shared nature of the long-living auto-index client. If there is some kind of "event" in the data model (or hardware controller), they can simply update the UI if there is only one and exactly one client. When switching to page functions, they updating UI becomes more complicated in this scenario. Therefore we decided to move our event system from [RoSys](https://github.com/zauberzeug/rosys/) to NiceGUI. It provides a simple API to define events, subscribe arbitrary callbacks to them, and to emit/call them if the event happens. ### Implementation The core implementation is taken from RoSys. But on a second look some details have changed, partly by relaxing some assumptions or because we can make better use of NiceGUI's infrastructure. - I chose the method names "subscribe" and "subscribe_ui" instead of "register" and "register_ui". - Callbacks can still be sync or async. - Callbacks can - but don't have to - receive arguments. - Callbacks are automatically called within the same slot where they subscribed. ### Progress - [x] I chose a meaningful title that completes the sentence: "If applied, this PR will..." - [x] The implementation is complete. - [x] Merge `subscribe` and `subscribe_ui` into a single `subscribe(..., unsubscribe_on_disconnect: bool | None = None)`. - [x] Pytests have been added. - [x] Documentation has been added.
Should we not use the new root page mechanism for the ROS2 example? |
@rodja I'm not sure. |
Looks good. I just would like to see more root page use instead of page decorators in the examples. But we can also do this in a separate PR... |
Motivation
As discussed in #4964, we could streamline the the setup of NiceGUI as an SPA by providing a positional
root
parameter inui.run
. The basic pattern becomes so simple that we do not need auto-index pages anymore 🎉Implementation
This PR implements an idea form @falkoschindler and me while discussing #4964: by evaluating the
root
builder function in the FastAPI default 404 handler we do not need to register catch-all FastAPI routes at all. Thereby any other defined FastAPI routes take precedence. Only if the FastAPI routing fails we route the path to theroot
builder function passed viaui.run
. A basic (already working) example looks like this:Of course that only works when auto-index page does not register a
/
page on it's own.Progress
root
implementation:ui.sub_pages
demosroot
builder behave exactly likeui.page
or ensure via tests that both are working as expectedroot
parameter forui.run_with
client.request
is neverNone
client.shared
individual_target
# @ui.page('/')
hack in demos (not needed anymore?)auto_index_client
ui.page