-
Notifications
You must be signed in to change notification settings - Fork 47
Add WorkflowSwiftUIExperimental #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
require_relative('version') | ||
|
||
Pod::Spec.new do |s| | ||
s.name = 'WorkflowSwiftUIExperimental' | ||
s.version = '0.1' | ||
s.summary = 'Infrastructure for Workflow-powered SwiftUI' | ||
s.homepage = 'https://www.github.com/square/workflow-swift' | ||
s.license = 'Apache License, Version 2.0' | ||
s.author = 'Square' | ||
s.source = { :git => 'https://github.com/square/workflow-swift.git', :tag => "swiftui-experimental/v#{s.version}" } | ||
|
||
# 1.7 is needed for `swift_versions` support | ||
s.cocoapods_version = '>= 1.7.0' | ||
|
||
s.swift_versions = [WORKFLOW_SWIFT_VERSION] | ||
s.ios.deployment_target = WORKFLOW_IOS_DEPLOYMENT_TARGET | ||
s.osx.deployment_target = WORKFLOW_MACOS_DEPLOYMENT_TARGET | ||
|
||
s.source_files = 'WorkflowSwiftUIExperimental/Sources/*.swift' | ||
|
||
s.dependency 'Workflow', WORKFLOW_VERSION | ||
s.dependency 'WorkflowUI', WORKFLOW_VERSION | ||
|
||
s.pod_target_xcconfig = { 'APPLICATION_EXTENSION_API_ONLY' => 'YES' } | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# WorkflowSwiftUIExperimental | ||
|
||
Experimental extensions to Workflow for writing Screens in SwiftUI. | ||
|
||
## Versioning | ||
|
||
Because this module is experimental, it is versioned separately from other modules in Workflow. You should bump its version as part of any pull request that changes it, and need not bump its version in PRs that change only other modules. | ||
|
||
Per semantic versioning, its major version remains at `0`, and only its minor version is incremented. Any increase in the minor version may come with breaking changes. | ||
|
||
To bump the minor version, update `s.version` in `WorkflowSwiftUIExperimental.podspec`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* | ||
* Copyright 2023 Square Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import SwiftUI | ||
import WorkflowUI | ||
|
||
private struct ViewEnvironmentKey: EnvironmentKey { | ||
static let defaultValue: ViewEnvironment = .empty | ||
} | ||
|
||
public extension EnvironmentValues { | ||
var viewEnvironment: ViewEnvironment { | ||
get { self[ViewEnvironmentKey.self] } | ||
set { self[ViewEnvironmentKey.self] = newValue } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/* | ||
* Copyright 2023 Square Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#if canImport(UIKit) | ||
|
||
import SwiftUI | ||
|
||
public extension ObservableValue { | ||
func binding<T>( | ||
get: @escaping (Value) -> T, | ||
set: @escaping (Value) -> (T) -> Void | ||
) -> Binding<T> { | ||
// This convoluted way of creating a `Binding`, relative to `Binding.init(get:set:)`, is | ||
// a workaround borrowed from TCA for a SwiftUI issue: | ||
// https://github.com/pointfreeco/swift-composable-architecture/pull/770 | ||
ObservedObject(wrappedValue: self) | ||
.projectedValue[get: .init(rawValue: get), set: .init(rawValue: set)] | ||
} | ||
|
||
private subscript<T>( | ||
get get: HashableWrapper<(Value) -> T>, | ||
set set: HashableWrapper<(Value) -> (T) -> Void> | ||
) -> T { | ||
get { get.rawValue(value) } | ||
set { set.rawValue(value)(newValue) } | ||
} | ||
|
||
private struct HashableWrapper<Value>: Hashable { | ||
let rawValue: Value | ||
static func == (lhs: Self, rhs: Self) -> Bool { false } | ||
func hash(into hasher: inout Hasher) {} | ||
} | ||
Comment on lines
+41
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 what is going on here? is this like to force some internal SwiftUI system to always treat things as having changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Subscript parameter types are required to be Hashable in order that keypaths can be Hashable. Since our parameter types are functions, they are not actually equatable, and it is safest for our |
||
} | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
/* | ||
* Copyright 2023 Square Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import Combine | ||
import Workflow | ||
|
||
@dynamicMemberLookup | ||
public final class ObservableValue<Value>: ObservableObject { | ||
private var internalValue: Value | ||
private let subject = PassthroughSubject<Value, Never>() | ||
private var cancellable: AnyCancellable? | ||
private let isEquivalent: ((Value, Value) -> Bool)? | ||
|
||
public private(set) var value: Value { | ||
get { | ||
return internalValue | ||
} | ||
set { | ||
subject.send(newValue) | ||
} | ||
} | ||
|
||
public private(set) lazy var objectWillChange = ObservableObjectPublisher() | ||
private var parentCancellable: AnyCancellable? | ||
|
||
public static func makeObservableValue( | ||
_ value: Value, | ||
isEquivalent: ((Value, Value) -> Bool)? = nil | ||
) -> (ObservableValue, Sink<Value>) { | ||
let observableValue = ObservableValue(value: value, isEquivalent: isEquivalent) | ||
let sink = Sink { newValue in | ||
observableValue.value = newValue | ||
} | ||
|
||
return (observableValue, sink) | ||
} | ||
|
||
private init(value: Value, isEquivalent: ((Value, Value) -> Bool)?) { | ||
self.internalValue = value | ||
self.isEquivalent = isEquivalent | ||
self.cancellable = valuePublisher() | ||
.dropFirst() | ||
.sink { [weak self] newValue in | ||
guard let self = self else { return } | ||
self.objectWillChange.send() | ||
self.internalValue = newValue | ||
} | ||
// Allows removeDuplicates operator to have the initial value. | ||
subject.send(value) | ||
} | ||
|
||
//// Scopes the ObservableValue to a subset of Value to LocalValue given the supplied closure while allowing to optionally remove duplicates. | ||
/// - Parameters: | ||
/// - toLocalValue: A closure that takes a Value and returns a LocalValue. | ||
/// - isEquivalent: An optional closure that checks to see if a LocalValue is equivalent. | ||
/// - Returns: a scoped ObservableValue of LocalValue. | ||
public func scope<LocalValue>(_ toLocalValue: @escaping (Value) -> LocalValue, isEquivalent: ((LocalValue, LocalValue) -> Bool)? = nil) -> ObservableValue<LocalValue> { | ||
return scopeToLocalValue(toLocalValue, isEquivalent: isEquivalent) | ||
} | ||
|
||
/// Scopes the ObservableValue to a subset of Value to LocalValue given the supplied closure and removes duplicate values using Equatable. | ||
/// - Parameter toLocalValue: A closure that takes a Value and returns a LocalValue. | ||
/// - Returns: a scoped ObservableValue of LocalValue. | ||
public func scope<LocalValue>(_ toLocalValue: @escaping (Value) -> LocalValue) -> ObservableValue<LocalValue> where LocalValue: Equatable { | ||
return scopeToLocalValue(toLocalValue, isEquivalent: { $0 == $1 }) | ||
} | ||
|
||
/// Returns the value at the given keypath of ``Value``. | ||
/// | ||
/// In combination with `@dynamicMemberLookup`, this allows us to write `model.myProperty` instead of | ||
/// `model.value.myProperty` where `model` has type `ObservableValue<T>`. | ||
public subscript<T>(dynamicMember keyPath: KeyPath<Value, T>) -> T { | ||
internalValue[keyPath: keyPath] | ||
} | ||
|
||
private func scopeToLocalValue<LocalValue>(_ toLocalValue: @escaping (Value) -> LocalValue, isEquivalent: ((LocalValue, LocalValue) -> Bool)? = nil) -> ObservableValue<LocalValue> { | ||
let localObservableValue = ObservableValue<LocalValue>( | ||
value: toLocalValue(internalValue), | ||
isEquivalent: isEquivalent | ||
) | ||
localObservableValue.parentCancellable = valuePublisher().sink(receiveValue: { newValue in | ||
localObservableValue.value = toLocalValue(newValue) | ||
}) | ||
return localObservableValue | ||
} | ||
|
||
private func valuePublisher() -> AnyPublisher<Value, Never> { | ||
guard let isEquivalent = isEquivalent else { | ||
return subject.eraseToAnyPublisher() | ||
} | ||
|
||
return subject.removeDuplicates(by: isEquivalent).eraseToAnyPublisher() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
/* | ||
* Copyright 2023 Square Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#if canImport(UIKit) | ||
|
||
import SwiftUI | ||
import Workflow | ||
import WorkflowUI | ||
|
||
public protocol SwiftUIScreen: Screen { | ||
associatedtype Content: View | ||
|
||
@ViewBuilder | ||
static func makeView(model: ObservableValue<Self>) -> Content | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where can I read about why this is a static? I don't love this, honestly... The ergonomics feel off to me. I guess so you can force folks to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's possible for this to be an instance method if we are directly mapping Screen values to View values. However, it's not clear how it can be an instance method when we want to use However, I think we should take another pass at this soon. You're not the first to be repulsed by it, and some of the rationale may be stale in iOS 15. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took another stab at conforming the rendering directly to View, eliminating the use of In iOS 16 at least, I'm not seeing the animation issues (e.g. in Toggle) that we observed at the time of the design doc. This approach might lead to more view body evaluations, but after my performance noodling last month, I think those evaluations are cheap as long as they don't trigger additional work in the Core Animation commit phase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opening a draft PR from that branch: #253 |
||
|
||
static var isEquivalent: ((Self, Self) -> Bool)? { get } | ||
} | ||
|
||
public extension SwiftUIScreen { | ||
static var isEquivalent: ((Self, Self) -> Bool)? { return nil } | ||
} | ||
|
||
public extension SwiftUIScreen where Self: Equatable { | ||
static var isEquivalent: ((Self, Self) -> Bool)? { { $0 == $1 } } | ||
} | ||
|
||
public extension SwiftUIScreen { | ||
func viewControllerDescription(environment: ViewEnvironment) -> ViewControllerDescription { | ||
ViewControllerDescription( | ||
type: ModeledHostingController<Self, WithModel<Self, EnvironmentInjectingView<Content>>>.self, | ||
environment: environment, | ||
build: { | ||
let (model, modelSink) = ObservableValue.makeObservableValue(self, isEquivalent: Self.isEquivalent) | ||
let (viewEnvironment, envSink) = ObservableValue.makeObservableValue(environment) | ||
return ModeledHostingController( | ||
modelSink: modelSink, | ||
viewEnvironmentSink: envSink, | ||
rootView: WithModel(model, content: { model in | ||
EnvironmentInjectingView( | ||
viewEnvironment: viewEnvironment, | ||
content: Self.makeView(model: model) | ||
) | ||
}) | ||
) | ||
}, | ||
update: { | ||
$0.modelSink.send(self) | ||
$0.viewEnvironmentSink.send(environment) | ||
} | ||
) | ||
} | ||
} | ||
|
||
private struct EnvironmentInjectingView<Content: View>: View { | ||
@ObservedObject var viewEnvironment: ObservableValue<ViewEnvironment> | ||
let content: Content | ||
|
||
var body: some View { | ||
content | ||
.environment(\.viewEnvironment, viewEnvironment.value) | ||
} | ||
} | ||
|
||
private final class ModeledHostingController<Model, Content: View>: UIHostingController<Content> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For my own understanding, this is so we can basically isolate each VC into its own SwiftUI hierarchy? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think of it as simply: we need some view controller class for |
||
let modelSink: Sink<Model> | ||
let viewEnvironmentSink: Sink<ViewEnvironment> | ||
|
||
init(modelSink: Sink<Model>, viewEnvironmentSink: Sink<ViewEnvironment>, rootView: Content) { | ||
self.modelSink = modelSink | ||
self.viewEnvironmentSink = viewEnvironmentSink | ||
|
||
super.init(rootView: rootView) | ||
} | ||
|
||
required init?(coder aDecoder: NSCoder) { | ||
fatalError("not implemented") | ||
} | ||
} | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* Copyright 2023 Square Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import SwiftUI | ||
|
||
struct WithModel<Model, Content: View>: View { | ||
@ObservedObject private var model: ObservableValue<Model> | ||
private let content: (ObservableValue<Model>) -> Content | ||
|
||
init( | ||
_ model: ObservableValue<Model>, | ||
@ViewBuilder content: @escaping (ObservableValue<Model>) -> Content | ||
) { | ||
self.model = model | ||
self.content = content | ||
} | ||
|
||
var body: Content { | ||
content(model) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the read/write tracking that SwiftUI does for environment values work with this one big ol' env block? Eg, since the environment isn't equatable, will it just always assume the value changed and re-render anything that read the environment?
For context, this was a major part of the blueprint caching exploration work, eg see:
https://github.com/square/Blueprint/pull/398/files#diff-d78ef8c9d425f712373581c1f800fb283208265b37b8a7a30145452b3780cbe2
and
https://github.com/square/Blueprint/pull/398/files#diff-e664d2d763ebe3ca35c985ddc39c338eff03d90a5dd5f6d90091b7612d02ce4a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I expect that if a view's body accesses any part of
viewEnvironment
, that body will be unnecessarily reevaluated for any change to any other part of theViewEnvironment
. It's also cumbersome that the View has to write out e.g.@Environment(\.viewEnvironment.marketStylesheet)
.I think the next step is to explore mapping ViewEnvironment keys to individual SwiftUI Environment keys, perhaps using UITraitBridgedEnvironmentKey as @n8chur has suggested.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rob and I did some experimentation on the
@Environment
property wrapper and found that anEnvironmentKey
observing view will not invalidate if the subset of the value your property wrapper points to (viaKeyPath
) is unchanged (using what behaves like the same magic comparison that SwiftUI uses for determining changes in other systems).For example, if you have an Environment value with two parameters in an Environment key:
and you have a view that observes
\.myThing.bar
:contained within another view that sets
\.myValue.foo
:Changes to
\.myValue.foo
do not cause the body of views observing the\.myValue.bar
KeyPath (MyObservingView
) to be re-evaluated.Full code for playground
This behavior makes me feel much better about the idea of using the shape proposed here for just composing the environment, but I'm sure there are other benefits to going with the bridged key approach still. This is, for example, still very easy to naively access in a way that would be non performant:
If the bridging key approach doesn't work out, perhaps an incremental improvement over what we have now would be to just not expose the
viewEnvironment
key directly and instead require access via another property wrapper which wraps it (e.g.@ViewEnvironment
)?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E.g. we could hide the
viewEnvironmnt
EnvironmentValues
access behind a property wrapper / view modifier set like this: