IOS-10545 Migrating to Swift 6.0 with strict concurrency #405

alejandroruizponce · 2024-10-25T12:02:57Z

🎟️ Jira ticket

https://jira.tid.es/browse/IOS-10545

🥅 What's the goal?

In a previous PR we made changes to make Mistica work in XCode 16 without using Swift 6. Now in this PR Swift 6 is enabled and many new concurrency issues arise. You can check more information here:
https://www.swift.org/migration/documentation/swift-6-concurrency-migration-guide/incrementaladoption

🚧 How do we do it?

Package has been updated to support Swift 6.0 and the SnapshotTesting library has been configured to the latest version to support strict concurrency testing.
Two elements closely related to all our Mistica logic such as UIView and View are now executed in @MainActor which implies that all components related to these two classes must also be isolated in the main thread.
Classes like FontStyle or MisticaConfig that are quite important in Mistica's logic have been refactored as @unchecked Sendable and concurrency will be handled manually through queues. This way we do not isolate these classes which would mean that any place where they are used would also have to be isolated and that would be a very big refactor.
All tests have been refactored with the changes in the latest version of SnapshotTesting, the most important change is:

Before in old version:

    override func setUp() {
        super.setUp()
        isRecording = false
    }

Now in latest version:

    override func invokeTest() {
        withSnapshotTesting(record: .never) {
            super.invokeTest()
        }
    }

*This change implies changing our screenshots action since now the isRecording variable does not exist and must be configured in the new function. In fact this function now allows several values: .all, .failed, .missing, .never.

The screenshots record has been set to run with .failed in order to re-record only those screenshots that fail to be regenerated.

🧪 How can I verify this?

A version of Catalog has been generated to verify that everything works as before.

alejandroruizponce · 2024-10-25T12:05:38Z

Sources/Mistica/Components/Feedback/FeedbackView.swift

-            .forEach(buttonsView.addArrangedSubview(_:))
+        Task { @MainActor in
+            try [primaryButton, secondaryButton].compactMap { $0 }
+                .forEach(buttonsView.addArrangedSubview(_:))


Now addArrangedSubview is isolated:
@MainActor open class UIStackView : UIView { .. }

alejandroruizponce · 2024-10-25T12:06:57Z

Sources/Mistica/Components/Feedback/FeedbackView.swift

@@ -365,7 +367,7 @@ private extension FeedbackView {
        }

        // Prepare
-        views.forEach(prepare(view:))
+        try? views.forEach(prepare(view:))


Now forEach method has a rethrow to be handled:
@inlinable public func forEach(_ body: (Element) throws -> Void) rethrows

alejandroruizponce · 2024-10-25T12:07:47Z

Sources/Mistica/Components/Feedback/FeedbackView.swift

+            Task { @MainActor in
+                self.feedbackGenerator?.notificationOccurred(hapticFeedbackStyle)
+                self.feedbackGenerator = nil
+            }


Now UINotificationFeedbackGenerator is isolated:
@MainActor open class UINotificationFeedbackGenerator : UIFeedbackGenerator

alejandroruizponce · 2024-10-25T12:18:29Z

Sources/MisticaCommon/Extensions/Lottie+Utils.swift

@@ -15,7 +15,7 @@ import Lottie
 **/

 public extension Lottie.LottieConfiguration {
-    static var current: LottieConfiguration = {
+    nonisolated(unsafe) static var current: LottieConfiguration = {


Lottie is not updated yet with strict concurrency.

salavert · 2024-10-28T10:30:14Z

.github/workflows/record-screenshots.yml

-          find . -type f -name "*.swift" -exec sed -i '' 's/isRecording = false/isRecording = true/' {} +
+          find . -type f -name "*.swift" -exec sed -i '' 's/record: .never/record: .failed/' {} +


What we do at Latch is 1 test plan with 3 configurations so we can specify them on xcodebuild -testPlan LatchSnapshots -only-test-configuration "Record all"

https://github.com/Telefonica/path2-ios/blob/master/LatchSnapshotTests/Helpers/LatchSnapshotTest.swift

if you define this env variable, if only CI executes this and you don't want a test plan to do it locally, you should be able to provide it in the makefile and set it before executing the xcodebuild call

export RECORD_SCREENSHOTS=all xcodebuild -project .... test

In teory yes, tried it but didn't work, maybe I'm doing something wrong

I think it would be a good idea to investigate this in a separate ticket.

DavidMarinCalleja · 2024-11-04T06:30:03Z

...omponents/Sheet/View/BottomSheetPresentation/BottomSheetInteractiveDismissalTransition.swift

@@ -11,7 +11,7 @@ import UIKit
 final class BottomSheetInteractiveDismissalTransition: NSObject {
    private enum Constants {
        static let maxBouncingHeight: CGFloat = 250
-        static let animationDuration: CGFloat = UIView.defaultAnimationDuration
+        static let animationDuration: CGFloat = 0.25


Why this change?

Missing fix, now it's fixed!

...omponents/Sheet/View/BottomSheetPresentation/BottomSheetInteractiveDismissalTransition.swift

DavidMarinCalleja · 2024-11-04T10:21:57Z

Sources/MisticaSwiftUI/Utils/Extensions/EnvironmentValues.swift

@@ -21,7 +21,7 @@ public extension EnvironmentValues {
 @available(iOSApplicationExtension, unavailable)
 private struct SafeAreaInsetsKey: EnvironmentKey {
    static var defaultValue: EdgeInsets {
-        UIApplication.shared.keyWindow?.safeAreaInsets.swiftUiInsets ?? EdgeInsets()
+        EdgeInsets()


Is this change right? I'm not sure we always want to return a new instance of EdgeInsets().

At first I did not see how to fix it, but now I do through preconcurrency.

DavidMarinCalleja

pbartolome · 2024-11-06T14:09:41Z

Sources/MisticaCommon/MisticaConfig.swift

+        set {
+            concurrentQueue.async { _currentColors = newValue }
+        }
+    }


seems that this pattern is being replicated in multiple places, as @salavert said, it would be nice to have a property wrapper for this kind of var

github-actions · 2024-11-12T09:45:30Z

Screenshot tests report

✔️ All passing

alejandroruizponce commented Oct 25, 2024

View reviewed changes

alejandroruizponce marked this pull request as ready for review October 25, 2024 12:24

alejandroruizponce requested review from salavert, DavidMarinCalleja, pbartolome, alexanegon, cbarcom and franciscojrp October 25, 2024 12:24

salavert reviewed Oct 28, 2024

View reviewed changes

salavert approved these changes Oct 28, 2024

View reviewed changes

franciscojrp approved these changes Oct 31, 2024

View reviewed changes

DavidMarinCalleja reviewed Nov 4, 2024

View reviewed changes

...omponents/Sheet/View/BottomSheetPresentation/BottomSheetInteractiveDismissalTransition.swift Outdated Show resolved Hide resolved

DavidMarinCalleja reviewed Nov 4, 2024

View reviewed changes

DavidMarinCalleja approved these changes Nov 4, 2024

View reviewed changes

pbartolome reviewed Nov 6, 2024

View reviewed changes

pbartolome approved these changes Nov 6, 2024

View reviewed changes

cbarcom approved these changes Nov 13, 2024

View reviewed changes

Alejandro Ruiz and others added 8 commits November 19, 2024 13:29

Check lottie previous version

6bb643a

Restore latest Lottie version

068f51f

Migrating to Swift 6

3e4c46b

Rebase with main

245990c

Fix fontstyle tests

0a1a83d

Rebase with main

911a6f7

Fix record-screenshots action

5fd8376

Run swiftformat

dd09ef2

franciscojrp and others added 9 commits November 19, 2024 13:34

Make InputField.ValidationState Sendable

9ef47f1

Isolate feedback closure and fix completions

fd40c3e

Run swiftformat

998ddbf

Isolate CustomCroutonContainerView

fc42db3

Fix styleControls isolation

1fc59c6

Rebase with main

2ff8d5e

Fix Feedback and Stepper tests

f7b4e16

Fix InputField tests

38a0e5f

Fix PR comments

cd7d14d

alejandroruizponce force-pushed the IOS-10545-Migrating-to-Swift-6 branch from 93b9001 to cd7d14d Compare November 19, 2024 12:38

Run swiftformat

7ae09b8

alexanegon approved these changes Nov 19, 2024

View reviewed changes

Make Sendable SnackBarStyle

1df65b5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IOS-10545 Migrating to Swift 6.0 with strict concurrency #405

IOS-10545 Migrating to Swift 6.0 with strict concurrency #405

alejandroruizponce commented Oct 25, 2024

alejandroruizponce Oct 25, 2024 •

edited

Loading

alejandroruizponce Oct 25, 2024

alejandroruizponce Oct 25, 2024

alejandroruizponce Oct 25, 2024

salavert Oct 28, 2024 •

edited

Loading

pbartolome Oct 30, 2024

salavert Nov 6, 2024

alejandroruizponce Nov 19, 2024

DavidMarinCalleja Nov 4, 2024

alejandroruizponce Nov 19, 2024

DavidMarinCalleja Nov 4, 2024

alejandroruizponce Nov 19, 2024

DavidMarinCalleja left a comment

pbartolome Nov 6, 2024

github-actions bot commented Nov 12, 2024 •

edited

Loading

		find . -type f -name "*.swift" -exec sed -i '' 's/isRecording = false/isRecording = true/' {} +
		find . -type f -name "*.swift" -exec sed -i '' 's/record: .never/record: .failed/' {} +

IOS-10545 Migrating to Swift 6.0 with strict concurrency #405

Are you sure you want to change the base?

IOS-10545 Migrating to Swift 6.0 with strict concurrency #405

Conversation

alejandroruizponce commented Oct 25, 2024

🎟️ Jira ticket

🥅 What's the goal?

🚧 How do we do it?

🧪 How can I verify this?

alejandroruizponce Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salavert Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidMarinCalleja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 12, 2024 • edited Loading

alejandroruizponce Oct 25, 2024 •

edited

Loading

salavert Oct 28, 2024 •

edited

Loading

github-actions bot commented Nov 12, 2024 •

edited

Loading