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

Resolve many concurrency warnings #349

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

Conversation

DePasqualeOrg
Copy link

This is a first attempt to resolve many concurrency warnings in preparation for strict concurrency with Swift 6.0. It depends on the changes I made in NetworkImage here: gonzalezreal/NetworkImage#33

I'm still unsure how to resolve the warnings related to the four instances of static var defaultValue, structs conforming to ImageProvider, and cmark in MarkdownParser.swift.

Copy link
Contributor

@MojtabaHs MojtabaHs left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I agree with the overall goal of making this package fully compatible with Swift 6. However, it would be helpful to break the changes into separate, meaningful commits and provide an example with each commit explaining how it addresses a specific issue. This will help streamline the merging process and make it easier to review.

Thanks again!

bundle: Bundle? = nil
) {
self.name = name
self.bundle = bundle
}

public func makeImage(url: URL?) -> some View {
if let url = url, let image = self.image(url: url) {
if let url, let image = self.image(url: url) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unrelated to the concurrency.

@@ -102,7 +102,7 @@ public struct FontProperties: Hashable {
set { self.widthStorage = newValue }
}

private var widthStorage: AnyHashable?
nonisolated(unsafe) private var widthStorage: AnyHashable? // Marking this as `nonisolated(unsafe)`, because it should really be `Font.Width?`, but this is only available from iOS 16. Change to type `Font.Width?` and remove `nonisolated(unsafe)` after raising minimum target to iOS 16
Copy link
Contributor

Choose a reason for hiding this comment

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

We may be able to define an adapter for this instead of raising the minimum platform version.

@DePasqualeOrg
Copy link
Author

@MojtabaHs, @gonzalezreal has already said that he's working on his own solution for Swift 6 compatibility. However, I'm not sure how you would break this into separate commits. This PR is meant as a starting point, and perhaps it won't even be used in the end. You can do with it what you like.

@DePasqualeOrg DePasqualeOrg marked this pull request as draft October 15, 2024 09:23
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