Fix: Easy use of Swift classes in Objective-C projects #4585
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4585 +/- ##
=============================================
+ Coverage 90.973% 91.004% +0.030%
=============================================
Files 616 617 +1
Lines 70734 70888 +154
Branches 25144 25300 +156
=============================================
+ Hits 64349 64511 +162
+ Misses 6293 6285 -8
Partials 92 92 see 33 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 8e4bc70 | 1229.18 ms | 1240.86 ms | 11.67 ms |
| fff4a70 | 1224.57 ms | 1239.06 ms | 14.49 ms |
| cf97209 | 1234.06 ms | 1251.49 ms | 17.43 ms |
| deeb22c | 1233.90 ms | 1250.19 ms | 16.29 ms |
| 62f06fc | 1213.91 ms | 1238.08 ms | 24.17 ms |
| 9fa5d27 | 1219.86 ms | 1221.71 ms | 1.85 ms |
| 8c50edb | 1212.98 ms | 1233.72 ms | 20.74 ms |
| 4aea556 | 1222.94 ms | 1248.02 ms | 25.08 ms |
| ea73af6 | 1230.96 ms | 1244.98 ms | 14.02 ms |
| c1ca4cb | 1228.71 ms | 1246.23 ms | 17.53 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 8e4bc70 | 21.58 KiB | 625.90 KiB | 604.32 KiB |
| fff4a70 | 21.58 KiB | 707.28 KiB | 685.70 KiB |
| cf97209 | 21.58 KiB | 632.16 KiB | 610.58 KiB |
| deeb22c | 21.58 KiB | 612.11 KiB | 590.53 KiB |
| 62f06fc | 21.58 KiB | 671.30 KiB | 649.72 KiB |
| 9fa5d27 | 20.76 KiB | 393.37 KiB | 372.61 KiB |
| 8c50edb | 20.76 KiB | 432.31 KiB | 411.55 KiB |
| 4aea556 | 22.85 KiB | 411.66 KiB | 388.81 KiB |
| ea73af6 | 20.76 KiB | 425.75 KiB | 404.99 KiB |
| c1ca4cb | 22.30 KiB | 747.52 KiB | 725.22 KiB |
| @@ -1,6 +1,6 @@ | |||
| #import "AppDelegate.h" | |||
| @import CoreData; | |||
| @import Sentry; | |||
| #import <Sentry/Sentry.h> | |||
There was a problem hiding this comment.
This is how we test if the SDK is working with objc.
Before this PR compiling iOS-ObjectiveC with this change would cause an error.
Sources/Sentry/Public/Sentry.h
Outdated
| @@ -42,11 +42,13 @@ FOUNDATION_EXPORT const unsigned char SentryVersionString[]; | |||
| # import <Sentry/SentrySpanProtocol.h> | |||
| # import <Sentry/SentrySpanStatus.h> | |||
| # import <Sentry/SentryStacktrace.h> | |||
| # import <Sentry/SentrySwift.h> | |||
There was a problem hiding this comment.
h: Doesn't this make all Swift classes public?
I can now access NSArray<NSString *> * features = [SentryEnabledFeaturesBuilder getEnabledFeaturesWithOptions:options]; in the AppDelegate.m from the iOS-Objective-C sample app.
There was a problem hiding this comment.
Yes, it does for Objc project.
If you think this is a problem we close the PR, I tried other ways to solve this without success, its this or nothing (until we find something else).
I dont think users will start to use things they dont know how to use. It's already hard to make them use the public APIs that have documentation.
Also, this will be a big problem for some users with special projects, like this one: #4543
There was a problem hiding this comment.
I dont think users will start to use things they dont know how to use.
Murphy's law: "Anything that can go wrong will go wrong." So someone will use these classes. They always do.
If I'm not mistaken, there is no difference between an actual public class in ObjC and an accidentally public class as SentryEnabledFeaturesBuilder. So, how can I tell the difference between the two in ObjC code? I didn't look closely into this, but maybe a stupid question: Can't we split up the classes into two headers? We can also discuss this on a call, if this turns out the be a longer discussion.
There was a problem hiding this comment.
Also, didn't we want to move the SR code into its own module / SPM / Cocoa package? Maybe this would help here.
There was a problem hiding this comment.
Also, didn't we want to move the SR code into its own module / SPM / Cocoa package? Maybe this would help here.
Yes, this would be a solution.
So, how can I tell the difference between the two in ObjC code?
You can't, both are public.
Can't we split up the classes into two headers?
No, its an auto generated header by Xcode
There was a problem hiding this comment.
I would appreciate it if we didn't make all the Swift classes public to fix this issue, especially if we plan on converting more code to Swift. So even though moving the SR code to its extra package seems like a lot of work to fix this, it's the right thing to do, IMO. It would also reduce the size of the sentry-cocoa lib.
|
We dont want to expose all the Swift classes to objc. We will have to find another solution. |
|
@brustolin, agreed thanks for the comment. |
📜 Description
In order to access Swift classes in an Objective-C project, developers need to either use
@import Sentry;which is not always possible because the project needs to be compatible with-fmodulesand-fcxx-modules, or they need to import both#import <Sentry/Sentry.h>and#import <Sentry/Sentry-Swift.h>, which is not intuitive and they are always coming to git hub to understand what's going on.💡 Motivation and Context
Closes #4543
💚 How did you test it?
Samples and CI
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.🔮 Next steps