Skip to content
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

Add support for transformable property types #28

Merged
merged 5 commits into from
Feb 16, 2024

Conversation

admkopec
Copy link
Contributor

This PR adds support for @Attribute(.transformed(by: Transformer.self)) property as per issue #4. This only adds support for type specified Transformers inheriting directly from ValueTransformer.

Due to the need of registering the transformer and issues with NSClassFromString() the named equivalent of this attributed was not implemented as part of this PR.

Additionally when testing the implementation, I encountered an error with CodableBox transformer which could be passed a base type T as the value. This was addressed with additional check. Not sure if the reverse transformer should return value of type T or CodableBox then? Although the committed approach with base type T appears to work with my tested model.

Additionally, while adding tests for Transformable properties, I added a test case for RawRepresentable enums.

@helje5
Copy link
Contributor

helje5 commented Feb 13, 2024

Thanks! Will have a look. It seems to reverse the fix I made for Codable structs (where the valueType setter was overwritten by the options), but I only had a quick look.

@helje5
Copy link
Contributor

helje5 commented Feb 15, 2024

Hm, if we can store the Codable value as-is, we shouldn't even need the CodableBox, right? The Transformer could directly do the coding from/to Data 🤔

But I don't understand how you ended up w/ the base value in the Transformer?

@helje5
Copy link
Contributor

helje5 commented Feb 15, 2024

I played w/ that, and it looks like we don't need the CodableBox but just can stick the Swift value into the dynamic property? Not sure whether this is going to have unwanted side effects :-)
branch feature/no-codable-box-1

@helje5 helje5 self-assigned this Feb 15, 2024
@helje5 helje5 merged commit c1a963c into Data-swift:develop Feb 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants