Add Fragment node for node grouping

[geigerzaehler]

A very basic implementation of Fragment as discussed in #82.

[pelme]

With the introduction of fragment, I would like to deprecate render_node / iter_node (but keep them around for a long time). That can be done in a separate PR though!

[geigerzaehler]

Thanks for the quick feedback. I’ve updated the PR accordingly

[pelme]

• this should be Iterable[str] so that it works exactly like iterating over an element.
• iter_node should iterater over x._nodes

We could add a private Protocol called _Renderable that we can use internally that requires __str__, __html__ and __iter__ (Iterator[str]). That can be a separate PR. Maybe not all other objects fully implement this either. The render() fixture would accept _Renderable as its argument. then it is consistent that all htpy provided objects can be converted to str/html/iterated over.

[geigerzaehler]

I see. I have to say that implementing __iter__ seems confusing and redundant. For the nodes it is implement for we always have __iter__ == iter_node. And there are two ways to stream the output: for chunk in iter_node(node) or for chunk in node. Only the latter does not always work as expected. IMHO it would improve the API if there was no __iter__.

[pelme]

a big motivation adding Fragment to me is that it makes render_node and iter_node redundant. the only interface you would need to work with htpy stuff is then str(element / context provider / context consumer / fragment), (element / context provider / context consumer / fragment).__html__() (typically via a template engine) or for chunk in element / context provider / context consumer / fragment: and it would just work. if you have a plain string, None or one of the other Node types, you wrap it in a Fragment instead of render_node, then it follows the same API and the same rules as everything else.

if we keep render_node and iter_node then I do not really see the point of Fragment. I see Fragment as a more elegant solution to the problem that render_node solves. if we keep render_node/iter_node then we have multiple ways to achieve the same thing which makes it worse/more confusing to me. I think str(Fragment(x)) and for chunk in Fragment(x) is cleaner than render_node(x) / iter_node(x). the power of fragments is that they both can be roots with str():ed and child elements at the same time.

I also think Node is not a good name for htpy and I like your idea of renaming it to Child. In react a, any node, including null and strings can be rendered with ReactDOM.render(). There is some render function that can deal with it. But in Python/htpy with have the str(), __html__() and iteration protocol which makes more sense. But currently htpy also have render_node/iter_node which is annoying and basically the same way of doing it.

Adding fragment is just the first step of fixing this:

1. Add Fragment (this PR)
2. Emit deprecation warnings from render_node+iter_node but keep them.
3. Rename the Node type to Child in code+only show Child in the docs. Using Node is deprecated.
4. Add a Component type that is BaseElement | Fragment | ContextConsumer | ContextProvider. Component means that it can be rendered. (I am not fully sure about naming it Component, maybe there would be a better name. Or maybe it is not needed and just using the other types is fine.)

does that make sense? 😆

[saeldion]

I think all this sounds good. And I think a union type like Component would be nice and would be okay with the name.

[geigerzaehler]

Sorry for the delayed response. I totally agree with your point that there should be one way to do render_node or iter_node. I don’t think I made this clear but that was one of my original points. I also don’t want to do away with implementing __str__ and __html__. Being able to reference a component in a template and have it render is convenient.

My point was that I found the use of the Iterable protocol for streaming rendering didn’t match my expectation and was confusing. I thought that iter(node) would produce an iterator over elements, like a generic tree data structure. I didn’t expect it to render the node in chunks.

A big part of this confusion is that for n in node and for n in iter_node(node) behave differently when node is not a component (str, tuple, None, etc.) but it’s always the behavior of iter_node that you want.

My suggestion would be to remove the __iter__ implementation and use a dedicated method like component.render_chunks() or component.render_iter(). This would make the code more verbose (for chunk in component.render_chunks() instead of for chunk in component, but I think being explicit here is actually good. What do you think?

---

I’m not sure either whether Component is the right term. A component is usually more like a function that requires some inputs before it can be rendered (although the number of inputs may be zero). In react this is reflected because you can’t call render(component), instead you call render(createElement(component, args)). What do you think about Tree or Element?

[pelme]

I agree with all your points:

• I never considered __iter__() strange but I agree it would be clearer to use a dedicated method. I would suggest naming it stream_chunks()
• Maybe the name we are looking for is Renderable? Renderable would be a Protocol that defines stream_chunks(), __html__, __str__. BaseElement, Fragment, ContextProvider, ContextConsumer would implement it.

This is my plan to address it (each step a separate PR):

1. Introduce the Fragment class (this PR)
2. Add the Renderable protocol and ensure that BaseElement, Fragment, ContextProvider, ContextConsumer all implements it.
3. Deprecate the __iter__() method, render_node(), iter_node().
4. To make streaming more straightforward and less weird, add Django/Starlette/Flask response classes that uses streaming+recommend those in the docs. htpy.django.HtpyResponse would subclass Django StreamingHttpResponse and accept a Renderable as argument and then call stream_chunks().

[pelme]

the comment function should return a Fragment instead of Markup too. that makes it possible to render it directly

[geigerzaehler]

I’ve addressed your comments and rebased the branch