Display a deprecation warning and fatal error when the top-level @Environment(\.viewEnvironment) is referenced
#319
Conversation
…ironment(\.viewEnvironment) is referenced, to enforce people reference a sub-property. This ensures proper SwiftUI view invalidation.
| Instead, reference your relevant sub-property, eg `@Environment(\\.viewEnvironment.myProperty)`. | ||
| """ | ||
| ) | ||
| @inlinable public init(_ keyPath: KeyPath<EnvironmentValues, Value>) { |
There was a problem hiding this comment.
I can't think of a case where we'd want to do this, but if we want to allow it, perhaps adding an SPI'd init(unsafe: ...) would allow that escape hatch.
|
|
||
| @available( | ||
| *, | ||
| deprecated, |
There was a problem hiding this comment.
Note that this needs to be a deprecation, not an unavailability, as if you mark the method as unavailable, the compiler simply falls back to the "regular" init.
| """ | ||
| ) | ||
| @inlinable public init(_ keyPath: KeyPath<EnvironmentValues, Value>) { | ||
| fatalError( |
There was a problem hiding this comment.
is fatal'ing unconditionally the right approach? what exactly is the downside if one does this? if it's 'just' poor performance, should we be more lenient with the production code and make this an assertion?
There was a problem hiding this comment.
I think a fatal is correct / preferred, since you should never do this in production code. If we already had consumers of SwiftUI doing this, I'd probably go w/ an assert, but given we have the opportunity to "close the door" on this permanently, I'd say let's do that
| message: | ||
| """ | ||
| Please do not create an `@Environment` property that references the top-level `viewEnvironment`: \ | ||
| it will break SwiftUI's automatic invalidation when any part of the `ViewEnvironment` changes. \ |
There was a problem hiding this comment.
for my own personal edification: can you elaborate on the risk & consequences a bit more? what is the behavioral difference in these 2 approaches?
There was a problem hiding this comment.
So basically, the way that SwiftUI does state invalidation is to compare the properties on a view between the old view and the new view, and if they're different or otherwise not comparable, the view is redrawn and updated.
Because ViewEnvironment is a big opaque bag o' crap, its possible that there's non-comparable stuff in there, or there's stuff in there that changes every render cycle, meaning that it'd lead to view invalidation. So the fix for that is to scope your Environment property down to just the data you need.
I know there's a point free or objc.io article about this in more depth somewhere, however I am struggling to find it.
There was a problem hiding this comment.
thanks! this makes sense – i had failed to realize that the root .viewEnvironment key is like putting a copy of another environment into the... SwiftUI.Environment, and clearly you don't want to depend on the entire environment itself, just the stuff your view 'cares about'.
For UI-5972
Display a deprecation warning and fatal error when the top-level
@Environment(\.viewEnvironment)is referenced, to enforce people reference a sub-property. This ensures proper SwiftUI view invalidation.Checklist