[feat]: add imperative environment customization support

[n8chur]

Overview

Adds support for declaring environment customizations to a ViewEnvironmentPropagatingNode without subclassing, custom wiring, or VC containment.

Kudos to @watt for the idea!

Checklist

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

[jamieQ]

is this still true?

[n8chur]

Ah, yeah I should add some nuance to this comment. The closure provided for onRemove is only called in deinit if it was not called manually.

• (7242f1f) Improve doc comment regarding remove() being called in deinit

[jamieQ]

this seems like an assert warning about unexpected behavior may be more appropriate. this is idempotent, right (other than the precondition)? will something break horribly if it's called more than once?

[n8chur]

Nothing would break if this was called more than once with the current implementation.

I'm fine with moving this to an assert with early return, but I'll check with @watt who suggested this change!

• (5b7a48d) Use assert instead of preconditionFailure?

[watt]

Asserting is fine with me. I think the idea here is that we consider it a programmer error to try to remove a customization more than once, even if it is effectively idempotent.

[n8chur]

Oh whoops, I added this to the wrong lifetime 🤦

• (39d493d) Apply removal assertion changes to customization lifetime

[jamieQ]

this all looks reasonable to me!

[jamieQ]

just to better my own understanding – should this trigger the 'setNeedsEnvironmentUpdate' (or whatever its name is) logic, or does that responsibility lie at a different level?

[n8chur]

Yeah—I'm fairly torn on whether or not this should automatically call setNeedsEnvironmentUpdate() on your behalf. You'll almost always want to call it after adding a customization, but I figured there may be cases where you'd prefer to defer that call or manually manage when the invalidation occurs (e.g. for performance reasons). setNeedsEnvironmentUpdate() does trigger environmentDidChange() calls which are not deferred to the next layout cycle, and we use this to perform updates in some cases within WorkflowUI and Market. For example, WorkflowHostingController will re-render its content whenever environmentDidChange() is called.

The current shape also makes it a bit easier to ensure events happen at more appropriate times. For example, if you were to be adding a customization to a new child VC in a container VC, you might have something like this:

addChild(childVC)
customizationLifetime = childVC
    .addEnvironmentCustomization(makeCustomization(for: column))
if let view = viewIfLoaded {
    view.add(childVC.view)
}
childVC.didMove(toParent: self)
childVC.setNeedsEnvironmentUpdate()

This order of operations means:

• The child VC is added as a child and the customization is added before the child VC's view is (conditionally) loaded. This ensures any environment access during viewDidLoad would have the appropriate environment wiring path and customizations.
• setNeedsEnvironmentUpdate() is called after the parent/child relationship is fully established (didMove(toParent:) has been called). I'm not aware of any real issues that could come from calling this before didMove(toParent:), but it seems like the most appropriate time in the VC lifecycle IMO.

I'm very open to automatically calling setNeedsEnvironmentUpdate() automatically when a customization is added if we prefer the safety here!

[jamieQ]

more of a general question about the changeset – is this alternate API mainly for convenience? curious what the motivating examples are (if they exist).

[n8chur]

It's mainly for convenience. The motivating example is support for what we call "pane-based size class", where a split container VC is able to override the horizontal size class for a particular pane based on the available horizontal width of that pane.

Without this change we'd either need to insert a node between the split container VC and it's children, or introduce a wrapper VC around those children to perform the environment mutation.