Replace ViewDeferredToRenderer, fix renderer tests
#408
Conversation
MaxDesiatov
changed the title
ViewDeferredToRenderer, fix renderer tests
| @@ -30,7 +30,7 @@ final class MountedApp<R: Renderer>: MountedCompositeElement<R> { | |||
| // They also have no parents, so the `parent` argument is discarded as well. | |||
| let childBody = reconciler.render(mountedApp: self) | |||
|
|
|||
| let child: MountedElement<R> = mountChild(childBody) | |||
| let child: MountedElement<R> = mountChild(reconciler.renderer, childBody) | |||
There was a problem hiding this comment.
renderer instances now need to be explicitly passed through in a lot more places. This allows mounted elements to call back to the renderer to determine whether a given view is primitive for this renderer.
| render(compositeElement: compositeView, body: \.view.view, result: \.view.bodyClosure) | ||
| let view = body(of: compositeView, keyPath: \.view.view) | ||
|
|
||
| guard let renderedBody = renderer.body(for: view) else { |
There was a problem hiding this comment.
I've only refactored ViewDeferredToRenderer for now. SceneDeferredToRenderer is kept as is to keep the PR small. And we didn't have problems with scenes (yet).
| bodyClosure = { AnyView(($0 as! V).body) } | ||
| } | ||
| // swiftlint:disable:next force_cast | ||
| bodyClosure = { AnyView(($0 as! V).body) } |
There was a problem hiding this comment.
So happy this mess in AnyView can be finally simplified!
| @@ -48,7 +48,7 @@ public struct Button<Label>: View where Label: View { | |||
| } | |||
| } | |||
|
|
|||
| public struct _Button<Label>: PrimitiveView where Label: View { | |||
| public struct _Button<Label>: _PrimitiveView where Label: View { | |||
There was a problem hiding this comment.
PrimitiveView is not meant to be re-exported by any renderer modules, making that explicit here with underscore naming.
| @@ -18,6 +18,149 @@ | |||
| import TokamakStaticHTML | |||
| import XCTest | |||
|
|
|||
| private let expectedHTML = | |||
There was a problem hiding this comment.
It would be nice to use https://github.com/pointfreeco/swift-snapshot-testing here instead, but we're running these tests in the Wasm environment, and file access is tricky there. Hardcoding the test fixture here for now.
Sources/TokamakCore/Renderer.swift
Outdated
| /** Returns a body of a given pritimive view, or `nil` if `view` is not a primitive view for | ||
| this renderer. | ||
| */ | ||
| func body(for view: Any) -> AnyView? |
There was a problem hiding this comment.
Would it make sense for this to be called primitiveBody(for:)? Then it’s more clear that it only returns the body of a primitive view?
This allows writing tests for
TokamakStaticHTML,TokamakDOM, andTokamakGTKtargets.The issue was caused by conflicting
ViewDeferredToRendererconformances declared in different modules, including theTokamakTestRenderermodule.This works around a general limitation in Swift, which was discussed at length on Swift Forums previously. When multiple conflicting conformances to the same protocol (
ViewDeferredToRendererin our case) exist in different modules, only one of them is available in a given binary (even a test binary). Also, only of them can be loaded and used. Which one exactly is loaded can't be known at compile-time, which is hard to debug and leads to breaking tests that cover code in different renderers. We had to disableTokamakStaticHTMLTestsfor this reason.The workaround is to declare two new functions in the
Rendererprotocol:Now each renderer can declare their own protocols for their primitive views, i.e.
HTMLPrimitive,DOMPrimitive,GTKPrimitiveetc, delegating to them from the implementations ofbody(for view:)andisPrimitiveView(_:). Conformances to these protocols can't conflict across different modules. Also, these protocols can haveinternalvisibility, as opposed toViewDeferredToRenderer, which had to be declared aspublicinTokamakCoreto be visible in renderer modules.