SwiftTwitch
SwiftTwitch copied to clipboard
Remove Marshal dependency
Marshal is a nice utility, but in the end the built-in Decodable
protocol offers the same functionality. The performance gain Marshal is said to offer is marginal for this project, IMO, given that it mostly contains structs with Int
and String
values in them. Instead, it adds the drag of having to maintain a dependency (plus it has not been updated in almost a year...) and a lot of boilerplate code.
So in this PR (which is still a draft becase it needs some work and it relies on #5) I will:
- [X] Make all previously
Unmarshaling
type conform toCodable
- [X] Remove
init(object: MarshaledObject)
initialisers and conformance toUnmarshaling
- [X] Remove conformances to Marshal's
ValueType
- [X] Update places where deserialisation takes place to use
JSONDecoder
- [X] Update places where types are converted into Dictionaries to use
JSONEncoder
instead of usingJSONSerialization
- [ ] Create new structs to use in place of dictionaries when setting query/body parameters of
URLRequest
- [X] Analytics
- [X]
GetExtensionAnalyticsParams
- [X]
GetGameAnalyticsParams
- [X]
- [X] Bits
- [X]
GetBitsLeaderboardParams
- [X]
- [X] Clips
- [X]
CreateClipParams
- [X]
GetClipsParams
- [X]
- [X] Games
- [X]
GetTopGamesParams
- [X]
GetGamesParams
- [X]
- [ ] Streams
- [X]
GetStreamsParams
- [ ]
GetFollowedStreamsParams
- [X]
GetStreamsMetadataParams
- [X]
CreateStreamMarkerParams
- [X]
GetStreamMarkersParams
- [X]
- [X] Users
- [X]
GetUsersParams
- [X]
GetUsersFollowsParams
- [X]
UpdateUserParams
- [X]
- [X] Videos
- [X]
GetVideosParams
- [X]
- [X] Analytics
- [ ] Update existing Result/Data structs to fix Codable conformance problems (var names vs. web keys, nested data....)
- [ ] Analytics
- [ ] Bits
- [ ] Clips
- [ ] Games
- [ ] Streams
- [ ] Users
- [ ] Videos
- [X] Update
performAPIWebRequest
method to make use of Codable objects rather than dictionaries - [ ] Add/Update documentation comments
Input on this is appreciated. Please let me know your thoughts.
This PR breaks source compatibility
In order to support Codable and make the API more Swifty (added type safety) by using custom structs instead of dictionaries I made the following changes:
- Public API calls now receive an struct containing the parameters of the request (if appropriate) instead of the raw values (This one breaks compatibility)
- Private API method
performAPIWebRequest
now uses 3 generic parameters:- The result from the call (was already being used)
- The query parameters (new)
- The body parameters (new)
-
WebRequestKeys
are no longer. By usingJSONEncoder
andJSONDecoder
it is possible to specify thekeyEncodingStrategy
. Therefore, as long as we keep the variable names as they should be in the API, we shouldn't need to store the keys. As a fallback in case there are strange keys we would like to modify, we could use custom CodingKeys. I moved the existing keys struct to its own file to maintain for future reference if needed.
Fully agreed on the removal of Marshal. Incidentally, I created an app after I wrote this library and that's when I first found out about the Decodable protocol. Moving forward with the Decodable protocol is the right move.
I'll keep an eye on this as I can, but please ping me once it moves out of the draft phase. Again, thanks so much for stepping up! :)
Hello again, @Chris-Perkins
I've pushed a few changes into this (I added explanation in the PR). I'd be great if you could take a look and tell me what you think about it.
Since you were already using structs for the results of the API calls, I thought it made sense to have them also for the request parameters (also, that's the swift way :P). This can help prevent bad encodings when making calls to the API. Unfortunately, it breaks source compatibility.
We may also want to explicitly call out the date formatting method in the JSON Decoder; not sure if the default one is compatible with Twitch's format
After looking at your comments it seems clear to me that it's a good idea to be explicit about the coding keys for each Codable type. I'm gonna open the API docs and review all the variable names/keys to make them match.
I also saw there are some changes to the pagination you mention. When I'm done with params/result types I'll take a look into that. Maybe we can handle it using a single PaginationData<T> object? (with generics) 🤷🏽♂️
As for the Encoder/Decoder date format, agreed. I'll take a look into it too.
I was able to go through all the Params
files and checked them agains the docs. Tomorrow I'll go over their Data
counterparts. In the meantime, if you read this, I've been thinking about a couple things:
- Is there any reason why there's some support for the v5 API? Should we try to move towards supporting only the New API?
- In that sense, I will be opening an issue so we can keep track of what is covered (sorry for bloating the repo 😬 it just helps me keep track of myself)
- I've noticed some things (like the pagination you mentioned or some filtering options) have changed in the API. When this PR is done, I'll get to fix those things as well
- For the docstrings I just used whatever Twitch says in the docs page. I hope this is alright. Right now what is missing is class docs, I will get to those later
Hi,
So the v5 API endpoints were added as at the time I wrote the library, there were no such replacement API calls in the New Twitch API. I needed this endpoints specifically, so I added them for my own uses.
No issues about keeping track of work items in this. Feel free to add them as you feel necessary. :)
Thanks again for all the work, this is a large task, and really grateful you could take up the torch to make the library better.