Skip to content
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

Add targeted dependency cache reset, useful for non single-entry point systems #318

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cmriboldi
Copy link

This is a first pass at a feature I'd like to suggest we add to the library. I'm happy to update any documentation/additional tests if needed as well.

In general this method allows users to reset a single dependency when they call the new DependencyValues.reset() method. This method will take either the keypath of the DependencyValue or the Object.self.

For example:
DependencyValues.reset(\.client) or DependencyValues.reset(Client.self) would reset the live implementation that has been cached in the cachedValues.cache.

The reason for this change is for those non single-entry point systems where it is prohibitive to pass dependencies down manually with the withDependencies function.

This method is one that I'm not suggesting lightly because I understand the benefit of the TaskLocal dependency lifetimes. Adding this method does break one of the principles of swift-dependencies which is that dependencies should change predictably and only within specific scope operation. However, I still believe there is a massive benefit for those systems that can't use TCA everywhere and are stuck on non single-entry point systems yet they don't want to pass dependencies down with the withDependencies function everywhere.

The specific use case that this is helpful for are those dependencies where we need to clear the entire state/reset the entire computed dependency. I understand that one way to do this is to move all state into a small piece of internal state for each dependency but that's not always possible, or practical everywhere in my current app without major refactors. The other thing that this helps with is the clearing out of sensitive data such as a user's information when a user logs out and another logs in.

Here is a link to the conversation in the point-free slack where we originally discussed the idea behind this feature.

@cmriboldi cmriboldi marked this pull request as draft December 17, 2024 22:34
@cmriboldi
Copy link
Author

cmriboldi commented Dec 18, 2024

There is very light documentation at the moment. I'm happy to add any necessary documentation to these methods and/or to the docs that generate online.

@cmriboldi cmriboldi marked this pull request as ready for review December 18, 2024 18:31
@stephencelis
Copy link
Member

@cmriboldi Thanks for exploring this. We need to think about the implications of it, and may have some questions for you soon. Since it's the holiday season we may not get a chance to look into it till the new year, though!

@cmriboldi
Copy link
Author

cmriboldi commented Jan 14, 2025

Yup no problem, that's totally understandable.

I get that there are some implications that may not want to be taken on. I would understand if this kind of change may come with a warning to the caller so that they understand it changes the way the library works and to avoid using the call if they are in single-entry point systems and already have clean ways to override/change the dependency.

I will probably have to make this change in a fork of the library that I use (at work), but I wanted to share it with you in case you were willing to consider adding it to the main library. But I'm definitely aware of some of the implications and wouldn't be butt hurt either way you decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants