[feat]: add ViewEnvironmentUI module

[n8chur]

Note: This PR is targeting the feature/viewenvironmentui branch. Further integrations into WorkflowUI will land in that branch before these changes are merged into main (#211).

Overview

Adds a new ViewEnvironmentUI module which handles propagation of ViewEnvironment through UIViewController, UIView, and any arbitrary object node.

This PR is part of the larger effort to add support for automatic ViewEnvironment bridging between UIKit propagation and WorkflowUI propagation within WorkflowUI.

See the associated proposal for more information.

Changes

The changes in this PR are most easily reviewable by commit:

Port ViewEnvironmentUI from Market and add to Development.podspec (6eacf4a)

This is essentially a copy paste of the framework which currently exists in MarketUI, but with a few typos in documentation fixed and added license header docs.

Simplify protocol hierarchy (7b7c59f)

Removes the ViewEnvironmentCustomizing protocol and merges the functions it defined into ViewEnvironmentObserving.

Adds default implementations of all functions on ViewEnvironmentObserving so you can pick and chose how you'd like to observe without requiring an empty implementation for functions you may not be concerned with.

Removes the ability to implement var viewEnvironment: ViewEnvironment yourself. This was useful when bridging between propagation contexts and/or when a node wanted to act as a root without pulling the environment through all ancestors above it. Instead, setting the nodes environmentAncestor to nil will prevent the pull from above and will prevent other nodes who have this node set as an ancestor from triggering setNeedsEnvironmentUpdate on the node.

Rename viewEnvironment to environment (593250d)

This rename will cause ambiguity when using ScreenViewController if ViewEnvironmentUI is imported since that superclass already has an environment property. This will be resolved in a future PR targeting the feature branch.

Add environmentDidChange observation option (b37dad6)

Adds a hook that observers can utilize to respond aggressively to environment changes rather than needing to wait until the setNeedsEnvironmentUpdate/apply(environment:) dance has completed (which is usually deferred since it's backed by setNeedsLayout/viewWillLayoutSubviews).

Checklist

• Unit Tests
• UI Tests
• Snapshot Tests (iOS only)
• I have made corresponding changes to the documentation

[kyleve]

Do you need this if let, given the chain upwards of the protocol inheritance?

[n8chur]

Yeah, this protocol does not inherit from ViewEnvironmentObserving.

[n8chur]

Nor does ViewEnvironmentPropagatingObject.

[kyleve]

Thought: Is it worth making a special type / subclass, so the memory debugger, etc, shows the type more obviously in case something goes wrong?

[n8chur]

We can!

[kyleve]

Trying to think through if we can get into any weird cases by making this a retain, vs a weak.

[n8chur]

The type here is a closure so it'd be really easy to mess that up I think...

The fact that this property is retained is part of the trick I used in the prototype to add support for inserting a propagation node above the view controller being described in a ViewControllerDescription—this closure is the only thing holding a reference to that fake node which provides the bridged ViewEnvironment.

[kyleve]

Couple questions; but approving since a lot of this has already existed in Market for the most part, and has been reviewed.

[watt]

Mostly tweaks to docs.

A couple thoughts on potential follow-ups:

• we should mob on diagrams for this some time
• might be nice to have a convenience function for inserting a customization node between existing ancestors & descendants

[watt]

Do we still have a use case for this?

[n8chur]

I'm still using it in the prototype inside of TrampolineModalListObserver.

I think we may actually want to be using this method of observation instead of overriding environmentDescendantsOverride when the logic making this observation is not owned by the view controller itself.

For example, I'm currently playing with having WorkflowHostingController set its environmentDescendantsOverride to an empty array to prevent vanilla propagation invalidations causing a re-render of it's owned rendering since we'll already be performing a re-render on environmentDidChange. In our TrampolineModalPresenter, we need some way to observe invalidations triggered by its owner, but that owner could be a WorkflowHostingController. Rather than trying to override environmentDescendantOverrides and having to compose the existing override (similar to calling super), it might be cleaner to consider environmentDescendantsOverride something that only the VC it's applied to should override, and if you need to hook into invalidations, you use the addEnvironmentNeedsUpdateObserver(...).

The composing approach does seem to work, but seems like it could get hard to follow when debugging and potentially difficult to document, but I'd be open to utilizing that approach for cases like this if we'd rather consolidate the API a bit.

[n8chur]

A couple thoughts on potential follow-ups:

• we should mob on diagrams for this some time
• might be nice to have a convenience function for inserting a customization node between existing ancestors & descendants

Definitely down to pair on both of these at some point in the near future!

[square-tomb]

Thanks for the careful organization of this PR. That and the context in the linked proposal made it a pleasure to review.

My comments are all nits and doc fixes. I wonder if there is some way we can avoid the overloading of the term "ancestor" in our terminology.

[square-tomb]

nit, orthogonal to this change: Do the customizations in self.customize(environment:) affect the value passed to self.apply(environment:), or only to decendants' apply(environment:)?

[n8chur]

Customizations currently apply to the environment on the node itself. I'll clarify this in documentation.

• (8d4478f) Add note about customizations applying to the node itself

Andrew mentioned that this was surprising during his review and I have a note to follow up on this. Which shape is most intuitive to you?

[square-tomb]

Good question. I did guess correctly that customizations do apply to self, but I was on the fence about it.

Applying customizations to descendants only would be sensible in that the node can easily opt into applying the same customizations to itself within self.apply(environment:). But if nodes almost always want that, then maybe it's annoying to have to start apply(environment:) with

var environment = environment
customize(environment: environment)

[square-tomb]

ancestors will not call notify

As a newbie to view environment propagation, the ancestor/descendant terminology is confusing to me because:

• "Ancestor" describes something that is analogous to a "parent" in other systems. In those other systems, "ancestor" refers to any node that can be reached by calling parent recursively
• Ditto for "descendant"/"child"
• The documentation sometimes refers to A as ancestor of B if A is in B's environmentDescendants, like in this sentence. In other places, the doc uses "ancestor" to mean environmentAncestor, which is now a distinct concept.

Am I misunderstanding anything there?

[n8chur]

Ah, yeah, we should probably be more careful with the phrasing here.

As you stated, "ancestor" is any parent (recursively), and "descendant" is children (recursively). A node's ancestor may not be that ancestor nodes' descendant and vice versa. I think this doc comment should be something like:

If the value of the ancestor is nil, by default, other nodes configured with this node as a descendant will not notify this node of needing an environment update as it changes. This allows a node to effectively act as a root node when needed (e.g. bridging from other propagation systems like WorkflowUI).

Does that make sense?

• (6802cb5 ) Improve documentation around the difference between ancestor and a node configured with the node in question as a descendant

[watt]

just minor suggestions!