square / workflow-swift

A Swift and Kotlin library for making composable state machines, and UIs driven by those state machines.
https://square.github.io/workflow
Apache License 2.0
322 stars 44 forks source link

[chore]: move ViewEnvironment's EnvironmentValues key to the ViewEnvironment module #266

Closed n8chur closed 7 months ago

n8chur commented 7 months ago

Why?

Moving this environment value key to ViewEnvironment means that modules that aren't dependent on WorkflowUI (like MarketSwiftUI) are able to access ViewEnvironment keys on the SwiftUI EnvironmentValues. Market will use this key to access the MarketContext on the ViewEnvironment which is bridged by either our Market SwiftUI Hosting Controller or the standard WorkflowUI SwiftUI Screen depending on context rather than having to manually bridge the MarketContext in addition to the ViewEnvironment, and have the potential for the MarketContexts in both environments to get out of sync.

The integration in Market can be found here: https://github.com/squareup/market/pull/7763

Checklist

UI-5335

watt commented 7 months ago

Would ViewEnvironmentUI be a better home? We'll need to pull that in anyway in order to bridge from VC land to SwiftUI, right?

n8chur commented 7 months ago

Would ViewEnvironmentUI be a better home? We'll need to pull that in anyway in order to bridge from VC land to SwiftUI, right?

I don't think so, but I might be missing something.

ViewEnvironmentUI is the framework that handles node-based propagation and integrates specifically with UIKit. I believe there is going to be a WorkflowSwiftUI module that would handle SwiftUI bridging of the ViewEnvironment—I don't think any bridging to/from SwiftUI would live in ViewEnvironmentUI, but we could talk about what the right place for that would be. I do think it'd be nice avoid importing all of the UIKit-specific code in MarketSwiftUI (at least in its current form where it does not rely on UIKit).

watt commented 7 months ago

Would ViewEnvironmentUI be a better home? We'll need to pull that in anyway in order to bridge from VC land to SwiftUI, right?

I don't think so, but I might be missing something.

ViewEnvironmentUI is the framework that handles node-based propagation and integrates specifically with UIKit. I believe there is going to be a WorkflowSwiftUI module that would handle SwiftUI bridging of the ViewEnvironment—I don't think any bridging to/from SwiftUI would live in ViewEnvironmentUI, but we could talk about what the right place for that would be. I do think it'd be nice avoid importing all of the UIKit-specific code in MarketSwiftUI (at least in its current form where it does not rely on UIKit).

I talked to Westin out of band about this. The integration PR doesn't implement bridging so it's a little abstract to talk about. Realistically, there will be a need for bridging the ViewEnvironment from view controllers to SwiftUI without Workflow, and that has got to be in, or depend on, ViewEnvironmentUI. Will there ever be a pure SwiftUI + Workflow world that doesn't involve Screens or UIKit, and uses ViewEnvironment? I doubt it.

I like the idea of keeping the ViewEnvironment module as a simple dictionary type without adding any UI concerns, and letting ViewEnvironmentUI handle those (meaning both UIKit and SwiftUI fall under the "UI" moniker). But I don't feel strongly enough to block this for now.