Skip to content

Commit

Permalink
Sort attributes in HTML nodes when rendering (#346)
Browse files Browse the repository at this point in the history
This makes attributes order deterministic and allows testing against HTML renderer output, while currently attributes order is random.

Benchmarks results:

```
name                            time            std        iterations
---------------------------------------------------------------------
render Text                         9667.000 ns ±   4.35 %     145213
render App unsorted attributes     51917.000 ns ±   4.23 %      26835
render App sorted attributes       52375.000 ns ±   1.62 %      26612
render List unsorted attributes 34546833.500 ns ±   0.79 %         40
render List sorted attributes   34620000.500 ns ±   0.69 %         40
```

Looks like on average there's ~0.2% difference in performance. I was leaning towards enabling sorting by default, but we're benchmarking here only with short attribute dictionaries, I wonder if the difference could become prominent for elements with more attributes. I kept sorting disabled by default after all, but still configurable.

`var html: String` on `StaticHTMLRenderer` was changed to `func render(shouldSortAttributes: Bool = false) -> String` to allow configuring this directly.

* Sort attributes in HTML nodes when rendering

* Make sorting configurable, add benchmarks

* Disable sorting by default, clean up product name

* Fix build errors
  • Loading branch information
MaxDesiatov authored Jun 6, 2021
1 parent 7775977 commit 4428084
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 38 deletions.
6 changes: 3 additions & 3 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ let package = Package(
targets: ["TokamakStaticHTML"]
),
.executable(
name: "TokamakStaticDemo",
targets: ["TokamakStaticDemo"]
name: "TokamakStaticHTMLDemo",
targets: ["TokamakStaticHTMLDemo"]
),
.library(
name: "TokamakGTK",
Expand Down Expand Up @@ -159,7 +159,7 @@ let package = Package(
resources: [.copy("logo-header.png")]
),
.target(
name: "TokamakStaticDemo",
name: "TokamakStaticHTMLDemo",
dependencies: [
"TokamakStaticHTML",
]
Expand Down
2 changes: 1 addition & 1 deletion Sources/TokamakDOM/DOMNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ extension AnyHTML {
dom.reinstall(dynamicSelf.listeners)
}

guard let innerHTML = innerHTML else { return }
guard let innerHTML = innerHTML(shouldSortAttributes: false) else { return }
dom.ref.innerHTML = .string(innerHTML)
}
}
Expand Down
10 changes: 8 additions & 2 deletions Sources/TokamakDOM/DOMRenderer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,16 @@ final class DOMRenderer: Renderer {

let maybeNode: JSObject?
if let sibling = sibling {
_ = sibling.ref.insertAdjacentHTML!("beforebegin", anyHTML.outerHTML)
_ = sibling.ref.insertAdjacentHTML!(
"beforebegin",
anyHTML.outerHTML(shouldSortAttributes: false, children: [])
)
maybeNode = sibling.ref.previousSibling.object
} else {
_ = parent.ref.insertAdjacentHTML!("beforeend", anyHTML.outerHTML)
_ = parent.ref.insertAdjacentHTML!(
"beforeend",
anyHTML.outerHTML(shouldSortAttributes: false, children: [])
)

guard
let children = parent.ref.childNodes.object,
Expand Down
2 changes: 1 addition & 1 deletion Sources/TokamakDOM/Views/Buttons/Button.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ extension _Button: ViewDeferredToRenderer where Label == Text {
["class": "_tokamak-buttonstyle-default"],
listeners: listeners
) {
HTML("span", content: label.innerHTML ?? "")
HTML("span", content: label.innerHTML(shouldSortAttributes: false) ?? "")
})
} else {
return AnyView(DynamicHTML(
Expand Down
10 changes: 7 additions & 3 deletions Sources/TokamakDOM/Views/DynamicHTML.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ public struct DynamicHTML<Content>: View, AnyDynamicHTML {
public let listeners: [String: Listener]
let content: Content

public var innerHTML: String?
fileprivate let cachedInnerHTML: String?

public func innerHTML(shouldSortAttributes: Bool) -> String? {
cachedInnerHTML
}

@_spi(TokamakCore)
public var body: Never {
Expand All @@ -52,7 +56,7 @@ public extension DynamicHTML where Content: StringProtocol {
self.attributes = attributes
self.listeners = listeners
self.content = content
innerHTML = String(content)
cachedInnerHTML = String(content)
}
}

Expand All @@ -67,7 +71,7 @@ extension DynamicHTML: ParentView where Content: View {
self.attributes = attributes
self.listeners = listeners
self.content = content()
innerHTML = nil
cachedInnerHTML = nil
}

@_spi(TokamakCore)
Expand Down
17 changes: 6 additions & 11 deletions Sources/TokamakStaticHTML/StaticRenderer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,14 @@ public final class HTMLTarget: Target {
}

extension HTMLTarget {
var outerHTML: String {
"""
<\(html.tag)\(html.attributes.isEmpty ? "" : " ")\
\(html.attributes.map { #"\#($0)="\#($1)""# }.joined(separator: " "))>\
\(html.innerHTML ?? "")\
\(children.map(\.outerHTML).joined(separator: "\n"))\
</\(html.tag)>
"""
func outerHTML(shouldSortAttributes: Bool) -> String {
html.outerHTML(shouldSortAttributes: shouldSortAttributes, children: children)
}
}

struct HTMLBody: AnyHTML {
let tag: String = "body"
let innerHTML: String? = nil
public func innerHTML(shouldSortAttributes: Bool) -> String? { nil }
let attributes: [HTMLAttribute: String] = [
"style": "margin: 0;" + rootNodeStyles,
]
Expand All @@ -70,7 +64,8 @@ public final class StaticHTMLRenderer: Renderer {
var rootTarget: HTMLTarget

static var title: String = ""
public var html: String {

public func render(shouldSortAttributes: Bool = false) -> String {
"""
<html>
<head>
Expand All @@ -79,7 +74,7 @@ public final class StaticHTMLRenderer: Renderer {
\(tokamakStyles)
</style>
</head>
\(rootTarget.outerHTML)
\(rootTarget.outerHTML(shouldSortAttributes: shouldSortAttributes))
</html>
"""
}
Expand Down
34 changes: 26 additions & 8 deletions Sources/TokamakStaticHTML/Views/HTML.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,30 @@ extension HTMLAttribute: ExpressibleByStringLiteral {
}

public protocol AnyHTML {
var innerHTML: String? { get }
func innerHTML(shouldSortAttributes: Bool) -> String?
var tag: String { get }
var attributes: [HTMLAttribute: String] { get }
}

public extension AnyHTML {
var outerHTML: String {
"""
func outerHTML(shouldSortAttributes: Bool, children: [HTMLTarget]) -> String {
let renderedAttributes: String
if attributes.isEmpty {
renderedAttributes = ""
} else {
let mappedAttributes = attributes.map { #"\#($0)="\#($1)""# }
if shouldSortAttributes {
renderedAttributes = mappedAttributes.sorted().joined(separator: " ")
} else {
renderedAttributes = mappedAttributes.joined(separator: " ")
}
}

return """
<\(tag)\(attributes.isEmpty ? "" : " ")\
\(attributes.map { #"\#($0)="\#($1)""# }.joined(separator: " "))>\
\(innerHTML ?? "")\
\(renderedAttributes)>\
\(innerHTML(shouldSortAttributes: shouldSortAttributes) ?? "")\
\(children.map { $0.outerHTML(shouldSortAttributes: shouldSortAttributes) }.joined(separator: "\n"))\
</\(tag)>
"""
}
Expand All @@ -68,7 +81,12 @@ public struct HTML<Content>: View, AnyHTML {
public let attributes: [HTMLAttribute: String]
let content: Content

public let innerHTML: String?
fileprivate let cachedInnerHTML: String?

public func innerHTML(shouldSortAttributes: Bool) -> String? {
cachedInnerHTML
}


@_spi(TokamakCore)
public var body: Never {
Expand All @@ -85,7 +103,7 @@ public extension HTML where Content: StringProtocol {
self.tag = tag
self.attributes = attributes
self.content = content
innerHTML = String(content)
cachedInnerHTML = String(content)
}
}

Expand All @@ -98,7 +116,7 @@ extension HTML: ParentView where Content: View {
self.tag = tag
self.attributes = attributes
self.content = content()
innerHTML = nil
cachedInnerHTML = nil
}

@_spi(TokamakCore)
Expand Down
2 changes: 1 addition & 1 deletion Sources/TokamakStaticHTML/Views/Spacers/Divider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
@_spi(TokamakCore) import TokamakCore

extension Divider: AnyHTML {
public var innerHTML: String? { nil }
public func innerHTML(shouldSortAttributes: Bool) -> String? { nil }
public var tag: String { "hr" }
public var attributes: [HTMLAttribute: String] {
[
Expand Down
6 changes: 3 additions & 3 deletions Sources/TokamakStaticHTML/Views/Text/Text.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ private struct TextSpan: AnyHTML {
let content: String
let attributes: [HTMLAttribute: String]

var innerHTML: String? { content }
public func innerHTML(shouldSortAttributes: Bool) -> String? { content }
var tag: String { "span" }
}

extension Text: AnyHTML {
public var innerHTML: String? {
public func innerHTML(shouldSortAttributes: Bool) -> String? {
let proxy = _TextProxy(self)
let innerHTML: String
switch proxy.storage {
Expand All @@ -126,7 +126,7 @@ extension Text: AnyHTML {
environment: proxy.environment
)
)
.outerHTML
.outerHTML(shouldSortAttributes: shouldSortAttributes, children: [])
}
.reduce("", +)
}
Expand Down
16 changes: 12 additions & 4 deletions Sources/TokamakStaticHTMLBenchmark/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,20 @@ struct BenchmarkApp: App {
}
}

benchmark("render App") {
_ = StaticHTMLRenderer(BenchmarkApp())
benchmark("render App unsorted attributes") {
_ = StaticHTMLRenderer(BenchmarkApp()).render(shouldSortAttributes: false)
}

benchmark("render List") {
_ = StaticHTMLRenderer(List(1..<100) { Text("\($0)") })
benchmark("render App sorted attributes") {
_ = StaticHTMLRenderer(BenchmarkApp()).render(shouldSortAttributes: true)
}

benchmark("render List unsorted attributes") {
_ = StaticHTMLRenderer(List(1..<100) { Text("\($0)") }).render(shouldSortAttributes: false)
}

benchmark("render List sorted attributes") {
_ = StaticHTMLRenderer(List(1..<100) { Text("\($0)") }).render(shouldSortAttributes: true)
}

Benchmark.main()
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ struct TestApp: App {
}
}

print(StaticHTMLRenderer(TestApp()).html)
print(StaticHTMLRenderer(TestApp()).render(shouldSortAttributes: true))

0 comments on commit 4428084

Please sign in to comment.