Attempt padding modifier fusion to avoid nested divs (#253)

This allows fusing nested `.padding` modifiers into a single `div` that sums up padding values from all these modifiers. Before: ```swift Text("text").padding(10).padding(20) ``` rendered to this (text styling omitted for brevity): ```html <div style="padding-top: 20.0px; padding-left: 20.0px; padding-bottom: 20.0px; padding-right: 20.0px;"> <div style="padding-top: 10.0px; padding-left: 10.0px; padding-bottom: 10.0px; padding-right: 10.0px;"> <span>text</span> </div> </div> ``` Now it renders as ```html <div style="padding-top: 30.0px; padding-left: 30.0px; padding-bottom: 30.0px; padding-right: 30.0px;"> <span>text</span> </div> ``` I hope this approach could be applied to other modifier combinations where it makes sense (in separate PRs). * Attempt `padding` modifier fusion * Fix linter warning * Add a test to verify that fusion works * Enable fusion of modifiers nested three times * Filter out empty attributes * Run snapshot tests only on macOS for now * Fully exclude snapshot testing on WASI * Fix `testOptional` snapshot * Clean up code formatting
TokamakUI · Jun 21, 2021 · e6c37a4 · e6c37a4
1 parent ae219e9
commit e6c37a4
Show file tree

Hide file tree

Showing 10 changed files with 493 additions and 165 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,17 +1,18 @@
 # See https://pre-commit.com for more information
 # See https://pre-commit.com/hooks.html for more hooks
+exclude: __Snapshots__
 repos:
--   repo: https://github.com/pre-commit/pre-commit-hooks
+  - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v2.5.0
     hooks:
-    -   id: trailing-whitespace
-    -   id: end-of-file-fixer
-    -   id: check-yaml
-    -   id: check-added-large-files
-    -   id: detect-private-key
-    -   id: check-merge-conflict
--   repo: https://github.com/hodovani/pre-commit-swift
+      - id: trailing-whitespace
+      - id: end-of-file-fixer
+      - id: check-yaml
+      - id: check-added-large-files
+      - id: detect-private-key
+      - id: check-merge-conflict
+  - repo: https://github.com/hodovani/pre-commit-swift
     rev: master
     hooks:
-    -   id: swift-lint
-    -   id: swift-format
+      - id: swift-lint
+      - id: swift-format
diff --git a/Package.resolved b/Package.resolved
@@ -6,8 +6,8 @@
         "repositoryURL": "https://github.com/swiftwasm/JavaScriptKit.git",
         "state": {
           "branch": null,
-          "revision": "ebd9ca04215397f0e3cb72d6e96406a980a424e5",
-          "version": "0.10.0"
+          "revision": "b19e7c8b10a2750ed47753e31ed13613171f3294",
+          "version": "0.10.1"
         }
       },
       {
@@ -33,8 +33,8 @@
         "repositoryURL": "https://github.com/apple/swift-argument-parser",
         "state": {
           "branch": null,
-          "revision": "9564d61b08a5335ae0a36f789a7d71493eacadfc",
-          "version": "0.3.2"
+          "revision": "986d191f94cec88f6350056da59c2e59e83d1229",
+          "version": "0.4.3"
         }
       },
       {
@@ -45,6 +45,15 @@
           "revision": "8e0ef8bb7482ab97dcd2cd1d6855bd38921c345d",
           "version": "0.1.0"
         }
+      },
+      {
+        "package": "SnapshotTesting",
+        "repositoryURL": "https://github.com/pointfreeco/swift-snapshot-testing.git",
+        "state": {
+          "branch": null,
+          "revision": "f8a9c997c3c1dab4e216a8ec9014e23144cbab37",
+          "version": "1.9.0"
+        }
       }
     ]
   },

diff --git a/Package.swift b/Package.swift
@@ -56,6 +56,11 @@ let package = Package(
     .package(url: "https://github.com/OpenCombine/OpenCombine.git", from: "0.12.0"),
     .package(url: "https://github.com/swiftwasm/OpenCombineJS.git", .upToNextMinor(from: "0.1.1")),
     .package(name: "Benchmark", url: "https://github.com/google/swift-benchmark", from: "0.1.0"),
+    .package(
+      name: "SnapshotTesting",
+      url: "https://github.com/pointfreeco/swift-snapshot-testing.git",
+      from: "1.9.0"
+    ),
   ],
   targets: [
     // Targets are the basic building blocks of a package. A target can define
@@ -180,7 +185,15 @@ let package = Package(
     ),
     .testTarget(
       name: "TokamakStaticHTMLTests",
-      dependencies: ["TokamakStaticHTML"]
+      dependencies: [
+        "TokamakStaticHTML",
+        .product(
+          name: "SnapshotTesting",
+          package: "SnapshotTesting",
+          condition: .when(platforms: [.macOS, .linux])
+        ),
+      ],
+      exclude: ["__Snapshots__"]
     ),
   ]
 )
diff --git a/Sources/TokamakCore/Modifiers/PaddingLayout.swift b/Sources/TokamakCore/Modifiers/PaddingLayout.swift
@@ -29,16 +29,31 @@ public struct _PaddingLayout: ViewModifier {
 }
 
 public extension View {
-  func padding(_ insets: EdgeInsets) -> some View {
+  func padding(_ insets: EdgeInsets) -> ModifiedContent<Self, _PaddingLayout> {
     modifier(_PaddingLayout(insets: insets))
   }
 
-  func padding(_ edges: Edge.Set = .all, _ length: CGFloat? = nil) -> some View {
+  func padding(
+    _ edges: Edge.Set = .all,
+    _ length: CGFloat? = nil
+  ) -> ModifiedContent<Self, _PaddingLayout> {
     let insets = length.map { EdgeInsets(_all: $0) }
     return modifier(_PaddingLayout(edges: edges, insets: insets))
   }
 
-  func padding(_ length: CGFloat) -> some View {
+  func padding(_ length: CGFloat) -> ModifiedContent<Self, _PaddingLayout> {
     padding(.all, length)
   }
 }
+
+public extension ModifiedContent where Modifier == _PaddingLayout, Content: View {
+  func padding(_ length: CGFloat) -> ModifiedContent<Content, _PaddingLayout> {
+    var layout = modifier
+    layout.insets?.top += length
+    layout.insets?.leading += length
+    layout.insets?.bottom += length
+    layout.insets?.trailing += length
+
+    return ModifiedContent(content: content, modifier: layout)
+  }
+}
diff --git a/Sources/TokamakCore/Reflection/Pointers/MetadataOffset.swift b/Sources/TokamakCore/Reflection/Pointers/MetadataOffset.swift
@@ -25,7 +25,7 @@ struct MetadataOffset<Pointee> {
 
   func apply(to ptr: UnsafeRawPointer) -> UnsafePointer<Pointee> {
     #if arch(wasm32)
-    return unsafeBitCast(offset, to: UnsafePointer<Pointee>.self)
+    return UnsafePointer<Pointee>(bitPattern: Int(offset))!
     #else
     return ptr.advanced(by: numericCast(offset)).assumingMemoryBound(to: Pointee.self)
     #endif

diff --git a/Sources/TokamakStaticHTML/Views/HTML.swift b/Sources/TokamakStaticHTML/Views/HTML.swift
@@ -58,7 +58,10 @@ public extension AnyHTML {
     if attributes.isEmpty {
       renderedAttributes = ""
     } else {
-      let mappedAttributes = attributes.map { #"\#($0)="\#($1)""# }
+      let mappedAttributes = attributes
+        // Exclude empty values to avoid waste of space with `class=""`
+        .filter { !$1.isEmpty }
+        .map { #"\#($0)="\#($1)""# }
       if shouldSortAttributes {
         renderedAttributes = mappedAttributes.sorted().joined(separator: " ")
       } else {

diff --git a/Tests/TokamakStaticHTMLTests/HTMLTests.swift b/Tests/TokamakStaticHTMLTests/HTMLTests.swift
@@ -15,153 +15,13 @@
 //  Created by Max Desiatov on 07/12/2018.
 //
 
+#if canImport(SnapshotTesting)
+
+import SnapshotTesting
 import TokamakStaticHTML
 import XCTest
 
-private let expectedHTML =
-  #"""
-  <html>
-  <head>
-    <title></title>
-    <style>
-      ._tokamak-stack > * {
-    flex-shrink: 0;
-  }
-  ._tokamak-scrollview-hideindicators {
-    scrollbar-color: transparent;
-    scrollbar-width: 0;
-  }
-  ._tokamak-scrollview-hideindicators::-webkit-scrollbar {
-    width: 0;
-    height: 0;
-  }
-  ._tokamak-list {
-    list-style: none;
-    overflow-y: auto;
-    width: 100%;
-    height: 100%;
-    padding: 0;
-  }
-  ._tokamak-disclosuregroup-label {
-    cursor: pointer;
-  }
-  ._tokamak-disclosuregroup-chevron-container {
-    width: .25em;
-    height: .25em;
-    padding: 10px;
-    display: inline-block;
-  }
-  ._tokamak-disclosuregroup-chevron {
-    width: 100%;
-    height: 100%;
-    transform: rotate(45deg);
-    border-right: solid 2px rgba(0, 0, 0, 0.25);
-    border-top: solid 2px rgba(0, 0, 0, 0.25);
-  }
-  ._tokamak-disclosuregroup-content {
-    display: flex;
-    flex-direction: column;
-    margin-left: 1em;
-  }
-  ._tokamak-buttonstyle-reset {
-    -webkit-appearance: none;
-    -moz-appearance: none;
-    appearance: none;
-    background: transparent;
-    border: none;
-    margin: 0;
-    padding: 0;
-    font-size: inherit;
-  }
-
-  ._tokamak-text-redacted {
-    position: relative;
-  }
-  ._tokamak-text-redacted::after {
-    content: " ";
-    background-color: rgb(200, 200, 200);
-    position: absolute;
-    left: 0;
-    width: calc(100% + .1em);
-    height: 1.2em;
-    border-radius: .1em;
-  }
-  ._tokamak-geometryreader {
-    width: 100%;
-    height: 100%;
-    display: flex;
-    align-items: center;
-    justify-content: center;
-  }
-  ._tokamak-navigationview {
-    display: flex;
-    flex-direction: row;
-    align-items: stretch;
-    width: 100%;
-    height: 100%;
-  }
-  ._tokamak-navigationview-content {
-    display: flex; flex-direction: column;
-    align-items: center; justify-content: center;
-    flex-grow: 1;
-    height: 100%;
-  }
-
-  ._tokamak-formcontrol {
-    color-scheme: light dark;
-  }
-
-  ._tokamak-link {
-    text-decoration: none;
-  }
-
-  ._tokamak-texteditor {
-    width: 100%;
-    height: 100%;
-  }
-
-  @media (prefers-color-scheme:dark) {
-    ._tokamak-text-redacted::after {
-      background-color: rgb(100, 100, 100);
-    }
-
-    ._tokamak-disclosuregroup-chevron {
-      border-right-color: rgba(255, 255, 255, 0.25);
-      border-top-color: rgba(255, 255, 255, 0.25);
-    }
-  }
-    </style>
-  </head>
-  <body style="margin: 0;display: flex;
-  width: 100%;
-  height: 100%;
-  justify-content: center;
-  align-items: center;
-  overflow: hidden;"><div class="_tokamak-stack" style="display: flex; flex-direction: column; align-items: center;
-  height: 100%;
-  "><span class="" style="
-  font-family: system,
-  -apple-system,
-  '.SFNSText-Regular',
-  'San Francisco',
-  'Roboto',
-  'Segoe UI',
-  'Helvetica Neue',
-  'Lucida Grande',
-  sans-serif;
-  color: rgba(0.0, 0.0, 0.0, 1.0);
-  font-style: normal;
-  font-weight: 400;
-  letter-spacing: normal;
-  vertical-align: baseline;
-  text-decoration: none;
-  text-decoration-color: inherit;
-  text-align: left;">text</span>
-  <div style="flex-grow: 1; "></div></div></body>
-  </html>
-  """#
-
-final class ReconcilerTests: XCTestCase {
+final class HTMLTests: XCTestCase {
   struct Model {
     let text: Text
   }
@@ -184,6 +44,22 @@ final class ReconcilerTests: XCTestCase {
     let resultingHTML = StaticHTMLRenderer(OptionalBody(model: Model(text: Text("text"))))
       .render(shouldSortAttributes: true)
 
-    XCTAssertEqual(resultingHTML, expectedHTML)
+    assertSnapshot(matching: resultingHTML, as: .lines)
+  }
+
+  func testPaddingFusion() {
+    let nestedTwice = StaticHTMLRenderer(
+      Text("text").padding(10).padding(20)
+    ).render(shouldSortAttributes: true)
+
+    assertSnapshot(matching: nestedTwice, as: .lines)
+
+    let nestedThrice = StaticHTMLRenderer(
+      Text("text").padding(20).padding(20).padding(20)
+    ).render(shouldSortAttributes: true)
+
+    assertSnapshot(matching: nestedThrice, as: .lines)
   }
 }
+
+#endif