EmilStenstrom / django-components

Create simple reusable template components in Django.
MIT License
1.16k stars 76 forks source link

Unexpected behaviour of `ComponentDependencyMiddleware` in combination with `as_view` usage #657

Open Indomian opened 1 month ago

Indomian commented 1 month ago

Hi! First I want to thank you for the great library.

I run into an issue which appears in combination of different features inside library. Issue appears if one uses ComponentDependencyMiddleware + RENDER_DEPENDENCIES setting + as_view usage of the component.

Error provided: The component "ClassName" is not registered at django_components/component_registry.py, line 245

How to reproduce

  1. Configure django_components with dependencies middleware following default documentation
  2. Create a simple component (doesn't matter which one)
  3. Configure RENDER_DEPENDENCIES: True
  4. Create any location which will be using given component
  5. Try to open location - you will receive an exception The component "MagicComponent" is not registered

I created a simple repository with demo application illustrating combination of settings: https://github.com/Indomian/django_component_as_view_error

Analysis

I belive problem is located somewhere in the combination of logic that is located here: https://github.com/EmilStenstrom/django-components/blob/b7e913a86087aee6f67d15805aca4a4252680e37/src/django_components/component.py#L673. In general when component is rendered from as_view call it doesn't have any provided name, so value of self.name would be equal to the class name and this will lead to rendering of placeholder with name of the component which was not registered before. See https://github.com/EmilStenstrom/django-components/blob/b7e913a86087aee6f67d15805aca4a4252680e37/src/django_components/middleware.py#L70, here components registry will throw an exception that component with given class name is not registered (which is correct, due to the fact that is was registered under another name).

How to approach to fix

I tried to check if it is possible to provide registred name to the as_view call, but right now it doesn't pass through any arguments. The only solution I came up with is:

from django.urls import path
from django_components import ComponentView

from components.test_component.component import MagicComponent

component = MagicComponent(
    registered_name="test_component"
)

urlpatterns = [
    path(
        'magic/',
        ComponentView.as_view(
            component=component
        ),
        name="magic_component_name"
    )
]

But currently I'm not sure about downsides of this solution :). Also, I think I can create separate class with a component view which I will use later, but it would be definitely confusing.

JuroOravec commented 1 month ago

Hey @Indomian, thanks for the report, you're right with the analysis. The middleware will undergo a revamp in a week or two, which will refactor the placeholders logic. But the component-name-passing behavior will still be there, so we can fix it before then.

So right now we insert the component's registered name with the placeholder into the rendered HTML, so that the rendered output can be passed around, so we can then extract the information inside the middleware.

As for the bug, currently we pass around the registered component name, and then look up the corresponding component class.

I think that, instead of component's registered name, we could pass around components import path (e.g. myapp.components.mycomp.MyComp). That way, we would also make the component lookup logic work across different ComponentRegistry instances.

JuroOravec commented 1 month ago

Alright, the actual fix is a bit more involved, so it will be easier to merge it with the rest of the refactor.

In the meantime, as part of v0.100 release, there will be also this refactor, which will pick up the registered name when you call as_view() on an component INSTANCE. So you will be able to do this:

MagicComponent(
    registered_name="test_component"
).as_view()
Indomian commented 1 month ago

Nice and easy, thank you! It still overhead for development, but will do the job.