Extra <html><head><body> inserted after updating to v0.110

PouriaMzn commented 3 days ago

First of all, thank you for the awesome work on this library! i just recently update from v0.94 to v0.114 and encountered an issue when rendering partial templates that do not contain any components. These templates are now being altered by the ComponentDependencyMiddleware.

I have a main template that includes multiple components. Within it, I dynamically include a partial template to add rows to a table. Below is an example:

Main Template:

<tbody id="items" class="divide-y divide-slate-300">
    {% for form in formset %}
        {% include "partials/items_list.html" with form=form row_number=forloop.counter %}
    {% endfor %}
</tbody>

<button 
    type="button"
    class="btn btn-sm btn-info"
    hx-get="{{ custom_url }}"
    hx-trigger="click"
    hx-target="#items"
    hx-swap="beforeend"
    hx-vals="{% jsonify form_count=formset|length %}"
    id="add-item-button">
        Add Row
</button>

Partial Template (partials/items_list.html):

{% load widget_tweaks %}
{% url 'products:search_product_service' as product_search_url %}

<tr class=" hover:bg-gray-200 py-0 {% cycle 'bg-white' 'bg-gray-50' %} divide-x "
    aria-rowindex="{{ row_number }}">
    <!-- row num -->
    <td class="whitespace-nowrap w-fit text-center px-4 w-px"
        aria-colindex="1">
        {{ row_number }}
        {% render_field form.id placeholder="" %}
        {% render_field form.DELETE type="hidden" %}
    </td>
    .....
</tr>

so with this setup i use the button to add new rows to the main template from the partial template that itself does not have any components. but now when i updated to v0.114, when i retrieve the response from the backend is :

<html><head></head><body>
    <!-- row num -->

        2
        <input type="hidden" name="items-1-id" placeholder="" id="id_items-1-id">
        <input type="hidden" name="items-1-DELETE" id="id_items-1-DELETE">

        .......

<script src="/static/django_components/django_components.min.js"></script><script>eval(Components.unescapeJs(`(() =&gt; {
        const loadedJsScripts = [];
        const loadedCssScripts = [];
        const toLoadJsScripts = [];
        const toLoadCssScripts = [];

        loadedJsScripts.forEach((s) =&gt; Components.manager.markScriptLoaded(&quot;js&quot;, s));
        loadedCssScripts.forEach((s) =&gt; Components.manager.markScriptLoaded(&quot;css&quot;, s));

        Promise.all(
            toLoadJsScripts.map((s) =&gt; Components.manager.loadScript(&quot;js&quot;, s))
        ).catch(console.error);
        Promise.all(
            toLoadCssScripts.map((s) =&gt; Components.manager.loadScript(&quot;css&quot;, s))
        ).catch(console.error);
    })();
    `))</script></body></html>

The <tr> and <td> tags are removed and replaced with <div>, and it's being rendered as a full document with added two scripts tags. And the partial won't render as indented. But, if i add:

{% component_css_dependencies %}
{% component_js_dependencies %}

to the partial, it prevents the <tr> and <td> tags from being replaced. However, the script tags are still appended unnecessarily. Since i do not have any component on the template partial, is that necessary? or maybe my configuration is wrong?

and here is my configuration by the way:

STATICFILES_FINDERS = [
    # Default finders
    'django.contrib.staticfiles.finders.FileSystemFinder',
    'django.contrib.staticfiles.finders.AppDirectoriesFinder',
    # Django components
    "django_components.finders.ComponentsFileSystemFinder",

]

INSTALLED_APPS = [
'django_components',
]

MIDDLEWARE = [
'django_components.middleware.ComponentDependencyMiddleware',
]

COMPONENTS = ComponentsSettings(
    autodiscover= True,
    dirs= [BASE_DIR / 'components'],
    reload_on_template_change= True,
    static_files_forbidden= [
        ".html", ".django", ".dj", ".tpl",
        ".py", ".pyc",
    ],
)

TEMPLATES = [
    {
        'BACKEND': 'django.template.backends.django.DjangoTemplates',
        'DIRS': [BASE_DIR / 'templates'],
        'OPTIONS': {
            'context_processors': [
                'django.template.context_processors.debug',
                'django.template.context_processors.request',
                'django.contrib.auth.context_processors.auth',
                'django.contrib.messages.context_processors.messages',
            ],
            'builtins': [
                'django_components.templatetags.component_tags',
            ],
            'loaders':[(
                'django.template.loaders.cached.Loader', [
                  # Default Django loader
                  'django.template.loaders.filesystem.Loader',
                  # Inluding this is the same as APP_DIRS=True
                  'django.template.loaders.app_directories.Loader',
                  # Components loader
                  'django_components.template_loader.Loader',
                ]
            )],
        },

    },

    ]

STATICFILES_DIRS = [
    BASE_DIR / 'static',
    BASE_DIR / "components",
]

JuroOravec commented 3 days ago

Note: I've edited your post to escape the tags like <div> or <td>, because Github interprets them as HTML otherwise.

JuroOravec commented 3 days ago

@PouriaMzn Thanks for reporting this! This is definitely a bug. What I think is happening is following:

We use selectolax to parse the HTML and modify it, e.g. by adding extra <script>, or in the future to support scoped CSS, JS / CSS vars, etc.
Selectolax, being a wrapper for Lexbor, tries to parse the HTML same way as the browser would. For example it always adds <html>, <head>, <body> tags if they are missing.
I wrote some internal code to account for this behavior of selectolax, but it seems that in this case it doesn't work for some reason.

That internal code, I've also it pushed upstream to selectolax. So in best case scenario, updating Selectolax to latest version would hopefully fix it.

Otherwise, I'll have to figure out what's going on.

Side note, but tbh Selectolax is causing me headaches. It's about 3-4x faster than lxml or Beautiful Soup or other alternatives, so it has great potential. But it's a wrapper for Lexbor, Lexbor was built to follow the specs of how the browser parses HTML. And this does not always align with what we need.

@PouriaMzn as for your issue at hand, how do you render the main template? This is a similar issue to #789. See in this comment how there the workaround was to render the whole template as a component, and use render_dependencies=False

def get_badge(label: StrOrPromise, color: BadgeColor = BadgeColor.INFO) -> SafeString:
    from components.badge import Badge

    return mark_safe(  # nosec
        Badge.render(
            kwargs={"style": color.value},
            slots={"content": label},
            render_dependencies=False
        )
    )

So if you are currently maybe using django's render() function to render the main template, then you'd need to make a component class that wraps that template:

Instead of

render("path/to/template.html", {"bla": "bla"})

do

from django_components import Component

class MainTemplate(Component):
    template_name = "path/to/template.html"

...

MainTemplate.render(
    ...
    render_dependencies=False,
)

PouriaMzn commented 3 days ago

thank you for your response. the way i render the main template is that i have a class-based view and in that view i have:

def render_response(self, request, context):
    template = self.partial_template if request.htmx else self.template
    return render(request, template, context)

def get(self, request):
    context = self.get_context(
            ....
    )
    return self.render_response(request, context)

I also use django-template-partials and htmx in all of my templates, so i'm not sure if making a component class that wraps the main template would be ideal here.

PouriaMzn commented 3 days ago

during my update, one more thing I noticed is that if i want to use WhiteNoise to server static files in production (DEBUG = False) and use WhiteNoise compression and caching backend like this:

STORAGES = {
    # ...
    "staticfiles": {
        "BACKEND": "whitenoise.storage.CompressedManifestStaticFilesStorage",
    },
}

it throws an error:

[1] Internal Server Error: /favicon.ico
[1] Traceback (most recent call last):
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django\core\handlers\exception.py", line 55, in inner
[1]     response = get_response(request)
[1]                ^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django_components\dependencies.py", line 785, in __call__
[1]     response = self.process_response(response)
[1]                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django_components\dependencies.py", line 798, in process_response
[1]     response.content = render_dependencies(response.content, type="document")
[1]                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django_components\dependencies.py", line 335, in render_dependencies
[1]     content_, js_dependencies, css_dependencies = _process_dep_declarations(content_, type)
[1]                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django_components\dependencies.py", line 511, in _process_dep_declarations   
[1]     js=[static("django_components/django_components.min.js")],
[1]         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django\templatetags\static.py", line 179, in static
[1]     return StaticNode.handle_simple(path)
[1]            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django\templatetags\static.py", line 129, in handle_simple
[1]     return staticfiles_storage.url(path)
[1]            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django\contrib\staticfiles\storage.py", line 203, in url
[1]     return self._url(self.stored_name, name, force)
[1]            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django\contrib\staticfiles\storage.py", line 182, in _url
[1]     hashed_name = hashed_name_func(*args)
[1]                   ^^^^^^^^^^^^^^^^^^^^^^^
[1]   File "D:\prj\miniconda3\envs\django\Lib\site-packages\django\contrib\staticfiles\storage.py", line 516, in stored_name
[1]     raise ValueError(
[1] ValueError: Missing staticfiles manifest entry for 'django_components/django_components.min.js'
[1] [30/Nov/2024 11:51:41] "GET /favicon.ico HTTP/1.1" 500 145

but i have the django_components.min.js present in my STATIC_ROOT directory under static-root/django_components/django_components.min.js

if i change the staticfiles backend to:

STORAGES = {
    # ...
    "staticfiles": {
        "BACKEND": "whitenoise.storage.CompressedStaticFilesStorage",
    },
}

it works fine, but now we do not have compression and caching.

JuroOravec commented 3 days ago

@PouriaMzn Would you make your last comment about CompressedManifestStaticFilesStorage into a separate issue?

JuroOravec commented 3 days ago

thank you for your response. the way i render the main template is that i have a class-based view and in that view i have:
def render_response(self, request, context):
  template = self.partial_template if request.htmx else self.template
  return render(request, template, context)

def get(self, request):
  context = self.get_context(
          ....
  )
  return self.render_response(request, context)
I also use django-template-partials and htmx in all of my templates, so i'm not sure if making a component class that wraps the main template would be ideal here.

Class-based view meaning using the Django's View class?

I haven't used django-template-partials, so I won't be able to help much on that front.

Do you use django-components a lot, or are you just introducing it? If this fragment is the only place where you are using django-components, or if your components don't use any JS / CSS, then in your case might be better off by removing the ComponentDependencyMiddleware for now.

PouriaMzn commented 3 days ago

yes, by Class-based i meant Django's View class. in terms of my django-components use cases, i have more than 10 components in my project that i use in many different places, and each of them has a separate JS file, i usually don't use the CSS file for my component, because i use tailwind straightly in the template of my component. So, removing ComponentDependencyMiddleware would cause a lot of headaches for me. I prefer to downgrade to v0.100 which works fine with my current workflow and i hope in the future we could find a way to solve this problem.

JuroOravec commented 3 days ago

Ok, in that case it should be safe to downgrade to 0.102 instead of 0.100, as 0.101 and 0.102 were only bug fixes.

Btw, is this the only place where you've come across this issue? Are there other places in your project where you render tables and table rows as HTML fragments / partials? This will help pinpoint whether this is an issue with selectolax or something else.

Also, I just noticed that the main template contains only <tbody>. Is the <tbody> wrapped in <table>? Potential cause of the problem could be that selectolax sees <tbody>, but no <table>, and maybe this trips it up.

PouriaMzn commented 3 days ago

yes this the only place i use a for loop to get a <tr> partials and add new rows dynamically, and as for the <tbody> , the main template includes that as well:

<table class="table-auto border-collapse divide-y divide-x divide-slate-300 w-full">
    <!-- Table head -->
    <thead>
        <tr class="py-0 my-0 h-7">
            <!-- Empty row -->
            <th class="min-w-12">#</th>
            .....
        </tr>
    </thead>
    <!-- Table body -->
    <tbody id="items" class="divide-y divide-slate-300">
        {% for form in formset %}
            {% include "partials/items_list.html" with form=form row_number=forloop.counter %}
        {% endfor %}
    </tbody>
</table>

JuroOravec commented 3 days ago

@PouriaMzn I think I was able to replicate the issue with the extra tags being inserted, wrote a fix in #797. Now it should correctly work such that if you define a template like you did, but it does not include {% component_dependencies %} tags, nor does not include <head> or <body> tags, then the extra <script> tag should NOT be inserted.

EmilStenstrom / django-components

Extra <html><head><body> inserted after updating to v0.110 #793