Add equivalency protocols and implementations (1/3) #568
Conversation
maxg-square
changed the title
|
|
||
| private var values: [ObjectIdentifier: Any] = [:] | ||
| private var values: [Keybox: Any] = [:] | ||
|
|
||
| private var internalValues: [ObjectIdentifier: Any] = [:] | ||
|
|
There was a problem hiding this comment.
The spec doc does a nice job outlining that internal key/values are "hidden from consumers and not considered in equivalency checks." Do you think we could carry that into docs here, to outline the differences between these two dictionaries?
* main: Release 6.3.0 Assert in debug mode when large content viewer is not placed inside an interaction container Expose largeContentViewerInteraction on LargeContentViewer backing view via protocol Stopped installing xcodes in GitHub Actions file chore: Release 6.2.0 Support accessibility large content viewer
| case elementSizing | ||
| } | ||
|
|
||
| public protocol ContextuallyEquivalent { |
There was a problem hiding this comment.
A few minor things:
-
you've got this in
Internalbut it's public -
strong preference from me to have 1:1 files to types if possible rather than
Equivalency.swift. Make a folder namedEquivalencyif the grouping is important. -
naming-wise
ContextuallyEquivalentis pretty generic for something that's still very Blueprint specific. I'll float something likeElementContextComparableas an alternative. -
find a new name for
ContextuallyEquivalent
There was a problem hiding this comment.
It's slightly more broad than element-specific (includes env values too) and I think comparable has a connotation of being sortable given the stl usage of it – maybe something like CacheEquivalent? Or could just go the other way and be less concretely tied to that specific method it requires and be something more tied to purpose like CrossLayoutCacheable?
There was a problem hiding this comment.
Some ideas that max and I had:
MultipassLayoutEquivalent, LayoutContextuallyEquivalent.
We also considered flipping polarity of function, so it can't be easily confused with Equatable or Comparable. In that world, instead of asking if the two objects are functionally equivalent, you would ask if one object invalidates another compared to the previous version.
func invalidatesLayout(from previous: Self?, in context: LayoutCacheContext) -> Bool
For that, we had protocol name ideas of LayoutDependency and LayoutInvalidatingDependency.
There was a problem hiding this comment.
For me, Equivalent is out for the same reason as Comparable (connotations of more generic concepts).
I like CrossLayoutCacheable, or anything similar with the Cacheable suffix, since that really dials in on the distinct purpose here.
"Dependency" and "invalidates" both kind of muddy the waters for me. In the status quo, everything invalidates layout.
| /// - rhs: The right hand side value being compared. | ||
| /// - context: The context to evaluate the equivalency. | ||
| /// - Returns: Whether or not the two values are equivalent in the specified context. | ||
| static func isEquivalent(lhs: Value, rhs: Value, in context: EquivalencyContext) -> Bool |
There was a problem hiding this comment.
If this is required I think there should be a default implementation returning false.
- default impl on isEquivalent
There was a problem hiding this comment.
Discused offline earlier but feel pretty strongly we shouldn't have that – there's default implementations when values are equatable and also convenience implementations like this that are one-liners https://github.com/square/Blueprint/pull/568/files#diff-e664d2d763ebe3ca35c985ddc39c338eff03d90a5dd5f6d90091b7612d02ce4aR55 but IMO new environmentkeys should be forced to consider this.
1/3 for caching changes.
Big-picture PR here: #587
Design doc: https://docs.google.com/document/d/1CLhFbzZGbEKgvZTUwLuGbFY-rfZNJVTi6f3PkoZT9qI/edit?usp=sharing
This PR adds a
ContextualEquivalencyprotocols and conformances. This is basically like a lightweightEquatable, which adds the ability to interrogate objects with the notion of a context.