slackapi / java-slack-sdk

Slack Developer Kit (including Bolt for Java) for any JVM language
https://slack.dev/java-slack-sdk/
MIT License
568 stars 210 forks source link

Update MigrationExchange sample for Node SDK type #1325

Closed davidlj95 closed 1 month ago

davidlj95 commented 1 month ago

In Slack's Node SDK, there's an issue where MigrationExchangeResponse type is an interface instead of a map. Which forces consumers of the SDK to perform a cast in order to properly consume the API response.

Types for responses there are automagically generated by a script based on the JSON samples in this repo. Then, script uses quicktype package in order to guess the type from the sample JSON. In order to tell quicktype that this field is a type, we can do a bit of trickery as @seratch mentioned in the issue. It's essentially using specific property names in examples in order to let quicktype infer the type is a map and not an interface & therefore avoid the cast by SDK consumers.

After a quick glance on how quicktype infers what should be a map, seems the most straightforward way is to add at least 2 properties with digit-only numbers as keys. Sooo there you go.

Tested by running the types generation script checking out this branch and trick works. Will open a PR later in Node SDK after running script to bring the change there.

Category (place an x in each of the [ ])

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you agree to those rules.

seratch commented 1 month ago

@davidlj95 Thanks for sending this PR! I found that this needs more changes, thus I've made a squashed commit to including your commit credit: https://github.com/slackapi/java-slack-sdk/commit/33645edb3597c842ceca8e18a5894605bfd21a1b