Skip to content

Conversation

@radianttap
Copy link

Sprinkled Sendable and @MainActor as needed.
Converted few vars into let.
Set strict-concurrency-checking to Complete.
Moved minimum deployment to iOS 15, tvOS 15, macOS 10.15 (concurrency aware versions).
Updated version info to 5.0 — I recommend to tag as such.

Turned-off BITCODE
Moved minimum deployment to iOS 15, tvOS 15, macOS 10.15
Set strict concurrency checking to Complete.
This is entirely UI-bound thus it's all in main thread.
}

public protocol Constrainable {
@MainActor public protocol Constrainable {
Copy link

@SomeRandomiOSDev SomeRandomiOSDev Jul 25, 2024

Choose a reason for hiding this comment

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

Strongly recommend the following instead:

Suggested change
@MainActor public protocol Constrainable {
@preconcurrency @MainActor
public protocol Constrainable {

This makes it so that existing code that doesn't yet have strict concurrency enabled can use this library version without having to make changes to their code.

}

extension LayoutGuide: Constrainable {
@MainActor extension LayoutGuide: Constrainable {

Choose a reason for hiding this comment

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

Not needed as the compiler infers @MainActor isolation for all members declared on this extension since Constrainable is isolated to the @MainActor.

Suggested change
@MainActor extension LayoutGuide: Constrainable {
extension LayoutGuide: Constrainable {

#endif

extension TinyView: Constrainable {
@MainActor extension TinyView: Constrainable {

Choose a reason for hiding this comment

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

Not needed as the compiler infers @MainActor isolation for all members declared on this extension since Constrainable is isolated to the @MainActor.

Suggested change
@MainActor extension TinyView: Constrainable {
extension LayoutGuide: Constrainable {

}

public extension Collection where Iterator.Element == Constraint {
@MainActor public extension Collection where Iterator.Element == Constraint {

Choose a reason for hiding this comment

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

Strongly recommend the following instead:

Suggested change
@MainActor public extension Collection where Iterator.Element == Constraint {
@preconcurrency @MainActor
public extension Collection where Iterator.Element == Constraint {

This makes it so that existing code that doesn't yet have strict concurrency enabled can use this library version without having to make changes to their code.


#if os(OSX)
public extension Constraint {
@MainActor public extension Constraint {

Choose a reason for hiding this comment

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

Not needed since Constraint is already isolated to the @MainActor

Suggested change
@MainActor public extension Constraint {
public extension Constraint {

}
#else
public extension Constraint {
@MainActor public extension Constraint {

Choose a reason for hiding this comment

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

Not needed since Constraint is already isolated to the @MainActor

Suggested change
@MainActor public extension Constraint {
public extension Constraint {

#endif

public extension TinyView {
@MainActor public extension TinyView {
Copy link

@SomeRandomiOSDev SomeRandomiOSDev Jul 25, 2024

Choose a reason for hiding this comment

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

Not needed since TinyView is already isolated to the @MainActor

Suggested change
@MainActor public extension TinyView {
public extension TinyView {

import AppKit

public extension TinyView {
@MainActor public extension TinyView {
Copy link

@SomeRandomiOSDev SomeRandomiOSDev Jul 25, 2024

Choose a reason for hiding this comment

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

Not needed since TinyView is already isolated to the @MainActor

Suggested change
@MainActor public extension TinyView {
public extension TinyView {

import UIKit

public extension TinyView {
@MainActor public extension TinyView {
Copy link

@SomeRandomiOSDev SomeRandomiOSDev Jul 25, 2024

Choose a reason for hiding this comment

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

Not needed since TinyView is already isolated to the @MainActor

Suggested change
@MainActor public extension TinyView {
public extension TinyView {

}

public extension TinyView {
@MainActor public extension TinyView {
Copy link

@SomeRandomiOSDev SomeRandomiOSDev Jul 25, 2024

Choose a reason for hiding this comment

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

Not needed since TinyView is already isolated to the @MainActor

Suggested change
@MainActor public extension TinyView {
public extension TinyView {

#endif

public extension Constrainable {
@MainActor public extension Constrainable {

Choose a reason for hiding this comment

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

Not needed since Constrainable is already isolated to the @MainActor

Suggested change
@MainActor public extension Constrainable {
public extension Constrainable {

}

public extension TinyView {
@MainActor public extension TinyView {

Choose a reason for hiding this comment

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

Not needed since TinyView is already isolated to the @MainActor

Suggested change
@MainActor public extension TinyView {
public extension TinyView {

#endif

extension TinyEdgeInsets {
@MainActor extension TinyEdgeInsets {

Choose a reason for hiding this comment

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

It's probably not recommended to add this since TinyEdgeInsets itself isn't isolated to the @MainActor and there's no code in any of these members that requires @MainActor isolation

@kimdv
Copy link

kimdv commented Sep 13, 2024

Any news on this or should someone else take over? 😁

@SomeRandomiOSDev
Copy link

I made all of the changes locally and was about to put up a PR myself before noticing this one. I don’t have an issue pushing my changes and opening a PR myself 🤷🏻‍♂️

@radianttap
Copy link
Author

I made all of the changes locally and was about to put up a PR myself before noticing this one. I don’t have an issue pushing my changes and opening a PR myself 🤷🏻‍♂️

Feel free and let me know, I'll close this. I have no time at the moment to work this out further.

@SomeRandomiOSDev
Copy link

Opened a PR here: #112

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.

3 participants