jessesquires / Foil

A lightweight property wrapper for UserDefaults done right
https://jessesquires.github.io/Foil/
MIT License
458 stars 26 forks source link

[Feature]: Be able to save `Codable` structs (or a workaround?) #72

Closed jgale closed 10 months ago

jgale commented 1 year ago

Guidelines

Description

Hello! Big Foil fan here. :)

I'd like to be able to save a Codable struct in UserDefaults using Foil. I know this is somewhat controversial, and "highly discouraged" but I'm aware of the tradeoffs and I'm OK with it. I'm open to any possible way of doing that, even if it's not a feature that's added to Foil itself.

Problem

Be able to save and retrieve small Codable structs from UserDefaults.

The problem I'm running into is that I want the decoding to fail gracefully if the struct can't be decoded, but I can't figure out how to do that.

I have a MyCodableStruct the conforms to Codable. I have exposed it in my AppSettings like so:

@WrappedDefaultOptional(key: "myCodableStructKey")
public var savedCodableStruct: MyCodableStruct? {
    willSet {
        objectWillChange.send()
    }
}

I've tried to have my type conform to UserDefaultsSerializeable via the following approach:

extension MyCodableStruct: UserDefaultsSerializable {
    public var storedValue: String? {
        guard
            let data = try? JSONEncoder().encode(self),
            let string = String(data: data, encoding: .utf8)
        else {
            return nil
        }
        return string
    }

    public init(storedValue: String?) {
        guard
            let storedValue,
            let data = storedValue.data(using: .utf8),
            let decoded = try? JSONDecoder().decode(Self.self, from: data)
        else {
            fatalError("Didn't expect something we couldn't decode!")
        }
        self = decoded
    }
}

This actually works, and I can save/restore the struct. However, I want it to be able to fail gracefully if there is some garbage stored in that string. Currently it hits the fatal error above. I would like it to just return nil if the decoding fails, but that's not an option in this initializer.

Proposed Solution

I'm a little out of my depth with property wrappers and generics, so I'm not sure how to do this.

What causes it to crash is the last line of fetchOptional<T>(_ key: String) -> T? where it does return T(storedValue: storedValue).

func fetchOptional<T: UserDefaultsSerializable>(_ key: String) -> T? {
    let fetched: Any?

    if T.self == URL.self {
        // Hack for URL, which is special
        // See: http://dscoder.com/defaults.html
        // Error: Could not cast value of type '_NSInlineData' to 'NSURL'
        fetched = self.url(forKey: key)
    } else {
        fetched = self.object(forKey: key)
    }

    if fetched == nil {
        return nil
    }

    guard let storedValue = fetched as? T.StoredValue else {
        return nil
    }

    return T(storedValue: storedValue)
}

If the protocol instead had a failable initializer like init?(storedValue:) it would work fine. It could return nil, which I would be happy with. Maybe if there was an optional init?(storedValue: StoredValue) in the protocol? Or a UserDefaultsSerializableOptional?

Alternatives Considered

I'm open to any other ideas as well. Thanks for your help and your great work!

jessesquires commented 1 year ago

Hi @jgale! πŸ‘‹πŸΌ Thanks for this!

I would happily accept a PR for this.

Maybe if there was an optional init?(storedValue: StoredValue) in the protocol?

I think this is possible, but I would have to check if the compiler complains about anything. This would be a good solution for a 4.x release, as it would not require any breaking changes.

If the protocol instead had a failable initializer like init?(storedValue:) it would work fine.

This works too, and I think is a slightly better solution, but a breaking change and requires a 5.0 release.

My only concern is that I don't want this to impact usage of the non-optional @WrappedDefault. Users shouldn't have to worry about optionals in this scenario.


Regarding planning / releasing β€” assuming both of the above are possible, I would prefer to do a non-breaking 4.x release first and then follow-up with a new 5.0 release with the better API.


Another thought (maybe for 5.0) is just providing direct support for Codable types, similar to all the existing "built-in" types. (Honestly, I should just give in and do this because I know folks want it. πŸ˜… So we might as well implement it correctly and unit test it so that users don't have to do this.)

jgale commented 1 year ago

Thanks for the reply @jessesquires! I played around with making these changes in Foil today. I couldn't get the optional init?(storedValue: StoredValue) to work because optional only works for Objective-C protocols, which we don't want. I couldn't come up with any other way to make it optional.

Then I tried converting the signature to init?(storedValue: StoredValue). This works very well for my goals, and has surprising (to me) effects. Most of the extensions that implement UserDefaultsSerializable still compile and work without changes! e.g. Bool/Int/URL/Data/others. There must be something I don't understand about Swift initializers where init(storedValue: StoredValue) matches the protocol requirement for init?(storedValue: StoredValue). Some of the ones do need to change, e.g. Array/Set/Dictionary/RawRepresentable. The changes are very simple, usually moving from map to compactMap.

So this would break some client code implements UserDefaultsSerializable, though I don't fully understand under what circumstances. Presumably that implies it should be a 5.0 change like you said.

These changes have an added benefit, IMO. For example, let's say you want to store an optional enum TestEnum: String using Foil. If there is an invalid rawValue string stored in UserDefaults, when you attempt to read it, you'd now get back nil instead of it blowing up in the force-unwrap in the UserDefaultsSerializable extension that implements RawRepresentable support.

I am totally fine with waiting for 5.0 since I can't come up with any other way to make it optional. Would you like me to send a PR with the changes?

Also, I'd be happy to have full-on Codable support in 5.0. 😁

jessesquires commented 1 year ago

I couldn't get the optional init?(storedValue: StoredValue) to work because optional only works for Objective-C protocols, which we don't want. I couldn't come up with any other way to make it optional.

Ahh yes. I knew there was something here, but I couldn't think of it the other day.

There must be something I don't understand about Swift initializers where init(storedValue: StoredValue) matches the protocol requirement for init?(storedValue: StoredValue).

Hm... that's interesting. I'll dig around and see what I can find out.

The changes are very simple, usually moving from map to compactMap.

πŸ‘πŸΌ

Presumably that implies it should be a 5.0 change like you said.

Yes, we'll just do this to be safe and folks and migrate over when they want.

These changes have an added benefit, IMO. For example, let's say you want to store an optional enum TestEnum: String using Foil. If there is an invalid rawValue string stored in UserDefaults, when you attempt to read it, you'd now get back nil instead of it blowing up in the force-unwrap in the UserDefaultsSerializable extension that implements RawRepresentable support.

Nice. 😎

Would you like me to send a PR with the changes?

Please do!

Also, I'd be happy to have full-on Codable support in 5.0. 😁

That sounds great! Let's do this one in a separate PR. πŸ˜„

jessesquires commented 10 months ago

cc @jgale https://github.com/jessesquires/Foil/pull/92

jgale commented 9 months ago

This is awesome, thanks so much @jessesquires. Sorry I failed to send the PR!

jessesquires commented 9 months ago

@jgale no worries! no apology necessary!