Skip to content

Fixes PropertyWrapper #267

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.

Sign up for GitHub

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

Open
spydercapriani wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fixes PropertyWrapper #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7eb2141
Fixes PropertyWrapper to support custoom adapter
Dec 2, 2020
c406e6a
Fixes PropertyWrapper to support custoom adapter (#1)
Visput Dec 2, 2020
d05a851
git ignore .swiftpm
spydercapriani Dec 5, 2020
bc7c59f
Save DefaultValue on first get if no value exists
spydercapriani Dec 5, 2020
d498f39
Merge branch 'master' into feature/FixDefaultsBehavior
spydercapriani Dec 5, 2020
0f37d9d
Merge pull request #2 from UnionPOS/feature/FixDefaultsBehavior
spydercapriani Dec 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ xcuserdata/
/www

/Carthage
/.swiftpm
2 changes: 2 additions & 0 deletions Sources/Defaults+Subscripts.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public extension UserDefaults {
// swiftlint:disable:next force_cast
return _value as! T
} else if let defaultValue = key.defaultValue {
T._defaults.save(key: key._key, value: defaultValue, userDefaults: self)
Copy link
Owner

@sunshinejr sunshinejr Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove the default behavior change from this PR and create another one for it? the reason is that this is a change that breaks compatibility with the current implementation, and so we would need a major version for that vs we don't have for the property wrapper fixes

Copy link
Author

@spydercapriani spydercapriani Dec 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this was suppose to be an internal change specific to our use case and not something for this PR. Didn't realize this PR was opened against our Master branch, will fix.

return defaultValue
} else {
return T.T.empty
Expand All @@ -106,6 +107,7 @@ public extension UserDefaults {
if let value = T._defaults.get(key: key._key, userDefaults: self) {
return value
} else if let defaultValue = key.defaultValue {
T._defaults.save(key: key._key, value: defaultValue, userDefaults: self)
return defaultValue
} else {
fatalError("Shouldn't happen, please report!")
Expand Down
23 changes: 7 additions & 16 deletions Sources/PropertyWrappers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,32 @@ public struct SwiftyUserDefaultOptions: OptionSet {
}

@propertyWrapper
public final class SwiftyUserDefault<T: DefaultsSerializable> where T.T == T {
public final class SwiftyUserDefault<T: DefaultsSerializable, KeyStore: DefaultsKeyStore> where T.T == T {

public let key: DefaultsKey<T>
public let options: SwiftyUserDefaultOptions
private var adapter: DefaultsAdapter<KeyStore>

public var wrappedValue: T {
get {
if options.contains(.cached) {
return _value ?? Defaults[key: key]
return _value ?? adapter[key: key]
} else {
return Defaults[key: key]
return adapter[key: key]
}
}
set {
_value = newValue
Defaults[key: key] = newValue
adapter[key: key] = newValue
}
}

private var _value: T.T?
private var observation: DefaultsDisposable?

public init<KeyStore>(keyPath: KeyPath<KeyStore, DefaultsKey<T>>, adapter: DefaultsAdapter<KeyStore>, options: SwiftyUserDefaultOptions = []) {
public init(keyPath: KeyPath<KeyStore, DefaultsKey<T>>, adapter: DefaultsAdapter<KeyStore>, options: SwiftyUserDefaultOptions = []) {
Copy link
Owner

@sunshinejr sunshinejr Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so ideally I'd love to have the Defaults be a default adapter - can we do that?

Copy link
Author

@spydercapriani spydercapriani Dec 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was thinking we would be able to utilize a global framework property as a default initializer but that doesn't appear to be the case. I might suggest that we set default initializer with the same adapter init used for Default for this public init. This implies that unless a different adapter is provided, it will always look for key/values in the standard UserDefaults using the DefaultsKeyStore. Which may be practical for most basic use cases.

Copy link
Owner

@sunshinejr sunshinejr Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spydercapriani yeah that would work - thanks!

self.key = adapter.keyStore[keyPath: keyPath]
self.adapter = adapter
self.options = options

if options.contains(.observed) {
Expand All @@ -69,17 +71,6 @@ public final class SwiftyUserDefault<T: DefaultsSerializable> where T.T == T {
}
}

public init(keyPath: KeyPath<DefaultsKeys, DefaultsKey<T>>, options: SwiftyUserDefaultOptions = []) {
self.key = Defaults.keyStore[keyPath: keyPath]
self.options = options

if options.contains(.observed) {
observation = Defaults.observe(key) { [weak self] update in
self?._value = update.newValue
}
}
}

deinit {
observation?.dispose()
}
Expand Down