Skip to content

Added @IgnoreHashable property wrapper#1604

Merged
NachoSoto merged 1 commit into
mainfrom
ignore-hashable
May 24, 2022
Merged

Added @IgnoreHashable property wrapper#1604
NachoSoto merged 1 commit into
mainfrom
ignore-hashable

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

This will be used for the new implementation of CustomerInfoResponse, and also for var rawData: [String: Any] properties that can't conform to Hashable, and that we wouldn't want to use for comparison since they contain duplicated data.

This will be used for the new implementation of `CustomerInfoResponse`, and also for `var rawData: [String: Any]` properties that can't conform to `Hashable`, and that we wouldn't want to use for comparison since they contain duplicated data.
@NachoSoto NachoSoto requested a review from a team May 24, 2022 17:53

@aboedo aboedo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cool idea!

@NachoSoto NachoSoto merged commit c5d70a5 into main May 24, 2022
@NachoSoto NachoSoto deleted the ignore-hashable branch May 24, 2022 21:14
NachoSoto added a commit that referenced this pull request May 24, 2022
…1565)

Follow up to #1496. Depends on ~~#1567,  #1568, #1604~~.

The solution implemented in #1496 was very simple, but [as pointed out by @joshdholtz](#1496 (comment)), it was only encoding the data that was formally decoded by the `struct`s.
We had no test coverage for this, so it went unnoticed.

The new solution adds a new field:
```swift
@IgnoreEncodable @IgnoreHashable
var rawData: [String: Any]
```

Which, together with a new helper method `decodeRawData()`, allows storing all the data to be used by `CustomerInfo`'s `RawDataContainer` implementation.

Unfortunately, despite many attempts, I realized that this can't be abstracted into a property wrapper, because a `KeyedDecodingContainer` can't access the parent `Decoder`.
That's why the `CustomerInfoResponse` and `CustomerInfoResponse.Entitlement` must manually call this method.
@joshdholtz joshdholtz mentioned this pull request May 26, 2022
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