Closed carbohydrate closed 7 months ago
Hey there, thanks for wanting to open an PR to fix these inaccuracies!
For the most part your thoughts are on point, anything that does not always return, should be marked as optional, and things that do always return should be marked as their type or null (traditionally this means that its not optional so if it was optional it should not have an optional badge). If you're finding that data is returning sometimes something, sometimes null and sometimes nothing then I would open a ticket in the mtgjson repo about that because that is historically not expected behavior. From a documentation standpoint data either returns sometimes something or sometimes nothing
The BoosterConfig
issue you are running in to is a bit confusing for me. This is relatively new documentation and I may have got it wrong. Feel free to comment on the data structure returned from various sets and lets compare with what were seeing in the documentation.
As far as the the "Partial" issue, I think that would be just find to document it that way. I usually don't deal with partials so I had not thought of doing that. It also can get a bit "mysterious" to our non-TS friends but I cant think of another solution because like you said, these keys are all a bit scattered.
Location of update needed
This one I'm less sure about, could probably be accomplished a few different ways.
BoosterPack contents: Record<string, number> -> Partial<Record<string, number>> BoosterConfig says boosters is BoosterPack[]. I think typescript is getting mad that some items in the array will have for example 'foilUncommon' defined, but others do not. I started down the path of doing something like this:
But decided just defining it as partial was better, as it the properties across the sets on these items are vastly different.
Additional context
I plan on working on a PR for this, just wanted a place to collect my notes. I also plan on working on audits on other files, just started here first.
Also, not 100% sure some of the suggestions are correct, specifically things that are optional, but then have a null value. Maybe the project should just omit those properties? I feel like I remember reading something about that is the project that generates the files, but maybe that is more work then it's worth? Let me know if anything should be done differently.