Middleware lets script placeholders slip through undeleted in certain cases

[a3lem]

In the course of PR #271 it came to light that one user was observing script placeholder tags in their generated html, despite the middleware being enabled.

(For the uninitiated, these placeholders are added by the component_js_dependencies tag when render_dependencies=True. Later, the middleware finds-and-replaces them with script tags linking to the appropriate js scripts.)

So far, we haven't been able to reproduce this behavior, but the conversation in the PR includes an interesting hint:

Just to be clear, I think this is an issue when you're loading a page that has components which don't have JS scripts defined, this script loads in anticipation but is not replaced. If there's a component on the page with a script, this will be replaced.

We most likely have a bug on our hands. The fact that we can't repro it probably indicates a blind spot in our tests. We're investigating the issue.

If you're noticing similar behavior in your projects, please let us know. Real-world code bases are much better for helping us understand subtle issues like this.

[EmilStenstrom]

Here's a simple testcase that reproduces this:

    def test_dependencies_with_no_components(self):
        template = Template(
            """
            {% load component_tags %}
            {% component_dependencies %}
            """
        )
        rendered = template.render(Context())

        # Assertions to ensure placeholders are properly replaced or removed
        self.assertNotIn(
            '<script name="JS_PLACEHOLDER"></script>', rendered, "JS placeholder should be removed"
        )
        self.assertNotIn(
            '<link name="CSS_PLACEHOLDER">', rendered, "CSS placeholder should be removed or replaced"
        )

[JuroOravec]

I had a brief look into this and the JS/CSS dependencies rendering. The middleware approach seems to be kinda-sorta working (if I stream the response, or I use different content-type than text/html, it will not work). So I think it might be better to handle the dependencies insertion BEFORE we hit the Django response. So decoupling the dependency insertion from middleware, so middleware becomes only one of possible ways how to apply the dependencies.

For that, we need to be aware of the rendering execution - when it starts and when it stops. We also need to access the rendered content, so we can do string replacement like we do in the middleware.

Some ideas to achieve that:

1. Function with callback

   • Inside component_dependencies, we first mark on the Context that we're registering components.
   • Then we pass the context to render function, which would return a string.
   • Then again hidden inside component_dependencies, we replace the deps placeholders with actual JS/CSS deps

   track_dependencies(
   	context,
   	lambda ctx: Template("""
   		{% component_dependencies %}
   		{% component 'hello' %}{% endcomponent %}
   	""").render(ctx),
   )
   

2. Context manager function

   • Same flow as before, except that instead of hiding the complexity, we expose it, and we require user to decide if they want to replace the deps or not by calling deps.renders_deps(content).
   • IMO I like the function with callback approach more. Because if user decides NOT to call deps.renders_deps, then there's no need for them to call with track_dependencies(context) in the first place. So user needlessly needs to do 2 steps which could be simplified into 1.

   with track_dependencies(context) as deps:
   	content = Template("""
   		{% component_dependencies %}
   		{% component 'hello' %}{% endcomponent %}
   	""").render(ctx)
   	return deps.renders_deps(content)
   

3. Track render execution via {% component_dependencies %} tag

   We can also do similar thing as Django's extends tag does, which is that it sets the rest of the Template as it's children nodelist. This way, we could turn component_dependencies tag into ComponentDepsNode. And when we call ComponentDepsNode.render(), we first render all it's child nodes (AKA rest of the template), whilst recording which components were used. And after that, we prepend the JS/CSS dependencies from all encountered components.

   While I liked this idea the most, because we wouldn't need to deal with the string replacement, I think that for this to work we would need to require users to use {% component_dependencies %} in the top-level Template. Because otherwise component_dependencies would not be aware of components in parent Templates.