Closed falkoschindler closed 1 year ago
I made some progress on the feature but there is still a lot to do (see list in the issue description). Noteworthy is our "requirement" to have a default page where elements are added. I only got it to work with a "shared" page (see https://github.com/zauberzeug/nicegui/commit/cc5029497b36c46feabce5d5ccc1af2ccd78c2b6). Because otherwise we need to re-evaluate the main script again and again.
My current test script:
from nicegui import ui
from uuid import uuid4
@ui.page('/individual_page')
def individual_page():
ui.label(f'your individual page with uuid {uuid4()}')
@ui.page('/shared_page', shared=True)
def shared_page():
ui.label(f'a shared page with uuid {uuid4()}')
# @ui.page('/')
# def index():
ui.link('individual page', '/individual_page')
ui.link('shared page', '/shared_page')
ui.run()
show 404 page for routes which do not exist
@rodja I guess you primarily want to avoid getting a 500 Server Error ("UnboundLocalError: local variable 'func_to_run' referenced before assignment"), as it is with the current implementation. Returning 404 would be new behavior, but probably better than redirecting to "/".
Returning 404 would be new behavior, but probably better than redirecting to "/".
@falkoschindler exactly. If it is much simpler to avoid the 500 Server Errors be redirecting to /, we could create a new issue for the 404 page. But I guess it will be an equally amount of work...
@rodja Here you stopped calling jp.justpy()
which creates the default route Route("/{path:path}", func_to_run, last=True, name='default')
. We can simply add this line to our startup()
.
remove pre-evaluation of ui.run
I think we are loosing the ability to exclude costly imports if we remove the pre-evaluation... 🤔
...and it's getting tricky to instantiate a default page without ui.page
decorator, because it should consider the title
argument in ui.run
, which is not known when creating the first elements. We'll need to create a page first and update attributes like title
, classes
, dark
etc. later.
how to exclude costly imports without pre-evaluation of
ui.run
?
exclude
played a roll in three different cases:
exclude
argument. This should also work dynamically when creating an element interactively.ui.run
. But maybe we have to require setting them before starting NiceGUI.After all, NiceGUI might be even nicer, because most JS libraries will be automatically served on demand. And there are only a few optional exclusions left as environment variables.
Update:
Although the variables "HIGHCHARTS" and "AGGRID" are already read when importing JustPy, they are only written into a dictionary template_options
for later use during a GET request. So we should be able to update this dictionary during ui.run
. This leaves only one remaining environment variable for Matplotlib.
I thought I found a solution for injecting new dependencies into running applications. But it's probably more complicated than running
let script = document.createElement("script");
script.src = "{src}";
document.body.prepend(script);
and calling page.update()
. When the script is large (like highcharts.js), it might not be fully loaded when the page update is called.
So I got a new, much simpler idea: In the rare case of dynamically introducing a new dependency, NiceGUI can simply trigger a location.reload()
. Since the dependency is now part of the template, it is automatically served and we don't have to inject it ourselves.
I think we have a memory leak for private pages:
#!/usr/bin/env python3
import gc
import os
import psutil
from nicegui import ui
@ui.page('/')
async def index_page():
process = psutil.Process(os.getpid())
gc.collect()
ui.label(f'memory: {process.memory_info().rss:,} bytes')
ui.run()
The memory usage keeps growing when refreshing the page.
Ok, the memory usage is similar for a plain JustPy app:
#!/usr/bin/env python3
import gc
import os
import justpy as jp
import psutil
def hello_world():
wp = jp.WebPage()
process = psutil.Process(os.getpid())
gc.collect()
wp.add(jp.Div(text=f'memory: {process.memory_info().rss:,} bytes'))
return wp
jp.justpy(hello_world)
See my implementation in #6. There's a line in on_disconnect_handler: self.individual_pages.pop(session_id).remove_page()
which should reclaim used memory (didn't test though)
@me21 Ok, but since I'm not holding the pages in a dictionary like you did, I'd expect the garbage collector to reclaim the memory. And especially JustPy's the bare hello-world example shouldn't have this issue.
I opened an issue over there: https://github.com/justpy-org/justpy/issues/547
Route("/{path:path}", func_to_run, doesn't exist with the path param any more.
@falkoschindler remove_page is a JustPy method, not mine. Try calling it.
Looks like an issue with starlette: https://github.com/tiangolo/fastapi/issues/4649
@me21 I tried, but it doesn't work. Apparently JustPy isn't the problem anyway, since its jp.WebPage.instances
and other lists and dictionaries don't grow.
I think the code is ready for merging and hence I created the pull request https://github.com/zauberzeug/nicegui/pull/86. Please merge it, if you agree @falkoschindler.
Ok, the pull request has been merged. But maybe we have still some work to do on main. Currently I'm struggling with the dependency-reload at https://github.com/zauberzeug/nicegui/blob/fdd580cb8493fa9653b5ea765523960a5894bff0/nicegui/routes.py#L87-L93
It seems that the page.await_javascript
never returns because the location.reload()
drops the connection. While investigating the issue I realised that the libs like tree.js etc are also working if I remove the code completely. @falkoschindler maybe we only need to reload the page if dependencies are added after the page is created, not after startup has completed?
And it seems we also have a problem with pages which are created after startup.
I've started with selenium tests with the pull request https://github.com/zauberzeug/nicegui/pull/88. The motivation was to document a simple reproduction for page builders which are created after startup (the first user.open()
calls ui.run()
internally):
https://github.com/zauberzeug/nicegui/blob/e9d775c659829c218553290b0aa7058d6a1754c2/tests/test_pages.py#L36-L44
Real world scenarios can be found in RoSys. For example the KPI page: it should only be available if the final application calls the constructor. Which is normally after the startup of NiceGUI.
Ok, the problem with page generators added after server start is fixed with https://github.com/zauberzeug/nicegui/commit/d5e8e128f4a588dd64bd1bf4fbdb2aae5e487611. I'll try to find a minimal reproduction for the dependency-reload problem I saw a few days ago.
The exception for the dependency-reload looks like this:
2022-09-21 10:51:08.985 [ERROR] nicegui/task_logger.py:54: Task raised an exception
Traceback (most recent call last):
File "/Users/rodja/Projects/nicegui/nicegui/task_logger.py", line 48, in _handle_task_result
task.result()
File "/Users/rodja/Projects/nicegui/nicegui/routes.py", line 92, in reload
await page.await_javascript('location.reload()')
File "/Users/rodja/Projects/nicegui/nicegui/page.py", line 122, in await_javascript
raise TimeoutError('JavaScript did not respond in time')
TimeoutError: JavaScript did not respond in time
It's quite simple to reproduce the error:
@ui.page('/')
def page():
ui.keyboard()
ui.run()
This will print the TimeoutError to the console after startup. I also created a selenium test (which is still on the pytest branch): https://github.com/zauberzeug/nicegui/blob/2b40c048591db4b09e49955a814f65883aae81b0/tests/test_pages.py#L72-L79
Strange. The TimeoutError can be easily avoided using run_javascript
instead of await_javascript
. But with shared=True
the page is not loading correctly and remains blank.
from nicegui import ui
@ui.page('/', shared=True)
def page():
ui.label('hello world')
ui.keyboard(on_key=lambda e: print(e))
ui.run()
Oh, I see! The get_current_view()
in this line https://github.com/zauberzeug/nicegui/blob/89c53c75d163b6c135cbb1665688231762d35984/nicegui/routes.py#L90 has a significant side effect: It is usually used to auto-create the index page. Here this messes up everything, because the index page is created explicitly with the @ui.page
decorator. I'll try to avoid using get_current_view
in this case.
@rodja I think I fixed the issue with the above-mentioned get_current_view
in route.py
. But one test is still red, unveiling a fundamental problem with dynamically creating elements.
In line https://github.com/zauberzeug/nicegui/blob/1172ff5331d96509ae09089457c48b075fb05daa/tests/test_dependencies.py#L31
it is unclear how ui.keyboard
should know where to place itself. The view_stack
is empty at this point, which causes several problems:
add_dependencies
does not know which page to reload.Element
initializer does not know which parent view to assign.get_current_view()
creates a new index page.There is another issue with reloading the page for new dependencies.
The following example should add a joystick when pressing the button:
from nicegui import ui
@ui.page('/')
def index():
def add():
with row:
ui.joystick()
row = ui.row()
ui.button('Add', on_click=add)
ui.run()
But the joystick appears only after pressing the second time. This is unexpected, but explainable: The button click adds a joystick to the DOM. The new dependency triggers a page reload. Because the page is private, the client obtains a new instance without the joystick. The next click adds a joystick again. This time, the dependency already exists, so there is no reload and the joystick remains visible.
The question is: How to add dependencies on private pages that loose their state when reloading.
Dependencies are not found when running with reload=False
:
from nicegui import ui
ui.joystick()
ui.run(reload=False)
nipple.min.js and joystick.js are not found (HTTP 404).
raise exception instead of reloading the page
This does not work, since a simple private page won't be able to add a dependency:
@ui.page('/')
def page():
ui.keyboard()
I guess we want to support such a trivial example.
I removed the automatic dependency management, because I think it's almost impossible to accomplish for private pages. When a new Vue component (e.g. "keyboard") is added to the DOM, we might be able to make connected clients load the required libraries (e.g. "keyboard.js"). But we would also need to trigger Vue to interpret the new DOM element (e.g. "
Instead I now re-introduced the exclude
argument for ui.run
that allows the user to explicitly specify a number of UI elements that should be excluded from the app. In ui.run
it's too late to impact the import process. But we can simply create routes for the corresponding libraries that yield empty response bodies. This way we save traffic without messing with the import process.
I remember the import of some Python packages on the Beaglebone Black computer was heavy (up to 50 seconds on startup). Would it be possible to exclude certain Python packages from import?
@me21
I remember the import of some Python packages on the Beaglebone Black computer was heavy (up to 50 seconds on startup). Would it be possible to exclude certain Python packages from import?
Avoiding the costly Matplotlib import is now controlled with a new environment variable "MATPLOTLIB". You can set it to "false" to disable it:
MATPLOTLIB=false python3 your_script.py
On branch https://github.com/zauberzeug/nicegui/tree/auto-context I'm experimenting with finding the right parent for new UI elements. But it easily fails with async functions like the following example. "A" and "B" should be added to separate cards, but are mixed up completely:
async def add_a():
await asyncio.sleep(0.1)
ui.label('A')
async def add_b():
await asyncio.sleep(0.1)
ui.label('B')
with ui.card():
ui.timer(1.0, add_a)
with ui.card():
ui.timer(1.1, add_b)
In https://github.com/zauberzeug/nicegui/commit/ee0fb9bdc5abcf4b86b152c9899a7e8dc4b79e85 I introduced separate view stacks per task. This solves the problem with async UI manipulation.
@rodja It looks like this epic issue is finally ready for review, integration tests in some productive contexts and release.
Fixed with release https://github.com/zauberzeug/nicegui/releases/tag/v0.9.0.
Discussed in https://github.com/zauberzeug/nicegui/discussions/61
After discussing several concepts, we tend to break with
with ui.page
contexts and move towards@ui.page
decorators. This simplifies defining page factory functions and is similar to how Flask and FastAPI define routes. In the end, pages are more like routes rather than hierarchical UI elements. Thus defining them like routes makes a lot of sense.Also, in the flavor of Flask, FastAPI or JustPy, pages should be "private" by default, i.e. without shared state between different clients. A page should only be "shared" when an optional decorator argument is set
True
.This change has interesting consequences:
with
expressions. Thus, the (internal)page_stack
is obsolete. This removes one source of confusion.ui.run
is no longer needed. Since the main script primarily defines functions, we don't need to bother about re-evaluation in case of auto-reload being activated. This complies with the traditional way of running uvicorn and removes another confusing "feature" of NiceGUI.To keep NiceGUI very accessible for new Python developers and to reduce boilerplate code for simple single-page applications, we plan to automatically create an index page if there is none. So the NiceGUI hello-world example should remain unchanged:
This will be equivalent to the following:
TODOs
page_stack
ui.page
factory)ui.page
ui.run
to default main page (which already has been created)main.py
)ui.link
andui.open
ui.run
ui.run
?reload=False