sliemeobn / elementary

A modern and efficient HTML rendering library - inspired by SwiftUI, built for the web.
https://swiftpackageindex.com/sliemeobn/elementary
Apache License 2.0
146 stars 6 forks source link

expose `_RenderingContext.emptyContext` #45

Closed alephao closed 1 month ago

alephao commented 1 month ago

This change makes the static var emptyContext public, so I can use it in my custom renderer.

That's necessary because when creating a custom renderer, It's impossible to run HTML._render since one of its arguments is a _RenderingContext and it doesn't expose initializers.

sliemeobn commented 1 month ago

I mean, I guess there is no harm in this, but for more fully supporting "custom rendering" outside this package we'd need to think strategy.

also, be aware that underscored stuff might be broken (as it is by convention not part of the API contract)

can I ask what you are implementing? is it something that should go in the package?

alephao commented 1 month ago

can I ask what you are implementing?

Sure, I'm implementing a renderer that outputs a more snapshot-friendly html, like this.

I use snapshot testing for 100% of my HTML tests and also HTTP tests that outputs HTML, and the default pretty format isn't very snapshot friendly compared to the example above. For instance, if I have an <input> with a bunch of attributes, and later decide to change only one of the attributes, my snapshot tests would show that only the placeholder line changed.

Example diffs:

With the default pretty print

- <input a="b" b="c" c="l" d="e">
+ <input a="b" b="c" c="I" d="e">

With diff-friendly

<input a="b"
       b="c"
-      c="l"
+      c="I"
       d="e">

is it something that should go in the package?

IDK if it fits the core lib, my use-case is to use this specifically with the swift-snapshot-testing library.

sliemeobn commented 1 month ago

I see, I can tell you that writing a correct and efficient html-formatter is quite tough ; ) I essentially gave up, because the current pretty-printer has bugs and I thought about removing it.

however, I think your use-case is probably the best reason to have something like pretty-print at all, so I would not mind if we add a formatting option to the renderFormatted() that fits your needs. I could see this being useful for many people.

if you are up for it that is, I have no issue with this one change here.

sliemeobn commented 1 month ago

to clarify, the tricky bit is that adding line breaks within "inline" elements (say between <span> or <i>) would produce a different output, because the browser interprets a line break as whitespace and adds a blank basically...

alephao commented 1 month ago

to clarify, the tricky bit is that adding line breaks within "inline" elements (say between or ) would produce a different output, because the browser interprets a line break as whitespace and adds a blank basically...

That's one thing I don't really care for my use-case, which makes the implementation much easier. I don't want the renderer to try doing it in an optimal way that cares about browsers quirks. I'm only using it for snapshot tests, not for rendering on the browser. In fact, I'll never send that HTML to the browser, not sure why anyone would.

What I need is to keep the rendered html as close as possible to what is actually sent, but adding a bunch of predictable break-lines to help with diff reading. If I remove/add spaces to my html to fix some weird rendering, the snapshot will have that extra/removed space in the diff as well, that's the behavior I want.

What do you think? If you agree with that approach, then I already have this like 80% implemented in my fork, I can send another PR soon

sliemeobn commented 1 month ago

I'll merge your change, we can always figure out how to deal with formatting in the core package later.