Rxswift: Xcode 11.4 beta: 'NSTextView' is incompatible with 'weak' references

[tonyarnold]

Short description of the issue:

When building using the just released Xcode 11.4 beta, RxCocoa fails to build on the following line: https://github.com/ReactiveX/RxSwift/blob/c3c0cac3d4c176b04404e3574d62b51776277384/RxCocoa/macOS/NSTextView%2BRx.swift#L20

Expected outcome:

RxCocoa compiles without errors or warnings.

What actually happens:

The compile fails with the following error:

RxSwift/RxCocoa/macOS/NSTextView+Rx.swift:20:12: 'NSTextView' is incompatible with 'weak' references

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

Version 5.0.1

Platform/Environment

• [ ] iOS
• [x] macOS
• [ ] tvOS
• [ ] watchOS
• [ ] playgrounds

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

• [x] easy, 100% repro
• [ ] sometimes, 10%-100%
• [ ] hard, 2% - 10%
• [ ] extremely hard, %0 - 2%

Xcode version:

  Version 11.4 beta (11N111s)

Installation method:

• [ ] CocoaPods
• [x] Carthage
• [ ] Git submodules

I have multiple versions of Xcode installed:
(so we can know if this is a potential cause of your issue)

• [x] yes (11.3.1, 11.4 beta 1)
• [ ] no

Level of RxSwift knowledge:
(this is so we can understand your level of knowledge
and formulate the response in an appropriate manner)

• [ ] just starting
• [ ] I have a small code base
• [x] I have a significant code base

[freak4pc]

This seems like a bug to me. Do you care filing a Feedback with Apple referring to this thread? There's no reason we would strongly capture a UI component afaik

[freak4pc]

Uhm seems that according to some resources, that class doesn't support a weak reference? ;O That's quite strange.

http://zpasternack.org/some-objects-dont-support-weak-references/

Regardless, seems like a fix that needs to be made.

[tonyarnold]

I’m happy to file feedback with Apple on this, but this limitation has been around for _years_ (it just wasn’t highlighted by Xcode).

I don’t know that there is an answer here that anyone is going to be happy with.

I’ll let you know what they say :+1:

[brunophilipe]

I've been told this _used_ to be the case but it had been fixed. Maybe they accidentally removed the incompatibility flag and only reintroduced it now?

[tonyarnold]

Ok, I've narrowed this down - it's the deployment target for RxSwift:

https://github.com/ReactiveX/RxSwift/blob/c3c0cac3d4c176b04404e3574d62b51776277384/Package.swift#L46

If you raise the target to .macOS(.v10_12), this works (which makes sense, given @brunophilipe's comments - I'd imagine this is when the issue was addressed).

I'm happy to submit a PR - could we bump this up for RxSwift 5.1, or 6.0?

[freak4pc]

I’m not too excited bumping a deployment target for no reason. If bumping to 10.12 “fixes” it, it would seem it’s more clear this error is some sort of discrepancy/change in how it used to work.

Could you share the FB link here once you make one ?

I’ll look around some more, regardless.

Thanks!

[tonyarnold]

OK, I've filed this as FB7566796.

I guess let's wait to see what the AppKit/Xcode team have to say about this - my guess is that this error was meant to be turned on a while back (or is considered an improvement to diagnostics & warnings), and we may be stuck with it.

Here's the relevant code from NSTextView.h:

#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_12
NS_AUTOMATED_REFCOUNT_WEAK_UNAVAILABLE
#endif

// ... 

/**************************** Ownership policy ****************************/
// Returns whether instances of the class operate in the object ownership policy introduced with macOS Sierra and later. When YES, the new object owner policy is used. Under the policy, each text view strongly retains its text storage and its text container weakly references the view. Also, the text views are compatible with __weak storage. The default is YES.
@property (readonly, class) BOOL stronglyReferencesTextStorage API_AVAILABLE(macos(10.12));

I understand your reluctance to bump deployment targets for a bug that might be transitory, but I don't think raising the deployment target is a bad thing for a major release. Consider the impact on yourself and the other developers of this project in supporting code that needs to compile and run on an operating system you no longer have access to. I believe you can't even run CI anywhere for Yosemite?

[freak4pc]

I don't think raising the deployment target is a bad thing for a major release

I have no issue with bumping the versions, but I'm not a macOS developer myself, so bumping a version of a platform I don't have intimate knowledge with seems a bit careless from my part.

Regardless, let's wait for Apple and if we don't get appropriate feedback, we might decide to bump.

Thanks for the research and time you've put into this @tonyarnold !

[serges147]

Is it possible to raise min macOS 10.12 only for RxCocoa?

[freak4pc]

If we bump, it would be for the framework as a whole

[serges147]

This whole "bumping to 10.12" is very sad story :( For us it means we stuck on 5.1.0 RxSwift and XCode 11.3.1 until we drop 10.11 (which won't happen till 2020 fall for sure). And the only reason for this is that lousy NSTextView rx in RxCocoa which we don't use at all.

[AdamAexol]

I'm hoping for different solution than bumping os version. A lot of users still using 10.10 and 10.11.

[tonyarnold]

Apple shipped this error in Xcode 11.4 (released today). As best I can see, there's not really another option available.

A lot of users still using 10.10 and 10.11.

What apps are you shipping that support macOS releases back that far? As I mentioned earlier in the conversation, I'm unsure if you can even run CI to test RxSwift on releases back that far, so I'd be squinty-eyed at shipping a library on a platform I couldn't test for (not that this is my project, mind you!).

[freak4pc]

Another option is counting on the fact Xcode 11.4 is Swift 5.2 and through a ton of #if compiler(>=5.2) around weak-handling portions.

[tonyarnold]

You can only do that around type constructs and methods though, right? Can you conditionally include/exclude properties that way?

[freak4pc]

It's a compiler directive, I imagine I can even do another version of the entire class, possibly. Better than dropping support if I see so many people against it.

"Luckily", my job was cut to 4 days a week thanks to Coronavirus, so I have some free time tomorrow. I'll look into it :D

[freak4pc]

Yup, this works. I'm cutting a patch release:

[bukira]

so no one can build this framework using swift 5.2 and Xcode 11.4 ?

[tonyarnold]

“Luckily” — sorry to hear that, Shai 😞

[freak4pc]

This was fixed in 5.1.1.

[nighthawk]

@freak4pc Has this also been pushed to the main CocoaPods repo? https://cocoapods.org/pods/RxCocoa still says 5.1.0 which. (RxSwift is on 5.1.1 though.)

[freak4pc]

Hey, you're right! This is odd, was sure I pushed all of them. Taking care of the rest RN.

[freak4pc]

Should be good to go :)

[nighthawk]

All good now, thank you very much!

[DivineDominion]

Late to the party, but I'd suggest making the property itself unavailable for macOS 10.11 and earlier: https://github.com/ReactiveX/RxSwift/blob/712ff5348ece316f5422f8cd371cb4fef4f0f962/RxCocoa/macOS/NSTextView%2BRx.swift#L78

Because now it compiles on all platforms, but on macOS 10.11, you simply get runtime exceptions as soon as you try to form weak references to a NSTextView.

[freak4pc]

@DivineDominion Just saw your note. Unfortunately we can't hard deprecate in a minor release, this is against semver.