-
-
Notifications
You must be signed in to change notification settings - Fork 761
Introduce ui.sub_pages to allow implementing single page applications (SPAs) #4821
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
base: main
Are you sure you want to change the base?
Conversation
with ui.sub_pages(...)
@Alyxion and @evnchn what do you think about this approach? I already talked with @falkoschindler last week, but I would much appreciate your feedback. Where do you see problems or inherent limitations? Especially compared to #2811. |
@rodja Thanks for inviting me on to this journey. Overall, this PR, #2811 (the original SPA), #4552 (Project ACID) and #4719 (Project BASE) should be viewed together, as at the end of the day, they achieve building a single-page application, hereby defined as a page which does NOT reload but loads-in new content nonetheless. Parallels between Project ACID and this PR - re-implmementing the HTTP layerWhy I am saying this, is because I think @rodja you mentioned that you have to implement "async support and path parameters", which sounds very similar to back in #4552 (project ACID), where I re-implemented client initialization with async support (but left out path parameters since I didn't got around to do it) when the ACID maneuver occurs, and we'd like to spin up a client to match the browser. In the end, we came to the conclusion that we'd be re-inventing the HTTP layer and all the middleware / path parameters / pydantic classes that FastAPI offers, and we did not pursue. So, high-level speaking, although this PR is obviously way less cursed than #4552 (project ACID), I fear that there will equally be a high maintenance burden in "async support and path parameters" Leveraging the existing HTTP layer with Project BASEThen, if I am not mistaken, that in #4719, since we are making a HTTP request to the server for the latest content, that we do not need to re-implement "async support and path parameters", since we are leveraging the original functionality in That PR has its own problems, but high-level speaking, the maintenance burden may be lower. Can we move forward with the Original SPA PR, with this new mindset in mind?Seems like this PR was created to supersede the original SPA PR, but given the above concerns over re-implementing the HTTP layer and maintenance burden, it may or may not be a good idea. Though the original SPA PR is also very long and bulky, but at least we see, @rodja I'd recommend to check out #4719 for a fresh perspective on how this SPA problem may be solved, now that a bug has been resolved in that PR, which made it work in the entire NiceGUI documentation (though tests are failing for some version for some reason) |
First of all thank you very much helping to drive the implementation forward @rodja. As already discussed in the video call I think yielding data can easily be replaced by storing in within the client storage for the most part - if calls are deeply nested - or by passing it directly to the sub pages as in your example. As the client storage is not context aware this requires manual cleanup in some scenarios though where as the yield apporach did this automatically when a sub page was left which yielded objects. Async is cruicial in my opinion, as NiceGUI is basically a single-threaded app no serious apps with mulitple users could be realized without page and the sub page being declarable as async. The samples above didn't contain code behind the sub page declaration, but I assume thats as well possible as code before? Regarding the general concept at least for us the actual approach with outlet and outlet.view allowed far cleaner structuring of larger apps in my opinion. It behaved like an ApiRouter in FastAPI or a Blueprint in Flask where you could define the outlet in a shared, central module file and then all "plugins" could import the outlet and register themselves to it... like in an APIRouter. With this approach now thats not possibl anymore. At the point of implementation the main page would already need to know all children and/or do some workaround everytime, e.g. like a registry where all sub pages had to enlist themselves so that the main page could then register them. Don't like it to be honest. Preferred the "router style". What I like though is that you used the same pattern for the declaration of sub pages as FastAPI, e.g. |
Response to @evnchn:
Exactly.
No. In my opinion, the url-path-handling is crucial to SPAs. Especially in NiceGUI where we offer a "backend-first" approach and handle the web-development details internally.
This PR's approach utilizes
True. I really like the concept and want to see project BASE (#4719) merged. Its remarkable elegant, small and brings a great user improvement. But it is not solution for SPAs in my opinion. In an SPA you want to only load once (in this its similar to BASE), provide browser navigation (eg. pushstate/popstate) and most importantly only update the changed content via efficient backend communication.
I tried. Which led me to this PR which avoids the reimplementation of full-fledged routing. Response to @Alyxion:
Yes,
True. Outlet, ApiRouter, Blueprint etc. are a lot more decentral by allowing self-registration. This "Inversion of Control" IoC is very modular and provides a loose coupling between main file and drop-in modules. But this startup-magic comes at the cost of hidden side-effects where an simple import mutates the global app state and it can be quite hard to understand the routing (because it is scattered through-out the project). I'm leaning towards the central composition root architecture because it's simpler to use in small code bases. And -- as you said -- it can be extended to allow IoC by adding a registration mechanism for applications which require a more modular approach. |
Interesting. I think we can define a Single-Page Appication as follows:
This definition shall be useful across all PRs attempting to take a shot at the problem. Let us bear this in mind. I have stuff to talk about how project BASE can achieve SPA, but I will put it at that PR's comment so avoid muddying waters. So the way I understand it, in this PR we're dragging on one @ui.page('/')
@ui.page('/{_:path}') # tells FastAPI/Starlette that all path's should land on our SPA
def index():
ui.label('some text before main content')
# some state-dependent element
ui.sub_pages({'/': child, '/other': child2, '/mutate': child3})
def child():
ui.link('goto other', '/other')
ui.link('goto mutate', '/mutate')
def child2():
ui.link('goto main', '/')
def child3():
# some stuff which modifies the state...
ui.link('goto main', '/') If I go from |
I do not quite understand your concern @evnchn. Can you provide an example with problematic code? Above you only placed comments which I do not know how to fill.
|
@falkoschindler you can start with a first review if you find the time. I'll look into creating the documentation. |
Oh, sorry @evnchn. I totally missed your comment
I do not see the problem. If you have pages which change a storage value -- of course they will alter the internal state. That behaviour is not specific to sub pages: app.storage.general['some_value'] = 0
@ui.page('/')
def index():
ui.label().bind_text_from(app.storage.general, 'some_value')
ui.link('goto mutate', '/mutate')
@ui.page('/mutate')
def mutate():
app.storage.general['some_value'] += 1
ui.link('goto main', '/') The main difference is that you can do such things with |
Motivation
We already have an example on how to hand-build a Singe Page Application (SPA). Then, @Alyxion worked out the feature-rich but super large pull request #2811 to integrate SPAs directly into NiceGUI. While working/reviewing the code I came up with this alternative pull request which allows the following SPA syntax:
Sub pages can also be nested. But only one
ui.sub_pages
element should be used on each hierarchy level.If a sub page needs to modify content from it's parent, the object is simply passed via lambda:
Same as with most of NiceGUI's callable definitions this also works for async functions:
Paths to sub pages can contain parameters:
Key differences compared to #2811 (@Alyxion's SPA PR)
yield
with a new kind of object storage)ui.sub_pages
instead of decorators, routers etc.ui.sub_pages
implementation (which looks at the url path to determine which content builder needs to be picked)ui.sub_pages
and override 'show()')Implementation
I tried to create a PR as small as possible. But adding nested sub pages, async support and path parameters made it a lot more complicated than I originally thought. It is also still far from done.
If you want to dive into this, I recommend looking at
test_sub_pages.py
for examples on how theui.sub_pages
api works.Progress
ui.navigate.to
work with sub pages