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

parent components (i) do not refresh children html and (ii) break when used as direct views #662

Closed
jacksund opened this issue Feb 22, 2024 · 15 comments · May be fixed by #670
Closed

parent components (i) do not refresh children html and (ii) break when used as direct views #662

jacksund opened this issue Feb 22, 2024 · 15 comments · May be fixed by #670

Comments

@jacksund
Copy link
Contributor

This could probably be two separate github issues, but I keep them together just to make things easier. I break them into section below though.

I've reproduced these errors in several environments:

  • python 3.9 & 3.11
  • django 4.2.7 & 5.0.2
  • django-unicorn 0.59.0
  • pip & conda-forge installs

And I'm using Chrome for my browser

issue 1: html not refreshing

I was struggling with this in a personal project, so I went back and was actually able to reproduce the issue with the example from the docs. I built the parent component from this example in the docs exactly, and only added one line:

# added to `updated_search` method in `filter.py`
print(f"'{query}' matches {len(self.parent.books)} books")

Here are the minimal django project files. Note, sqlite file has everything loaded already, so you can just use python manage.py runserver to run the app:
mysite_example1.zip

Once on the books/ page, you can see that the unicorn calls & backend is working as expected but the page doesn't refresh:

image

No matter what I try, I can't get the html to update when the filter component is used.

issue 2: parent components can't be direct views

This is the exact same app as before, except I'm using TableView.as_view() instead of adding the component to an index.html:
mysite_example2.zip

When you first spin up your django server, the view works as expected (except for what I describe in issue 1). However, once the page is refreshed and used a 2nd time, the page fails and cannot be loaded:

image

^^ note in the logs that this happens after a page refresh. This is consistent and happening in my personal project as well.

Here's the full error traceback:

Internal Server Error: /books/
Traceback (most recent call last):
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\core\handlers\exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\core\handlers\base.py", line 220, in _get_response
    response = response.render()
               ^^^^^^^^^^^^^^^^^
  File "<decorator-gen-2>", line 2, in render
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\decorators.py", line 20, in timed
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\components\unicorn_template_response.py", line 125, in render
    response = super().render()
               ^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\response.py", line 114, in render
    self.content = self.rendered_content
                   ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\response.py", line 92, in rendered_content
    return template.render(context, self._request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\backends\django.py", line 61, in render
    return self.template.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\base.py", line 171, in render
    return self._render(context)
           ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\base.py", line 163, in _render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\base.py", line 1000, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\base.py", line 1000, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django\template\base.py", line 961, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\templatetags\unicorn.py", line 179, in render
    self.view = UnicornView.create(
                ^^^^^^^^^^^^^^^^^^^
  File "<decorator-gen-19>", line 2, in create
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\decorators.py", line 20, in timed
    result = func(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\components\unicorn_view.py", line 843, in create
    cached_component._cache_component(parent=parent, component_args=component_args, **kwargs)
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\components\unicorn_view.py", line 411, in _cache_component
    cache_full_tree(self)
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\cacher.py", line 104, in cache_full_tree
    with CacheableComponent(root) as caching:
  File "C:\Users\nxj625\AppData\Local\anaconda3\envs\uni2\Lib\site-packages\django_unicorn\cacher.py", line 40, in __enter__
    if component.component_id in self._state:
       ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'PointerUnicornView' object has no attribute 'component_id'
@jacksund
Copy link
Contributor Author

@adamghill sorry to keep blowing up your repo with long-winded issues 😅. This one in particular is holding back my team's apps, so let me know if there's a way I can sponsor it to help bump it in your priority list

@adamghill
Copy link
Owner

sorry to keep blowing up your repo with long-winded issues

No worries! Much better to have a lot of details so I can reproduce the errors, so thank you! I'm going to take a look at this today and see if I can figure out what is going on. I will let you know. 👍

@adamghill
Copy link
Owner

adamghill commented Feb 24, 2024

Ok, I think there are two things going on for issue 1:

  1. because filter is a child component that interacts with the parent, you will need to call self.parent.force_render = True when changing the parent
image
  1. the html comments at the top of the templates are messing with the morphdom algorithm (morphdom requires a tree structure with one root node; since HTML comments are considered a node, there are two roots in your templates and morphdom gets confused). Moving the comment inside a root node should make it work as expected.
image

Here you can see after I made those changes, things seem to work as expected:
https://github.com/adamghill/django-unicorn/assets/317045/9a2fd932-027e-4cb3-a4b5-f56538e8b96a

Let me know if you run into any other problems or what I said above doesn't make sense!

force_render is mentioned at https://www.django-unicorn.com/docs/views/#force-render, however I need to update the examples to include it. It should also be mentioned in the child components section since that is when it's needed most often.

I should also add a warning to the docs about HTML comments at the top of a component template since that has bitten me before as well. Even better, it would be nice to figure out how to handle that so it doesn't cause an issue at all. I'll dig into this some more today.

I have not checked into issue 2 yet, but I'll look into it next.

@adamghill
Copy link
Owner

Just verified that issue 2 still occurs even with the fixes I laid out above. So, there must be something going on with the direct view. I'll let you know what I find.

@jacksund
Copy link
Contributor Author

you will need to call self.parent.force_render = True

Awesome! I didn't see this in the docs, so thank you. Once I added this and removed my comments, issue 1 is fixed!!

Even better, it would be nice to figure out how to handle that so it doesn't cause an issue at all.

Let me know what you think of these two ideas for helping users avoid this:

  1. in the intro tutorial & across the docs, set a "rule" that users should stick with django comments ({# example #} or {% comment %} blocks) instead of html comments (<!-- example -->). You can explain this somewhere in the docs and reference it in the intro. Comments throughout the docs would then be like....

    {# filter.html #}
    <div>
      <input type="text" unicorn:model="search" />
    </div>
  2. Maybe add a new template tag or update the existing unicorn one to auto-wrap templates in an extra div. If you integrate it directly into existing tags, it should also be possible to disable it in the settings . I'm imagining a simple tag like this:

    from django import template
    
    register = template.Library()
    
    @register.simple_tag
    def include_with_extra_div(template_name):
        return f'<div>{% include "{template_name}" %}</div>'

@adamghill
Copy link
Owner

I think I have solved for all of these issues in #663.

Interestingly in my testing now, having the HTML comment at the top of a component does not seem to be a problem. I could have sworn it was causing an issue of some kind, but it might have been a red herring, so apologies if that caused any concern for you!

I am going to try to get another PR mergeable for this release, but should get a new version published by the end of this weekend.

@jacksund
Copy link
Contributor Author

awesome!! Thank you for doing this so quickly 😄

@adamghill
Copy link
Owner

This got fixed in 0.60.0.

@jacksund
Copy link
Contributor Author

Sorry to do this... But it's looking like issue 2 was only silenced & replaced with a new bug.

After refreshing the page of a direct view, I think the component cache isn't grabbing the correct object. What happens in the official example is that the filter component does not take any input (and incorrectly see's filter staying at None):

image

You can see after the refresh that nothing from my 2nd search reaches the backend

@adamghill
Copy link
Owner

Ok, thanks for letting me know. I'll take a look when I can.

@jacksund
Copy link
Contributor Author

Thanks 😄

This line might need to be re-added by the way:
https://github.com/adamghill/django-unicorn/pull/663/files#diff-5f660caaf7c3ac2a7b0ad9d85ac80265db45f68c0bf1ce7babf1bb426700cbfbL382

Without it, changes from mount are lost on the page's first load. Most of the time it can just recreate this info after the first unicorn/message/ call -- but rare cases, mount will actually fail via the message url. This happens in my one views:

# in urls.py
urlpatterns = [
    # .....
    path(
        route="example/<pk>",   # Note the <pk> here
        view=components.ExampleView.as_view(),
        name="example",
    ),
]
# in component/example.py

class ExampleView(UnicornView):
def mount(self):
    # URL arguments are stored under self.kwargs
    # and it is only available in the first direct view
    self.target = CortevaTarget.objects.get(id=self.kwargs["pk"])

So on page load I am using the url kwarg pk. If the first page load doesn't cache the component, then it fails after.

This works normally in 0.59.0, but throws errors in 0.60.0. I was able to fix this by just re-adding your line self._cache_component(**kwargs) back to UnicornView.dispatch

@jacksund
Copy link
Contributor Author

Seems like I'm causing a lot of headache by trying to use direct views. If you think this will take a while to fix, let me know and I'll switch back to normal components 😅

@adamghill
Copy link
Owner

This line might need to be re-added by the way

Yeah. 😅 That line directly causes the previous error, so I need to dig into the caching context manager to figure out what to do.

If you think this will take a while to fix

Fixing this bug is my highest priority, but I don't have a good sense of how long it'll take -- I spent a decent amount of time the other day debugging the original problem. Might be best to skip direct views for now so you are unblocked, at least. I'll let you know what I find.

@adamghill
Copy link
Owner

@jacksund I think I have a fix for nested components being used in a direct view in #670. If you could verify it for me, I would really appreciate it. Thanks!

@jacksund
Copy link
Contributor Author

jacksund commented Mar 11, 2024

I'm seeing that the initial page works properly and there is no longer an error after refreshing -- however it doesn't look like any message calls are reaching the backend after the refresh:

image

I'm not seeing any of the ajax requests send from the frontend after the refresh either:
image

Here's my test case btw:
mysite_test_case.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants