NavigationBackport icon indicating copy to clipboard operation
NavigationBackport copied to clipboard

Use `@StateObject` backport to avoid crash on iOS 13

Open maguerrieri opened this issue 3 years ago • 4 comments

I've used another package, shaps80/SwiftUIBackports, to enable this; how do you feel about introducing other dependencies?

maguerrieri avatar Jul 26 '22 22:07 maguerrieri

I don't think it's a good idea to import a whole package just for this. Maybe the package can be used as a guide/template for a custom @StateObject instead? https://github.com/shaps80/SwiftUIBackports/blob/main/Sources/SwiftUIBackports/Shared/StateObject/StateObject.swift

theedov avatar Jul 31 '22 01:07 theedov

@maguerrieri Thanks very much for opening this PR - I hadn't realised the library was crashing on iOS 13!

I agree with @theedov that adding a dependency on SwiftUIBackports feels a bit heavy, and I also wonder if a backported StateObject could be added manually without the dependency. Perhaps it could default to SwiftUI.StateObject in iOS 14 and above? I also wonder if it would be enough to add this in an ios13 branch that supports iOS 13, and make the main library support >= iOS 14. My assumption is that very few people are building SwiftUI apps that support iOS 13, but maybe I'm wrong?

johnpatrickmorgan avatar Jul 31 '22 07:07 johnpatrickmorgan

Thanks for the feedback!

For the package dependency, what would y'all think of reducing it to just the StateObject backport? Like, fork the package and remove everything else?

I do also like the idea of using real StateObject on 14+, but trying briefly I didn't come up with a neat way to switch implementations for a property wrapper. I can fiddle with it some more this week; do y'all have any suggestions there?

maguerrieri avatar Jul 31 '22 16:07 maguerrieri

Thanks @maguerrieri! I'll try to experiment with lightweight ways of achieving this too.

johnpatrickmorgan avatar Jul 31 '22 23:07 johnpatrickmorgan

I think the best approach for this would be to maintain an ios13 compatibility branch with the proposed change and bump the main library's minimum deployment to iOS/tvOS 14.0.

johnpatrickmorgan avatar Sep 01 '22 22:09 johnpatrickmorgan

That seems like a reasonable solution, though how will it interact with release versioning? Does SPM handle the semver metadata field okay? e.g. 0.3.1+ios13, or something?

maguerrieri avatar Sep 01 '22 22:09 maguerrieri

I've set up an iOS 13 branch with your changes - thanks a lot @maguerrieri ! I've also bumped the minimum deployment targets for iOS and tvOS to 14.

how will it interact with release versioning? Does SPM handle the semver metadata field okay? e.g. 0.3.1+ios13, or something?

As far as I can tell, SPM allows you to specify metadata in a dependency version, but I'm assuming it wouldn't resolve newer versions automatically (e.g. I don't think specifying from: "0.3.1+ios13" would resolve to a version 0.3.2+ios13 if available).

johnpatrickmorgan avatar Sep 01 '22 23:09 johnpatrickmorgan

Great, thanks! 🙂

We exactly specify all of our versions, so not getting automatic updates suits me fine. Any other ideas on how to handle versioning?

maguerrieri avatar Sep 02 '22 02:09 maguerrieri

We exactly specify all of our versions, so not getting automatic updates suits me fine. Any other ideas on how to handle versioning?

Thanks for clarifying @maguerrieri. In that case, I think specifying either the ios13 branch or a specific commit is probably the best approach. Reading the semver spec, it seems like build metadata is intended for other purposes.

johnpatrickmorgan avatar Sep 20 '22 22:09 johnpatrickmorgan