-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix WordPressData runtime issues #24494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ae36b86
f09fef4
b2e2666
56f5e02
2eacc63
e2b433b
393cf58
43032c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import Foundation | ||
|
||
extension Bundle { | ||
@objc public class var wordPressData: Bundle { | ||
Bundle(for: BundleToken.self) | ||
} | ||
} | ||
|
||
private final class BundleToken {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,8 @@ import WordPressKit | |
/// Furthermore, sites eligible for unlimited sharing will still return a `PublicizeInfo` along with its sharing | ||
/// limitations, but the numbers should be ignored (at least for now). | ||
/// | ||
@objc public class PublicizeInfo: NSManagedObject { | ||
@objc(PublicizeInfo) | ||
public class PublicizeInfo: NSManagedObject { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The following annotation implicitly adds a module name to the Objective-C class name.
This breaks most of our code that works with entities (it uses The fix is to manually set the global name:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL. |
||
|
||
public var sharingLimit: SharingLimit { | ||
SharingLimit(remaining: Int(sharesRemaining), limit: Int(shareLimit)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swift-only types like
ManagedAccountSettings
are not present in the global namespace. You have to specify the module..
means "current module".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that mean the migration from version 154 (which has an incorrect module) to 155 is broken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make this change (adding
.
) to all modal files?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, my mistake was not making the leap in understanding that Objective-C types are handled differently from Swift ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a unit test could clear this up? #24496
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazytonyli you are onto it! ☝️
In version 154, we crash upon trying to init
BlogAuthor
withThe solution is to apply the same
representedClassName
change in 154:See also CI from #24496
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed 2a17e73 in my PR that updates all the models. I used two bash commands, so I might have messed up something. Still, the test above now passes. Progress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first thing the app does is migrate from the production model (154). It never loads the represented entities in memory. It uses
NSManagedObject
and KVC for migration. It's impossible to retroactively change the models and there is also no need to. The app hasrepresentedClassName
values in the previous models with class name that no longer even exist in the codebase – it's what the migration is for.