-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add Django CSS/JS de-duplication #226
base: main
Are you sure you want to change the base?
Add Django CSS/JS de-duplication #226
Conversation
tests/test_app/components.py
Outdated
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 | ||
) |
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.
@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.
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.
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))
Note to self, once we bugfix the script element's duplicate renders, this can be implemented as a client-side feature. |
Description
Since CSS/JS is loaded as raw strings within a
style
tag, it's currently difficult to prevent duplicate loads.This PR implements a simple server-side de-duplication algorithm for CSS and JS.
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).