Fragment node to avoid context pitfall

[geigerzaehler]

It’s been mentioned before (#23) that a fragment might be helpful. I have the following situation: I’m gradually introducing htpy to a Django project to implement small components. I expose these components Jinja2 globals and it roughly looks like this:

from htpy import Context, Node, div

feature_x_context: Context[bool] = Context("feature_enabled", default=False)


def outer_component() -> Node:
    return feature_x_context.provider(
        True,
        div[div()["outer"], inner_component],
    )


def inner_component() -> Node:
    return (
        div()["inner"],
        _feature_component(),
    )


@feature_x_context.consumer
def _feature_component(feature_enabled: bool) -> Node:
    if feature_enabled:
        return div()["with feature"]
    else:
        return div()["without feature"]

def setup_jina_environment(**options):
    ...
    env.globals["outer"] = outer
    env.globals["inner"] = inner

Of course, calling inner in a template does not work because it actually returns a tuple and renders something like (<Element '<div>...</div>'>, <function _feature_component at 0x76110619f4c0>)

So I tried to fix this as follows:

def inner_component() -> Node:
    return render_node((
        div()["inner"],
        _feature_component(),
    ))

But then rendering the outer component does not respect the context anymore.

<div><div>outer</div><div>inner</div><div>without feature</div></div>

The example may look a bit contrived, but I’m working under a lot of constraints that I can’t address.

My suggestion is to introduce a fragment class (or whatever you want to name it):

class fragment:
    def __init__(self, *nodes: Node) -> None:
        self.nodes = nodes

    def __str__(self) -> str:
        return "".join(render_node(n) for n in self.nodes)

    __html__ = __str__

def inner_component() -> Node:
    return fragment(
        div()["inner"],
        _feature_component(),
    )

fragment could replace render_node everywhere. (Plus it has a slightly more convenient API since you don’t need to wrap the arguments in a list or tuple)

The _iter_node_context implementation is pretty straight forward:

def _iter_node_context(x: Node, context_dict: dict[Context[t.Any], t.Any]) -> Iterator[str]:
    ...
    elif isinstance(x, fragment):
        for node in x.nodes:
            yield from _iter_node_context(node, context_dict)

[pelme]

I do not fully follow your example, can you have a look at the variable names, what is small_component, outer and inner? Also, can you show how you call inner/outer from your jinja template? I understand that this is not the full thing you are doing but if you clarify those I think I can understand what you are trying to do!

(If possible I would like to avoid adding fragments since lists basically fill the same need in most cases. But yes, maybe we could replace render_node with a fragment class, that could be interesting!)

Edit: I really want to make it possible to introduce htpy as gradually as possible and have it play nice with jinja/django templates is really important!

[geigerzaehler]

I do not fully follow your example, can you have a look at the variable names, what is small_component, outer and inner?

Sorry for the confusion. small_component should be inner_component. (I updated the example)

Also, can you show how you call inner/outer from your jinja template? I understand that this is not the full thing you are doing but if you clarify those I think I can understand what you are trying to do!

In one template I only need the inner component so I just have {{ inner }} and in another template I want to use the outer component (with the feature of the element enabled), so I use {{ outer }}. Hope that clarifies it.

I put together a branch with a test case and my expectations: https://github.com/geigerzaehler/htpy/blob/fragment/tests/test_fragment.py. test_render_outer fails because the context is wrong and test_render_inner_alternative fails because the node is a tuple.

[geigerzaehler]

Edit: I really want to make it possible to introduce htpy as gradually as possible and have it play nice with jinja/django templates is really important!

On a larger note: One thing I like about this library (and that I value in general with APIs) is that there are few surprises and the behavior is predictable. And one expectation I built up while using it and reading through the docs is that I can freely use a Node everywhere and it renders properly. But this expectation was broken by with Node being a tuple (or any iterator).

An idea that I had was to distinguish between the return type Node and the input type Child . For example, any iterable or callable is a Child, so it can be used as an argument to __getitem__. But they are not instances of Node so you can’t return them or expect str() to work. You would need to wrap them with something like fragment whenever you return them from a component with return type Node. This would make the APIs more consistent and predictable. However, the obvious downside is that it is also more verbose.

[pelme]

In your example, before passing inner to jinja, what about something like this:

    env.globals["inner"] = lambda *args, **kwargs: render_node(inner(*args **kwargs)) 

Would that solve your immediate problem? Then you should be able to turn it into a string from the template while still be able to put inner as a child to outer.

I still find that your proposed fragment class a nicer/more straightforward way of dealing with this since it would avoid having to make this wrapper altogether and the fragment element can both be used as a child node and render itself as a string if needed. fragment should also then implement __iter__. that would make render_node/iter_node redundant and they could then be deprecated.

An idea that I had was to distinguish between the return type Node and the input type Child . For example, any iterable or callable is a Child, so it can be used as an argument to getitem. But they are not instances of Node so you can’t return them or expect str() to work. You would need to wrap them with something like fragment whenever you return them from a component with return type Node. This would make the APIs more consistent and predictable. However, the obvious downside is that it is also more verbose.

The intention of Node is how you describe Child. Node's are never returned anywhere from htpy or in the docs examples. React/TypeScript uses ReactNode for the same thing, that is why I went with Node and not something like Child. Maybe Child would be a better name but I think it would cause too much pain/breakage the rename it now. If you have ideas on how to improve the docs/other ways to communicate on this that would be very welcome!

If we introduce a Fragment class, your inner_component could return Fragment, not Node, which would solve the problem:

def inner_component() -> Fragment:
    return Fragment(
        div()["inner"],
        _feature_component(),
    )

(Sometimes I use Node as return value, for instance if a component can return None sometimes. Such a component would be more generally useful with a Fragment, it can then return Element|Fragment and use return Fragment() instead of None. Then it would always be possible to render as str/iterate over it no matter where it is used.)

... so just adding the Fragment class would solve a lot of things? 😆

[pelme]

An idea, not sure it would be useful, we could add a protocol like:

class Component(typing.Protocol):
    def __str__(self) -> str: ...
    def __html__(self) -> Markup: ...
    def __iter__(self) -> Iterator[str]: ...

that could be documented as the preferred return value of all component functions pretty much everywhere.

from htpy import Component, Fragment, div
def inner_component() -> Component:
    return Fragment(
        div()["inner"],
        _feature_component(),
    )

It will guide you to wrap strings, lists, tuples or any other kind of Node types in a fragment to ensure all components can be composed and rendered.

[saeldion]

As an avid user of htpy I would not be opposed to a Fragment element/class to be introduced.

The Component protocol would be a nice addition, but I guess not strictly necessary.

[pelme]

Thinking more about the Component thing, it should probably not be a protocol, more like:

Component = BaseElement | Fragment | ContextProvider | ContextConsumer

so the Component type would be both 1) part of Node (can be a child element) and 2) having the ability to render by str()/ __html__() or iteration of chunks (without the need for render_node/iter_node).

[pelme]

Also comment() should then be Comment and be part of Component. Not sure how often one wants to use a comment like that but it should follow the same API.

Edit: comment() can also be changed to return a fragment instead: return Fragment(_Markup(f"<!-- {escaped_text} -->"))

[geigerzaehler]

Happy to see that you like the idea for a fragment. My two cents on the points that were raised.

An idea, not sure it would be useful, we could add a protocol like:

I think a protocol would be a useful way for users to extend the library. With such a protocol I could have implemented Fragment myself.

fragment should also then implement __iter__. that would make render_node/iter_node redundant and they could then be deprecated.

I think the protocol would need to include an _iter_node_context method. Without it the implementation would suffer from the context problem that originated this issue.

(Sometimes I use Node as return value, for instance if a component can return None sometimes. Such a component would be more generally useful with a Fragment, it can then return Element|Fragment and use return Fragment() instead of None. Then it would always be possible to render as str/iterate over it no matter where it is used.)

In my experience working with React-style code bases I found it more practical to use Node as a return type everywhere. I sometimes found myself changing the implementation of a component which required me to update the return type which occasionally might break consumer’s with types that required an element but only needed a node in practice. Using Node everywhere decreased churn, made APIs much more uniform and decreased mental overhead. With Component this can be implemented. And maybe that’s a pattern that could be used throughout the documentation.

Of course, this is just my experience, so just treat as some input into your decision.

[geigerzaehler]

I’d be happy to give the Fragment implementation and documentation a shot. (Leaving the component for a more thorough discussion)

[pelme]

I think the introduction of Fragment would make such a protocol for extending things unnecessary :) Custom stuff could then always be wrapped in Fragment to make it adhere to the Component type. I am not sure what other points of extensions would be useful.

Feel free to work on a Fragment implementation! :)