Static check for liveValue?

[mamouneyya]

We recently migrated from a very old version of TCA to 1.x. One thing that has been scaring me about the new dependencies system is the fact that you can build (and potentially distribute) an app without having a live implementation of a dependency, since the requirement of subscripting DependencyValues is conforming to TestDependencyKey and not to DependencyKey. This, in a way, makes the newer version less safe in this aspect, which is a bit surprising. Especially with our huge project, it's pretty hard to make such a guarantee. Our codebase is huge, and we support many different platforms, so the dependencies are kinda scattered everywhere.

Having said that, I cannot even describe how convenient the ergonomics of Dependencies are, compared to manually passing down the whole Environment, which can get very messy when having nested domains. So, I get it. I also get the problem that sometimes you cannot (or even don't want to) implement the live version in the same module. Having that as a hard requirement will basically prevent you from building, so again, I completely understand the reasons behind the decision.

I've been thinking about ways to make it safer, and one idea that came to my mind is to introduce this restriction only for RELEASE. One quick way to implement that is to add an #else if RELEASE here and use #error("/*whatever*/") to stop the build. My goal is to make it impossible to ship an app without implementing liveValue, but also keep the flexibility of being able to build during development.

One argument is that we cannot break building for RELEASE either for the same reasons above, in case you chose to implement liveValue outside. I don't claim to have a good answer to that, but here are some ideas:

1. Is there a way to identify that we're building "an app" (as opposed to just a package) at build time? If so, would it be reasonable to fail the build when we compile (A) an app & (B) in RELEASE mode? I think it is.
2. How about making this configurable somehow? You can opt-out if it doesn't work for you, but unless you do some work to do so, we'll try to assume that this shouldn't happen, to protect you from unexpected behaviors. Could it be a compiler flag? I am not sure, just throwing ideas. Could be configurable per a dependency or just globally.

Curious what others think about this?

[stephencelis]

One thing that has been scaring me about the new dependencies system is the fact that you can build (and potentially distribute) an app without having a live implementation of a dependency, since the requirement of subscripting DependencyValues is conforming to TestDependencyKey and not to DependencyKey.

One thing to note is that while we advertise the new dependencies system, nothing is stopping folks from manually passing dependencies to reducers. That system still works just fine, but instead of a generic Environment you can hold onto dependencies directly on the reducer conformance as a let.

One quick way to implement that is to add an #else if RELEASE here and use #error("/*whatever*/") to stop the build.

Are you sure this works? Doesn't #error unconditionally fail the build when it's in a compiling path? Do you have a proof-of-concept?

As for your other questions, we're not sure there's a way to address it since dependencies and modular and extensible from the outside. Maybe there's a way to dynamically crash on app launch if a dependency exists and there is no liveValue, but we're just not sure how to do that, or if it's even possible. Definitely open to suggestions and pull requests to improve the situation, though!

[mbrandonw]

Hi @mamouneyya, can you describe more how you think your proposed change would actually work out in practice? From what I can tell it forces you to provide a live value at the moment of registering your dependency in RELEASE mode, which means you will not be able to separate the live implementation from its interface. That kind of defeats one of the main purposes of the library.

[mamouneyya]

Sorry for the delay. So, what I'm looking for is for the library to enforce the strict check when 2 conditions are met:

1. I'm building an app (as opposed to a Swift package, where it's okay to not have a liveValue)
2. I'm building for RELEASE

Having said that, as per my knowledge and given how SwiftPM works, I don't think this is really feasible. Maybe this thread would inspire someone who's smarter than me to come up with something? But, until then, I'm inclined to a simple compiler flag that we (the library consumers) can opt-in to. I can figure out the logic to only pass it when the conditions are met in my case. What do you think?

[stephencelis]

@mamouneyya As far as I know, it's not possible for SPM packages to use compiler flags in this way. Really it sounds like the best solution to your problem is via the build phase @jflan-dd shared: #123 (comment)

Have you given it a shot and seen if it meets your needs? I just don't think Swift provides the hooks you're looking for otherwise.

[mamouneyya]

As far as I know, it's not possible for SPM packages to use compiler flags in this way.

I was thinking maybe we can use the environment variables to guard that logic. So, in swift-depdnencies' Package.swift, we can have something like this:

var settings: [SwiftSetting] = []

if ProcessInfo.processInfo.environment["SWIFT_DEPENDENCIES_STRICT_ENVIRONMENT"] {
    settings.append(.define("SWIFT_DEPENDENCIES_STRICT_ENVIRONMENT"))
}

But anyway, I haven't tried @jflan-dd's script yet. I'm planning to. If you have specific concerns with this approach, then I'm okay with playing with that script. I just think that this can be a very valuable feature for consumers who are concerned about safety given their specific setup, etc.

[stephencelis]

@mamouneyya I'm really not sure environment variables can be used in this way in a package.swift file, but if you can share an example of it working I'd love to see it!

[jflan-dd]

I wrote a script to scan a binary to check if every TestDependencyKey conformance has a matching DependencyKey conformance. We're running it during every build and it takes ~6-10 seconds on our app. I think there are a couple weird edge cases it can miss, but it has caught several issues for us.

Script:

#!/bin/zsh

# This script checks that every conformance of TestDependencyKey has a matching DependencyKey
# conformance for a given binary. It is intended for use in a Xcode post-build script.

if [ $# -ne 1 ]; then
  echo 'Binary file named required to check for conformances'
  exit 1
fi

conformance_symbols=$(symbols "$1" | grep -E "protocol witness for static (Test){0,1}DependencyKey")

test_conformances=$(echo "$conformance_symbols" | grep "protocol witness for static TestDependencyKey.testValue.getter" | awk '{ print $11 }')
live_conformances=$(echo "$conformance_symbols" | grep "protocol witness for static DependencyKey.liveValue.getter" | awk '{ print $11 }')

typeset -A conformance_names

# Add live confomances to a set
for conformance in $(echo "$live_conformances"); do
  conformance_names[$conformance]=true
done

# Flag if any conformances are missing
has_missing=false

# Check the every test conformance is also in the live conformance set
for conformance in $(echo "$test_conformances"); do
  if [[ ! -v conformance_names[$conformance] ]]; then
    echo "error: $conformance does not have a liveValue in $(basename $1). Ensure a DependencyKey conformance is defined and the module it's defined in is linked into the target"
    has_missing=true
  fi
done

if [ $has_missing = true ]; then
  exit 1
fi

Xcode build phase:

./check-dependency-conformances.sh "${BUILT_PRODUCTS_DIR}/${EXECUTABLE_PATH}"

[jflan-dd]

We apply that script to every target in our project. That resulted in finding some keys that are defined in common frameworks, but not used in app extension targets like notification service extensions.

In those targets I added an UnusedDependency protocol to specifically mark them as intentionally unused:

extension DecodingErrorTrackerKey: UnusedDependencyKey {}

extension NetworkResponseTrackerKey: UnusedDependencyKey {
    static var shouldAssert: Bool { false }
}

// MARK: - UnusedDependencyKey

protocol UnusedDependencyKey: DependencyKey {
    /// Whether an assertion failure should be trigger if the unused dependency is accessed. Defaults to `true`.
    static var shouldAssert: Bool { get }
}

extension UnusedDependencyKey {
    static var shouldAssert: Bool { true }

    public static var liveValue: Value {
        assert(!shouldAssert, "Unused dependency \(Value.self) unexpectedly accessed.")
        return testValue
    }
}

[mamouneyya]

That's neat. Thanks for sharing!

[blevass3-ford]

To my understanding, the purpose of the swift-dependencies library is to provide a guided way to insert dependencies into a service locator like pattern using a single point of entry system.

With this being noted, there's trade offs to approaching a service locator in a way that deters runtime dependency swap outs versus static dependency definitions.

It's been noted on a few occasions that it's encouraged to fail default dependency implementations in tests to ensure they're explicit in use-age. This is a tradeoff point for the library where we're clearly requiring an up-front definition (typically a protocol witness like structure) with unimplemented functions to ensure we're failing at call sites for dependency interaction, but not at the call site for fetching the dependency itself.

Service locators encourage runtime dependency manipulation and swapping, hence leaning into liveValue implementations is slightly contradictory when we already have failed testValue dependencies that need runtime overrides when appropriate. If liveValues were required up front, we would just end up duplicating unimplemented interfaces and run into the same potentially unsafe routine as we cannot validate the difference between placeholder logic that requires runtime dependencies and interactions vs a missing implementation for a liveValue. This is ok and how most service locators function.

Sacrificing some level of runtime awareness and allowing teams to implement their own checks/rules is a good approach for something that isn't necessarily an issue trying to be solved.

For our team, TestDependencyKey has granted us the flexibility we wanted and needed for certain approaches in this area. Sure, these issues can be solved with scaffolds and offset to the DI'ed item rather the DI interface - this has historically never been an attractive solution for Service Locator libraries though.

[mamouneyya]

I really don't want to derail this into a philosophical discussion on whether swift-dependencies is the best implementation of a pattern X or Y. The way I think about it is very simple, following the purpose of this package, is it safer to statically guarantee that a liveValue exists before you distribute your app? For me, I think the answer is definitely yes.

I didn't really get your point about runtime swapping because that is not how this library is designed to work. And the tests are kinda irrelevant because they don't use liveValue anyway, right?

[blevass3-ford]

Having

enum MyKey: DependencyKey {
    static var liveValue: MyThing = UnimplementedMyThing()
}

or

enum MyKey: TestDependencyKey {
    static var testValue: MyThing = UnimplementedMyThing()
}

Has no logical protection or difference. There will always be dependencies that will require runtime dependencies. The discussion doesn't enforce any form of protection on the implementors, it's a debate on code overhead to adopt very real world project needs adding more friction with little to no gain.

My point is not as much philosophical as it is pointing out this library is a service locator and these types of conversations for service locators are very old. Service locators are typically runtime bias inversion points that have to balance both runtime and static safety needs.

[mamouneyya]

adding more friction with little to no gain

What friction? How would implementing a liveValue be a friction? Isn't that something that 99.9% of swift-dependencies consumers need to do anyway in order to ship their apps?

I understand that checking whether "an implementation" exists isn't necessarily a full protection, but it's definitely some protection.

[mbrandonw]

I think what @blevass3-ford is trying to describe here is that every dependency management library is going to have pros, cons and tradeoffs. In order to support modular dependencies, where interface can be separated from implementation, as well as some other features, there must be a little bit of uncertainty in dependencies being provided. This is similar to SwiftUI's @EnvironmentObject in which if you do not provide the object it crashes (at least we do not crash and instead runtime warn).

There is tooling that can be built to help the situation (as @jflan-dd has shown), but if you want to be 100% certain that it is impossible to run an app without a dependency being provided, you can always go back to the old style of dependency passing where the reducer explicitly holds each dependency.