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

types: add event payloads #1395

Closed
1 of 7 tasks
JMauroNeto opened this issue Dec 4, 2021 · 8 comments · Fixed by #1907
Closed
1 of 7 tasks

types: add event payloads #1395

JMauroNeto opened this issue Dec 4, 2021 · 8 comments · Fixed by #1907
Labels
area:typescript issues that specifically impact using the package from typescript projects auto-triage-skip discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality pkg:types applies to `@slack/types` semver:minor

Comments

@JMauroNeto
Copy link

JMauroNeto commented Dec 4, 2021

Edits

What about adding types for event payloads to the types package? This could be used to then help address a variety of issues across different packages (bolt, socket-mode, deno-sdk):

Original Message

Is really necessary to use the type generator instead of manually improve types? I've found out that the ModalView is missing some parameters, like app_installed_team_id, id and state. About the state, on the places that have references to it, it is just a string, it could be so much better. Depending on your answer, I can help you with the types that I've created.

Packages:

Select all that apply:

  • @slack/web-api
  • @slack/types
  • @slack/rtm-api
  • @slack/webhooks
  • @slack/oauth
  • @slack/socket-mode
  • I don't know
@JMauroNeto JMauroNeto changed the title Missing parameters Missing parameters on types Dec 4, 2021
@seratch
Copy link
Member

seratch commented Dec 4, 2021

Hi @JMauroNeto, thank for writing in!

Could you elaborate a bit more? I might not fully understand the issue you mention here.

the ModalView is missing some parameters, like app_installed_team_id, id and state

You've selected web-api package in the description but are you talking about https://github.com/slackapi/node-slack-sdk/blob/%40slack/types%402.4.0/packages/types/src/index.ts#L42-L53 ? If so, the types package is not auto-generated.

We are happy to improve it. The bolt-js project has a duplicated but more complete type definition https://github.com/slackapi/bolt-js/blob/main/src/types/view/index.ts Is this something similar to your own type definition?

If you mean the ViewsXXX response types in web-api package, they have the properties that you mentioned correctly (only the too simple State interface can be improved). Indeed, they are auto-generated but the benefit is not just code-generation. Our other SDK's integration tests have been detecting newly added properties in production API responses and the code generation utilize the outputs by those tests. Thanks to that, we can easily make the updates on the Node SDK in a timely manner.

If we stop doing so, maintaining the types can be harder. Actually, before starting doing this, we didn't have any type definitions for the API responses. We are not planning to change this approach at least in the short term (if future maintainers want to change this, I'm totally fine with it, though).

Thanks again for taking the time to write in here.

@seratch seratch added area:typescript issues that specifically impact using the package from typescript projects needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info pkg:types applies to `@slack/types` pkg:web-api applies to `@slack/web-api` and removed untriaged labels Dec 4, 2021
@seratch seratch self-assigned this Dec 4, 2021
@JMauroNeto
Copy link
Author

JMauroNeto commented Dec 4, 2021

Hey @seratch, thanks for your quick response.

Sorry, I've checked the incorrect option, I was actually referring to the file that you mentioned, on @slack/types.

And yes, my type definitions are more similar to the types on the bolt-js project.

So, can we improve the node sdk, since we have this project as a reference?


PS: Btw, I've been using [your types](https://github.com/seratch/seratch-slack-types) for a while, and helped me a lot, thanks again!

@seratch seratch removed the pkg:web-api applies to `@slack/web-api` label Dec 4, 2021
@seratch seratch added this to the [email protected] milestone Dec 4, 2021
@seratch
Copy link
Member

seratch commented Dec 4, 2021

So, can we improve the node sdk, since we have this project as a reference?

Yes, we are happy to improve it. If you have time and interest, your pull requests are always welcome! If you are busy, this project's maintainers will improve it sometime soon.

PS: Btw, I've been using your types for a while, and helped me a lot, thanks again!

Wow, I'm so glad to hear that (and a bit sorry about that I haven't been updating the package for a while 🙇 )

@seratch seratch changed the title Missing parameters on types Add missing properties in ModalView type in types package Dec 4, 2021
@seratch seratch changed the title Add missing properties in ModalView type in types package Add missing properties to ModalView type in types package Dec 4, 2021
@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented good first issue and removed needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info labels Jul 4, 2022
@filmaj
Copy link
Contributor

filmaj commented Sep 26, 2023

@seratch what do you think about trying to integrate https://github.com/seratch/seratch-slack-types into @slack/types as part of the next major version release I am (slowly) working on? My intention is to have @slack/types be the source of truth for payloads/events/block kit/any kind of shapes related to Slack's business domain.

@seratch
Copy link
Member

seratch commented Sep 26, 2023

@filmaj That sounds great. Perhaps, we may need to manually adjust some parts of it as it's auto-generated, but merging good parts from it should be a good approach. Also, you may be interested in https://github.com/seratch/slack-web-api-client/tree/main/src/block-kit for Block Kit too. The Block Kit typing is more sophisticated than any others.

@filmaj filmaj changed the title Add missing properties to ModalView type in types package Add missing payloads/shapes/interfaces/types from bolt-js, seratch-slack-types and slack-web-api-client to @slack/types Sep 26, 2023
@filmaj
Copy link
Contributor

filmaj commented Sep 26, 2023

Ooo nice! Absolutely interested! I have updated the title of this issue and assigned it to the 3.0 milestone for the types package.

@JMauroNeto
Copy link
Author

Sorry, I left the project where I used to work with the types mentioned when I opened the issue. By the way, can I help with something?

@filmaj
Copy link
Contributor

filmaj commented Sep 26, 2023

👋 Hi @JMauroNeto

One thing perhaps that you can assist with: in your original submission you mentioned:

I can help you with the types that I've created.

Do you have any types that you created that you found helpful for your projects? I'm trying to aggregate projects where devs are formalizing the types that are useful to them. I'd like to make improvements to the @slack/types package - it is very popular and I think there is a lot of room for improvement.

@filmaj filmaj added semver:minor enhancement M-T: A feature request for new functionality auto-triage-skip and removed bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented good first issue labels Jun 14, 2024
@filmaj filmaj removed this from the [email protected] milestone Jun 14, 2024
@filmaj filmaj changed the title Add missing payloads/shapes/interfaces/types from bolt-js, seratch-slack-types and slack-web-api-client to @slack/types types: add event payloads Jun 14, 2024
@filmaj filmaj added the discussion M-T: An issue where more input is needed to reach a decision label Jun 14, 2024
@filmaj filmaj added this to the [email protected] milestone Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:typescript issues that specifically impact using the package from typescript projects auto-triage-skip discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality pkg:types applies to `@slack/types` semver:minor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants