-
-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable aliased received properties to be received again further down the chain #23
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -55,10 +55,10 @@ final class Scope { | |||||||||
.compactMap { | ||||||||||
switch $0.source { | ||||||||||
case .received: | ||||||||||
return $0.fulfillingProperty ?? $0.property | ||||||||||
$0.fulfillingProperty ?? $0.property | ||||||||||
case .forwarded, | ||||||||||
.instantiated: | ||||||||||
return nil | ||||||||||
nil | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
@@ -81,7 +81,12 @@ final class Scope { | |||||||||
instantiableStack | ||||||||||
.flatMap(\.dependencies) | ||||||||||
.filter { | ||||||||||
$0.source != .received | ||||||||||
( | ||||||||||
// If the source is not received, the property has been made available. | ||||||||||
$0.source != .received | ||||||||||
// If the dependency has a fulfilling property, the property has been aliased. | ||||||||||
|| $0.fulfillingProperty != nil | ||||||||||
Comment on lines
+87
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This matches similar code that we have over in the ScopeGenerator: SafeDI/Sources/SafeDICore/Generators/ScopeGenerator.swift Lines 55 to 58 in e1d20ca
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest the more I think about this (and the PR built on top of this one)... the more I think the abstraction here is wrong. This change works and is mergeable, however I may take a bigger pass at cleaning this up before the next PR makes it out of draft. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah alright pushed up something way better in #24. |
||||||||||
) | ||||||||||
&& !propertyStack.contains($0.property) | ||||||||||
&& $0.property != property | ||||||||||
} | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a style change. not related to the underlying fix here