adamghill / django-unicorn

The magical reactive component framework for Django ✨
https://www.django-unicorn.com
MIT License
2.39k stars 119 forks source link

UnicornView._attribute_names() fails in some components because of unresolved type hints #639

Open pa-hqt opened 10 months ago

pa-hqt commented 10 months ago

One of my components crashes on page load due to recent type hint changes in django_unicorn.components.unicorn_view.UnicornView._attribute_names() of version 0.58.0 (commit 8a9448793)

The head of my component looks like

import datetime
from django_unicorn.components import UnicornView
from my_app.models import AggregatSt

class MyComponentView(UnicornView):
    aggregate: AggregatSt = None
    date: datetime.date = None

Exception:

Traceback (most recent call last):
[...]
  File "...\venv\lib\site-packages\django\template\base.py", line 966, in render_annotated
    return self.render(context)
  File "...\venv\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 "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 895, in create
    component = construct_component(
  File "<decorator-gen-3>", line 2, in construct_component
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 151, in construct_component
    component = component_class(
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 245, in __init__
    self._set_caches()
  File "<decorator-gen-5>", line 2, in _set_caches
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 273, in _set_caches
    self._attribute_names_cache = self._attribute_names()
  File "<decorator-gen-13>", line 2, in _attribute_names
  File "...\venv\lib\site-packages\django_unicorn\components\unicorn_view.py", line 593, in _attribute_names
    for type_hint_attribute_name in get_type_hints(self).keys():
  File "...\venv\lib\site-packages\django_unicorn\typer.py", line 75, in get_type_hints
    type_hints = typing_get_type_hints(obj)
  File "C:\Users\philip.arnold\AppData\Local\Programs\Python\Python310\lib\typing.py", line 1870, in get_type_hints
    value = _eval_type(value, globalns, localns)
  File "C:\Users\philip.arnold\AppData\Local\Programs\Python\Python310\lib\typing.py", line 327, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "C:\Users\philip.arnold\AppData\Local\Programs\Python\Python310\lib\typing.py", line 694, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'AggregatSt' is not defined

In typing.get_type_hints()#L1859 the hints dictionary has following values:

hints = {'aggregate': 'AggregatSt', 'date': 'datetime.date'}

Since the values are strings, they are used to construct ForwardRef, but they cannot be solved, because globalns and localns are empty.

Any ideas, why the imports fail? May it be a solution to add NameError to catch block in django_unicorn.typer#L81?

(same happens with 'datetime' if I remove the type hint of my custom class 'AggregatSt')

I am using python 3.10, Django 4.2.9 and django-unicorn 0.58.0, here.

adamghill commented 10 months ago

Oh shoot, I'm sorry you ran into this issue. I will release a hot-fix tonight to at least prevent this from crashing your component. Thanks again for the bug report.

adamghill commented 10 months ago

Unfortunately, I cannot replicate this using Django 4.2.9 and Python 3.10.12. However, I added a catch for NameError anyway in the hopes that it solves your problem. I also wrote a test to show that the key for the dictionary is a class and not a string for me. 🤷 Maybe you can look at my test and see if it breaks for you? I'm curious what else might be going on.

adamghill commented 10 months ago

My fix has been merged into main and 0.58.1 has published (https://pypi.org/project/django-unicorn/). Please let me know if that solves your problem or if you are still running into an issue.

pa-hqt commented 10 months ago

Thank you very much for providing this hotfix instantly! Yes, that helps me, because the component can be constructed without exception now.

I do not understand what is different with that component, I descriped above, than others that have similar class attribute definitions and worked without error. Also your new test case does not fail on my maschine. So this seams to be an issue of category "unpredictable".

But inspired by your test case, I wrote some more that pass with 0.58.1 and do not with 0.58.0. Maybe you should check, if that test expectations match your personal expectation what should happen in a component.

import datetime

from django_unicorn.components import UnicornView
from django_unicorn.typer import get_type_hints

def test_get_type_hints_gh_639_hint():
    class MyComponentView(UnicornView):
        a_date: datetime.date

    expected = {"a_date": datetime.date}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected

def test_get_type_hints_gh_639_hint_with_default():
    class MyComponentView(UnicornView):
        a_date: datetime.date = None

    expected = {"a_date": datetime.date}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected

def test_get_type_hints_gh_639_ref():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = {}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected

def test_get_type_hints_gh_639_ref_with_default():
    class MyComponentView(UnicornView):
        a_date: "datetime.date" = None

    expected = {}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123"))
    assert actual == expected

def test_get_type_hints_gh_639_ref_with_kwarg():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = {}
    actual = get_type_hints(MyComponentView(component_name="test", component_id="123", a_date=datetime.date.fromisoformat("2024-01-05")))
    assert actual == expected

def test_component_attribute_names_gh_639_hint():
    class MyComponentView(UnicornView):
        a_date: datetime.date

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected

def test_component_attribute_names_gh_639_hint_with_default():
    class MyComponentView(UnicornView):
        a_date: datetime.date = None

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected

def test_component_attribute_names_gh_639_ref():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = []
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected

def test_component_attribute_names_gh_639_ref_with_default():
    class MyComponentView(UnicornView):
        a_date: "datetime.date" = None

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123")._attribute_names()
    assert actual == expected

def test_component_attribute_names_gh_639_ref_with_kwarg():
    class MyComponentView(UnicornView):
        a_date: "datetime.date"

    expected = ["a_date"]
    actual = MyComponentView(component_name="test", component_id="123", a_date=datetime.date.fromisoformat("2024-01-05"))._attribute_names()
    assert actual == expected
adamghill commented 10 months ago

Thanks for these test cases! Looking through them, I think there are some gaps in how unicorn deals with certain type annotations for sure.