-
Notifications
You must be signed in to change notification settings - Fork 662
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
fix: improve type for upload file v2 #1848
Conversation
@@ -144,6 +144,8 @@ export type FileUploadV2 = FileUpload & { | |||
alt_text?: string; | |||
/** @description Channel ID where the file will be shared. If not specified the file will be private. */ | |||
channel_id?: string; | |||
/** @deprecated use channel_id instead */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we could do anything else then deprecate this 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nailed it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Nicely done. Hopefully sorting through all the different combinations of types/interfaces wasn't too bad 🙏
And don't worry about the node 22 failures - I am looking into it. I think it's a new npm bug. As long as 18 and 20 pass, you are good! Merge away!
@@ -144,6 +144,8 @@ export type FileUploadV2 = FileUpload & { | |||
alt_text?: string; | |||
/** @description Channel ID where the file will be shared. If not specified the file will be private. */ | |||
channel_id?: string; | |||
/** @deprecated use channel_id instead */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nailed it!
Summary
This PR aims to fix #1846 by adding additional typing and related tests
Testing
cd packages/web-api
npm build
app.ts
npm install ~/path/to/node-slack-sdk/packages/web-api
the above should not create any typescript errors or warningschannel_id
->channels
, you should see the following warning when running the appRequirements (place an
x
in each[ ]
)