Enable injecting optional properties that are only fulfilled when available #168
Conversation
136f45e to
09cf720
Compare
09cf720 to
0e81531
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
========================================
Coverage 99.92% 99.92%
========================================
Files 32 32
Lines 3757 3942 +185
========================================
+ Hits 3754 3939 +185
Misses 3 3
🚀 New features to boost your workflow:
|
9da75a3 to
52e815e
Compare
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| set -e | |||
|
|
|||
| VERSION='1.1.0' | |||
| VERSION='1.2.2' | |||
There was a problem hiding this comment.
this version is always behind
| @attached(peer) | ||
| public macro Received() = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro") | ||
| public macro Received(onlyIfAvailable: Bool = false) = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro") |
There was a problem hiding this comment.
| public macro Received(onlyIfAvailable: Bool = false) = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro") | |
| public macro Received(availability: ReceiveMode = .always) = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro") |
public enum ReceiveMode {
/// The parent always provides dependency.
case always
/// Dependency is provided only when available.
case ifAvailable
}Just a suggestion, but I think using an enum here could make the dependency availability clearer and more extensible compared to a boolean. Let me know what you think!
There was a problem hiding this comment.
Another idea that I have in mind, if SafeDI can infer if the value is optional, then it's not mandatory to receive this value? Then there's no need to pass an additional value and keep the same Swift fundamental syntax.
There was a problem hiding this comment.
Love the idea of the future proofed enum. Gonna chew on that. Makes it easier to avoid breaking changes which is great. But I'm struggling to think of ways I'd expand this.
Re inferring options, there are two reasons I didn't do this:
- If someone spells out Optional I don’t know if they’re referring to Swift.Optional or their own type, and I can’t force them to clarify because they haven’t told me that’s what they’re doing – it’s a bit too easy to screw up. You might legitimately be forwarding in an optional and want the safety
- The parameter makes it very clear when you’re opting out of the safety provided by SafeDI
There was a problem hiding this comment.
What do you think of creating a separate macro: @Received and @ReceivedIfAvailable? I'm generally a fan of creating two separate functions, macros, etc instead of creating a single function with a boolean property.
There was a problem hiding this comment.
I've been pretty steadfast in trying to reduce the number of macros that folk need to learn.
By limiting the number of entry points to SafeDI, we:
- make the basics of SafeDI easier to learn
- make it easy to discover more advanced functionality, since all advanced functionality is available off of the four macros
There are four core concepts in SafeDI, and each maps 1:1 to a macro. I plan on riding that wave for as long as we can.
| return "let \(property.label) = \(property.typeDescription.asSource)(\(fulfillingProperty.label))" | ||
| case let .alias(property, fulfillingProperty, erasedToConcreteExistential, onlyIfAvailable): | ||
| return if onlyIfAvailable, unavailableProperties.contains(fulfillingProperty) { | ||
| "// Did not create `\(property.asSource)` because `\(fulfillingProperty.asSource)` is unavailable." |
There was a problem hiding this comment.
With this, it's the developer's responsibility to ensure this dependency exists when needed.
| @attached(peer) | ||
| public macro Received() = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro") | ||
| public macro Received(onlyIfAvailable: Bool = false) = #externalMacro(module: "SafeDIMacros", type: "InjectableMacro") |
There was a problem hiding this comment.
What do you think of creating a separate macro: @Received and @ReceivedIfAvailable? I'm generally a fan of creating two separate functions, macros, etc instead of creating a single function with a boolean property.
Correct! Though I do want to underline that this is an opt-in feature, so by default you still have compile-time safety guaranteed. Just giving you more adventures to choose from |
It can be really useful to share an
@Instantiableacross boundaries where only some of their properties exist