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

Async dependency injection support #16

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

DanielCech
Copy link
Member

@DanielCech DanielCech commented Feb 17, 2025

Goals ⚽

  • This PR contains a lot of changes in dealloc tests implementation because of async dependency injection and migration to Swift 6
  • The core was rewritten to support Swift concurrency
  • If you run tests in sample apps, tests are intentionally failing (to test the feature as described in README) and can be easily fixed.
  • I removed the workspace because it was obsolete and not maintained. The new convention is to open the Swift package as a directory.

⚠️ We need to make a deal about how to integrate this version because the old version should be still available - it is a dependency for other projects. This version contains breaking changes. ⚠️

@DanielCech DanielCech marked this pull request as ready for review February 17, 2025 12:17
@DanielCech DanielCech changed the title Async dependency injection support [WIP] Async dependency injection support Feb 20, 2025
Copy link
Contributor

@ipek ipek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I have just some final minor comments.

@@ -26,8 +26,7 @@ let package = Package(
],
dependencies: [
// Dependencies declare other packages that this package depends on.
.package(url: "https://github.com/strvcom/ios-dependency-injection.git", from: "1.0.2")
],
.package(url: "[email protected]:strvcom/ios-dependency-injection.git", from: "1.0.4") ],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.package(url: "[email protected]:strvcom/ios-dependency-injection.git", from: "1.0.4") ],
.package(url: "[email protected]:strvcom/ios-dependency-injection.git", from: "1.0.4")
],

guard let self = self else {
return
}
try? await Task.sleep(for: .seconds(delayTime))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the delayTime? Is it possible to reduce to milliseconds at least in some cases?

Comment on lines +176 to +178
// #if canImport(DependencyInjection)
// await self?.container.releaseSharedInstances()
// #endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove or uncomment.

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