`html_to_vdom` transform to remove html/body but preserve head content #832

Archmonger · 2022-11-09T08:13:44Z

There's currently some awkwardness in converting django views to VDOM, due to the fact that the head content is never loaded into the page.

For example, CSS/JS that is within <head> is simply never added to the page.

This PR makes html_to_vdom use the same approach browsers use when merging two HTML/body node trees. Namely, the contents of the <head> and <body> tags are directly spit out onto the page, while removing those top level tags themselves.

Checklist

Please update this checklist as you complete each item:

Tests have been included for all bug fixes or added functionality.
The changelog.rst has been updated with any significant changes.
GitHub Issues which may be closed by this Pull Request have been linked.

rmorshea · 2022-11-09T18:57:40Z

Could this be implemented as a transform instead? I suppose one could imagine using the VDOM data structures as a way to create normal HTML templates rather than injecting it into the page in the way IDOM does. Thus, retaining the head and body elements could be useful in that case.

Archmonger · 2022-11-09T22:29:21Z

I don't actually believe there's any use case for people templating HTML/Body/head tags within IDOM.

Using scripts outside of the <html> tag is not within HTML5 spec, so browsers are free to reject that behavior at any time. We shouldn't expect all browsers to support this.

And the <html> and <body> tag gets auto constructed (on a lot of browsers) if it doesn't exist upon first paint so we might get into some messy territory if we support that.

If we want to support <html>/<body> IDOM rendering, in my opinion that's a separate PR that will require some hacky JavaScript. Even then, I don't see a good cross-browser way to do this.

rmorshea · 2022-11-10T05:07:19Z

I don't think we should bake this into html_to_vdom. It's not clear to me that this approach to dealing with html/head/body is universally applicable. For example, in the future, we could allow users to declare head elements in IDOM by leveraging a tool like react-helmet. In which case, we'd actually want to preserve the head/body distinction. Thus I think this should be implemented as a transform. One that could be included in IDOM core given its broad, but not necessarily universal, appeal.

Archmonger · 2022-11-10T05:15:57Z

Given that helmet exists, I'm okay with that. On a different note, is there truly a purpose for us to keep the idom.html.html and idom.html.body representations?

rmorshea · 2022-11-10T05:48:54Z

We could, though there's not much cost to having them there either aside from the extra make_vdom_constructor call. If you feel strongly about it, I'd be ok removing them though.

Archmonger · 2022-11-14T05:02:09Z

It has been converted to a transform.

Archmonger · 2022-11-16T09:02:31Z

Let me know if deleting body is something you want to commit to. I personally think removing it helps set expectations, since currently I don't see any way to use it.

src/idom/utils.py

Archmonger · 2022-11-20T02:43:52Z

I didn't have it set to VdomDict because mypy didn't like that for some reason. Was easier to set as a generic dict than figure it out at the time.

If we wanna go back to VdomDict we'll need to fix that.

This reverts commit d78afdb.

…ontent

Archmonger · 2022-11-23T08:56:56Z

I reverted the VdomDict type hint change since I wasn't entirely sure what was wrong.

Let me know if you're comfortable with that.

rmorshea · 2022-11-30T04:19:39Z

Could this transform be more simply written as?

def del_html_body_transform(vdom: VdomDict) -> VdomDict:
    """Removes ``<html>``, ``<head>``, and ``<body>`` elements.

    This is accomplished by turning the aforementioned elements into fragments.
    See :func:`idom.html._` for more details on element fragments.

    .. note::

        Intended for use with :func:`html_to_vdom`.

    Parameters:
        vdom:
            The VDOM dictionary to transform.
    """
    return {**vdom, "tagName": ""} if vdom["tagName"] in {"html", "body", "head"} else vdom

Archmonger · 2022-11-30T05:50:42Z

Yeah.

Results in an uglier final VDOM with a bunch of fragments, but functionally would work the same when rendered.

…ontent

src/idom/utils.py

Archmonger added 7 commits November 9, 2022 00:13

Insert head content into html_to_vdom

975f5f2

more efficient node appending

77270a2

add changelog

a815efc

nodes -> body node

745a537

parsed_document -> html_node

ac7d948

remove useless has_root_node variable

025a42d

fix comment

a4a076f

Archmonger marked this pull request as ready for review November 9, 2022 09:09

Archmonger requested a review from rmorshea November 9, 2022 09:09

Archmonger added 11 commits November 12, 2022 17:17

del_html_body_transform

a70ddfa

fix type hint errors

bc890a4

type hint fixes 2

763ef8e

remove unusable head and body tags

0e8cbf7

uno mas

45b3c2e

import future annotations

9e740e8

fix docs warnings

24a788d

clean up last warning

ab3cb04

fix docstrings

ce2c972

del_html_body_transform docstrings

e5aba03

re-add head API

a348a98

Archmonger changed the title ~~Insert head content into html_to_vdom~~ html_to_vdom transform to remove html/body but preserve head content Nov 14, 2022

Archmonger added 2 commits November 14, 2022 12:39

fix changelog PR links

a0407d0

docstring cleanup

16285ff

rmorshea previously approved these changes Nov 19, 2022

View reviewed changes

rmorshea reviewed Nov 19, 2022

View reviewed changes

src/idom/utils.py Outdated Show resolved Hide resolved

rmorshea dismissed their stale review via d78afdb November 19, 2022 23:34

Better type hint

d78afdb

Archmonger added 2 commits November 23, 2022 00:52

Revert "Better type hint"

18e781b

This reverts commit d78afdb.

Merge remote-tracking branch 'upstream/main' into html_to_vdom_head_c…

b290afa

…ontent

Archmonger added 3 commits November 29, 2022 22:12

more concise transform implementation

2a9d2e2

Merge remote-tracking branch 'upstream/main' into html_to_vdom_head_c…

5b08771

…ontent

fix merge error

dcec026

Archmonger requested a review from rmorshea November 30, 2022 06:28

merge changelog better

bdd5f16

rmorshea reviewed Dec 1, 2022

View reviewed changes

src/idom/utils.py Outdated Show resolved Hide resolved

rmorshea added 2 commits December 1, 2022 10:58

annotate as VdomDict

e4267a5

Merge branch 'main' into html_to_vdom_head_content

54423f3

rmorshea merged commit 9c3c300 into reactive-python:main Dec 1, 2022

Archmonger deleted the html_to_vdom_head_content branch December 2, 2022 08:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`html_to_vdom` transform to remove html/body but preserve head content #832

`html_to_vdom` transform to remove html/body but preserve head content #832

Archmonger commented Nov 9, 2022 •

edited

Loading

rmorshea commented Nov 9, 2022 •

edited

Loading

Archmonger commented Nov 9, 2022 •

edited

Loading

rmorshea commented Nov 10, 2022

Archmonger commented Nov 10, 2022

rmorshea commented Nov 10, 2022 •

edited

Loading

Archmonger commented Nov 14, 2022

Archmonger commented Nov 16, 2022

Archmonger commented Nov 20, 2022 •

edited

Loading

Archmonger commented Nov 23, 2022 •

edited

Loading

rmorshea commented Nov 30, 2022 •

edited

Loading

Archmonger commented Nov 30, 2022

html_to_vdom transform to remove html/body but preserve head content #832

html_to_vdom transform to remove html/body but preserve head content #832

Conversation

Archmonger commented Nov 9, 2022 • edited Loading

Checklist

rmorshea commented Nov 9, 2022 • edited Loading

Archmonger commented Nov 9, 2022 • edited Loading

rmorshea commented Nov 10, 2022

Archmonger commented Nov 10, 2022

rmorshea commented Nov 10, 2022 • edited Loading

Archmonger commented Nov 14, 2022

Archmonger commented Nov 16, 2022

Archmonger commented Nov 20, 2022 • edited Loading

Archmonger commented Nov 23, 2022 • edited Loading

rmorshea commented Nov 30, 2022 • edited Loading

Archmonger commented Nov 30, 2022

`html_to_vdom` transform to remove html/body but preserve head content #832

`html_to_vdom` transform to remove html/body but preserve head content #832

Archmonger commented Nov 9, 2022 •

edited

Loading

rmorshea commented Nov 9, 2022 •

edited

Loading

Archmonger commented Nov 9, 2022 •

edited

Loading

rmorshea commented Nov 10, 2022 •

edited

Loading

Archmonger commented Nov 20, 2022 •

edited

Loading

Archmonger commented Nov 23, 2022 •

edited

Loading

rmorshea commented Nov 30, 2022 •

edited

Loading