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.

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

use_effect's unmount method is not always called with dynamically rendered children #1198

Open
Archmonger opened this issue Feb 19, 2024 · 1 comment
Labels
priority-0-critical Must be resolved and released immediately. release-patch Warrents a patch release type-bug About something that isn't working

Comments

@Archmonger
Copy link
Contributor

Archmonger commented Feb 19, 2024

Current Situation

This issue was first discovered while trying to develop reactive-python/reactpy-django#226

I created a component that simulates a list of conditionally rendered children. Each child in the list contains a unique key. However, the use_effect unmount method does not always appear to be called when the component is dismounted.

Here is a minimum viable example:

from uuid import uuid4

from reactpy import component, hooks, html, run


@component
def child():
    ownership_uuid = hooks.use_memo(lambda: uuid4().hex)

    @hooks.use_effect(dependencies=[])
    async def unmount_manager():
        print("registering new unmount func")
        def unmount():
            # FIXME: This is not working as expected. Dismount is not always called.
            print("unmount func called")
        return unmount

    return ownership_uuid


@component
def root():
    components = hooks.use_ref([child(key=uuid4().hex)])
    reload_trigger, set_reload_trigger = hooks.use_state(True)

    async def add_front_child(event):
        components.current.insert(0, child(key=uuid4().hex))
        set_reload_trigger(not reload_trigger)

    async def add_end_child(event):
        components.current.append(child(key=uuid4().hex))
        set_reload_trigger(not reload_trigger)

    async def remove_front_child(event):
        if components.current:
            components.current.pop(0)
            set_reload_trigger(not reload_trigger)

    async def remove_end_child(event):
        if components.current:
            components.current.pop()
            set_reload_trigger(not reload_trigger)

    return html.div(
        html.div(
            html.button({"on_click": add_front_child}, "Add End"),
            html.button({"on_click": add_end_child}, "Add Front"),
            html.button({"on_click": remove_front_child}, "Remove End"),
            html.button({"on_click": remove_end_child}, "Remove Front"),
        ),
        html.div(f"Components: {components.current}"),
        components.current,
    )


run(root)

You can demo this by adding a bunch of children, then removing them. If you look at terminal, you'll notice that unmount was only called once despite multiple children being unloaded.

Proposed Actions

No response

@JamesHutchison
Copy link

JamesHutchison commented Nov 3, 2024

This might be related to a bug I've seen where nested children are marked as "rendered" and then if there's subsequent updates to their state they get skipped. Changing the rendering order can expose or hide the bug.

If I had to guess, an example of this:
Button alters state on child and triggers callback on parent
Parent alters parameter passed to child based on callback
Child doesn't render with new parameter because it was already rendered.

This can create a case where subsequent triggers to the child fail because the component is unmounted

I'm writing this off the top of my head, I don't have a reproducible state off hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-0-critical Must be resolved and released immediately. release-patch Warrents a patch release type-bug About something that isn't working
Projects
None yet
Development

No branches or pull requests

2 participants