Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Django CSS/JS de-duplication #226

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Add Django CSS/JS de-duplication #226

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5e8e593
functional only-once implementation
Archmonger Feb 6, 2024
fdf399e
cleanup
Archmonger Feb 6, 2024
1225951
css test component
Archmonger Feb 6, 2024
6cd6154
Set default to false
Archmonger Feb 6, 2024
767aeea
rename only_once to allow_duplicates
Archmonger Feb 10, 2024
dcd6a55
Only configure the scope if using `allow_duplicates=false`
Archmonger Feb 12, 2024
57b1950
DRY static file loading
Archmonger Feb 12, 2024
726e244
minor docs syntax highlighting
Archmonger Feb 18, 2024
f8cb34c
docstrings
Archmonger Feb 18, 2024
6f64d46
bump deprecated workflows
Archmonger Feb 18, 2024
acf7e6d
functional test components
Archmonger Feb 18, 2024
b5bfe4b
add changelog
Archmonger Feb 19, 2024
d04fdab
test stubs
Archmonger Feb 19, 2024
d5ff8fa
Redo how we set up lists for the test
Archmonger Feb 19, 2024
999a469
try using discrete mount/unmount managers
Archmonger Feb 19, 2024
d5b16dd
Make sure test elements always have keys
Archmonger Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 64 additions & 12 deletions src/reactpy_django/components.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
from typing import Any, Callable, Sequence, Union, cast, overload
from urllib.parse import urlencode
from uuid import uuid4
from warnings import warn

from django.contrib.staticfiles.finders import find
Expand All @@ -15,6 +16,7 @@
from reactpy.types import Key, VdomDict

from reactpy_django.exceptions import ViewNotRegisteredError
from reactpy_django.hooks import use_scope
from reactpy_django.utils import generate_obj_name, import_module, render_view


Expand All @@ -27,8 +29,7 @@ def view_to_component(
compatibility: bool = False,
transforms: Sequence[Callable[[VdomDict], Any]] = (),
strict_parsing: bool = True,
) -> Any:
...
) -> Any: ...


# Type hints for:
Expand All @@ -39,8 +40,7 @@ def view_to_component(
compatibility: bool = False,
transforms: Sequence[Callable[[VdomDict], Any]] = (),
strict_parsing: bool = True,
) -> Callable[[Callable], Any]:
...
) -> Callable[[Callable], Any]: ...


def view_to_component(
Expand Down Expand Up @@ -122,7 +122,9 @@ def constructor(
return constructor


def django_css(static_path: str, key: Key | None = None):
def django_css(
static_path: str, allow_duplicates: bool = False, key: Key | None = None
):
"""Fetches a CSS static file for use within ReactPy. This allows for deferred CSS loading.

Args:
Expand All @@ -132,10 +134,12 @@ def django_css(static_path: str, key: Key | None = None):
immediate siblings
"""

return _django_css(static_path=static_path, key=key)
return _django_css(
static_path=static_path, allow_duplicates=allow_duplicates, key=key
)


def django_js(static_path: str, key: Key | None = None):
def django_js(static_path: str, allow_duplicates: bool = False, key: Key | None = None):
"""Fetches a JS static file for use within ReactPy. This allows for deferred JS loading.

Args:
Expand All @@ -145,7 +149,9 @@ def django_js(static_path: str, key: Key | None = None):
immediate siblings
"""

return _django_js(static_path=static_path, key=key)
return _django_js(
static_path=static_path, allow_duplicates=allow_duplicates, key=key
)


@component
Expand Down Expand Up @@ -250,13 +256,59 @@ def _view_to_iframe(


@component
def _django_css(static_path: str):
return html.style(_cached_static_contents(static_path))
def _django_css(static_path: str, allow_duplicates: bool):
scope = use_scope()
ownership_uuid = hooks.use_memo(lambda: uuid4())
scope.setdefault("reactpy", {}).setdefault("css", {})
scope["reactpy"]["css"].setdefault(static_path, ownership_uuid)

# Load the file if no other component has loaded it
@hooks.use_effect(dependencies=None)
async def duplicate_manager():
if allow_duplicates:
return

# If the file currently isn't rendered, let this component render it
if not scope["reactpy"]["css"].get(static_path):
scope["reactpy"]["css"].setdefault(static_path, ownership_uuid)

# The component that loaded the file should notify when it's removed
def unmount():
if scope["reactpy"]["css"].get(static_path) == ownership_uuid:
scope["reactpy"]["css"].pop(static_path)

return unmount

if allow_duplicates or (scope["reactpy"]["css"].get(static_path) == ownership_uuid):
return html.style(_cached_static_contents(static_path))


@component
def _django_js(static_path: str):
return html.script(_cached_static_contents(static_path))
def _django_js(static_path: str, allow_duplicates: bool):
scope = use_scope()
ownership_uuid = hooks.use_memo(lambda: uuid4())
scope.setdefault("reactpy", {}).setdefault("js", {})
scope["reactpy"]["js"].setdefault(static_path, ownership_uuid)

# Load the file if no other component has loaded it
@hooks.use_effect(dependencies=None)
async def duplicate_manager():
if allow_duplicates:
return

# If the file currently isn't rendered, let this component render it
if not scope["reactpy"]["js"].get(static_path):
scope["reactpy"]["js"].setdefault(static_path, ownership_uuid)

# The component that loaded the file should notify when it's removed
def unmount():
if scope["reactpy"]["js"].get(static_path) == ownership_uuid:
scope["reactpy"]["js"].pop(static_path)

return unmount

if allow_duplicates or (scope["reactpy"]["js"].get(static_path) == ownership_uuid):
return html.script(_cached_static_contents(static_path))


def _cached_static_contents(static_path: str) -> str:
Expand Down
74 changes: 68 additions & 6 deletions tests/test_app/components.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import asyncio
import inspect
from pathlib import Path
from uuid import uuid4

import reactpy_django
from channels.auth import login, logout
Expand Down Expand Up @@ -135,6 +136,67 @@ def django_css():
)


@component
def django_css_allow_duplicates():
scope = reactpy_django.hooks.use_scope()
css_files, set_css_files = hooks.use_state(
[
reactpy_django.components.django_css(
"django-css-only-once-test.css", allow_duplicates=True
)
]
)

async def add_end_css(event):
set_css_files(
css_files
+ [
reactpy_django.components.django_css(
"django-css-only-once-test.css",
allow_duplicates=True,
key=str(uuid4()),
)
]
)

async def add_front_css(event):
set_css_files(
[
reactpy_django.components.django_css(
"django-css-only-once-test.css",
allow_duplicates=True,
key=str(uuid4()),
)
]
+ css_files
)
Copy link
Contributor Author

@Archmonger Archmonger Feb 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmorshea Is changing a list supposed to cause all components in the list to re-render (unmount, remount) every time?

I know the identity of the list is changing, but it's still a list. Not sure if you intended for swapping out lists to cause everything to unmount and re-render.

Copy link
Contributor Author

@Archmonger Archmonger Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some more debugging and I've managed to characterize this issue. I forgot that use_effect(..., dependencies=None) cleans up after every render. I created an issue to see if there's viability in an alternative effect condition.

In the meantime, I tried creating a workaround but was unsuccessful in getting use_effect(..., dependencies=[]) unmount to trigger when the component gets removed from a list. This might actually be a significant bug and potential memory leak.

@component
def _django_static_file(
    static_path: str,
    prevent_duplicates: bool,
    file_type: str,
    vdom_constructor: VdomDictConstructor,
):
    scope = use_scope()
    mount_trigger, set_mount_trigger = hooks.use_state(True)
    ownership_uuid = hooks.use_memo(lambda: uuid4())

    # Configure the ASGI scope to track the file
    if prevent_duplicates:
        scope.setdefault("reactpy", {}).setdefault(file_type, {})
        scope["reactpy"][file_type].setdefault(static_path, ownership_uuid)

    # Check if other _django_static_file components have unmounted
    @hooks.use_effect(dependencies=None)
    async def mount_manager():
        if prevent_duplicates and not scope["reactpy"][file_type].get(static_path):
            scope["reactpy"][file_type].setdefault(static_path, ownership_uuid)
            set_mount_trigger(not mount_trigger)

    # Notify other components that we've unmounted
    @hooks.use_effect(dependencies=[])
    async def unmount_manager():
        if not prevent_duplicates:
            return

        def unmount():
            if scope["reactpy"][file_type].get(static_path) == ownership_uuid:
                scope["reactpy"][file_type].pop(static_path)

        return unmount

    # Render the component, if needed
    if not prevent_duplicates or (
        scope["reactpy"][file_type].get(static_path) == ownership_uuid
    ):
        return vdom_constructor(cached_static_contents(static_path))


async def remove_end_css(event):
if css_files:
set_css_files(css_files[:-1])

async def remove_front_css(event):
if css_files:
set_css_files(css_files[1:])

return html.div(
{"id": "django-css-only-once"},
html.div({"style": {"display": "inline"}}, "django_css_allow_duplicates: "),
html.button(
"This text should be blue. The stylesheet for this component should only be added once."
),
html.div(
html.button({"on_click": add_end_css}, "Add End File"),
html.button({"on_click": add_front_css}, "Add Front File"),
html.button({"on_click": remove_end_css}, "Remove End File"),
html.button({"on_click": remove_front_css}, "Remove Front File"),
),
html.div(f'CSS ownership tracked via ASGI scope: {scope.get("reactpy_css")}'),
html.div(f"Components with CSS: {css_files}"),
css_files,
)


@component
def django_js():
success = False
Expand Down Expand Up @@ -720,9 +782,9 @@ async def on_submit(event):
"data-fetch-error": bool(user_data_query.error),
"data-mutation-error": bool(user_data_mutation.error),
"data-loading": user_data_query.loading or user_data_mutation.loading,
"data-username": "AnonymousUser"
if current_user.is_anonymous
else current_user.username,
"data-username": (
"AnonymousUser" if current_user.is_anonymous else current_user.username
),
},
html.div("use_user_data"),
html.button({"class": "login-1", "on_click": login_user1}, "Login 1"),
Expand Down Expand Up @@ -788,9 +850,9 @@ async def on_submit(event):
"data-fetch-error": bool(user_data_query.error),
"data-mutation-error": bool(user_data_mutation.error),
"data-loading": user_data_query.loading or user_data_mutation.loading,
"data-username": "AnonymousUser"
if current_user.is_anonymous
else current_user.username,
"data-username": (
"AnonymousUser" if current_user.is_anonymous else current_user.username
),
},
html.div("use_user_data_with_default"),
html.button({"class": "login-3", "on_click": login_user3}, "Login 3"),
Expand Down
3 changes: 3 additions & 0 deletions tests/test_app/static/django-css-only-once-test.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#django-css-only-once button {
color: rgb(0, 0, 255);
}
Loading