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

web-api(fix): include file or contents types in file_uploads arguments #1744

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

zimeg
Copy link
Member

@zimeg zimeg commented Feb 10, 2024

Summary

This PR ensures both the file and content arguments are included in the type for file_uploads. Fixes #1743.

Reviewers

The following snippets can be used to ensure these types are available! Using TypeScript or JSDoc is recommended to experience the full typing experience.

Single file uploads

Everything should remain working just as expected:

(async () => {
    const web = new WebClient(token);
    const result = await web.files.uploadV2({
        channel_id: "C0123456789",
        filename: "GREETINGS.txt",
        content: "hello world",
    });
    console.log(result);
})();

Multiple file uploads

Multiple file uploads can be done without needing to specify a top-level file or content. These attributes are still allowed and will be uploaded if provided, but aren't required anymore!

But the main fix of this PR is exposing the types for the file and content within file_uploads:

(async () => {
    const web = new WebClient(token);
    const result = await web.files.uploadV2({
        channel_id: "C0123456789",
        file_uploads: [
            {
                filename: "README.md",
                file: "./README.md",
            },
            {
                filename: "GREETINGS.txt",
                content: "hello world",
            },
        ]
    });
    console.log(result);
})();

Requirements

@zimeg zimeg added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch area:typescript issues that specifically impact using the package from typescript projects pkg:web-api applies to `@slack/web-api` labels Feb 10, 2024
@zimeg zimeg added this to the web-api@7.0.2 milestone Feb 10, 2024
@zimeg zimeg requested a review from seratch February 10, 2024 21:22
@zimeg zimeg self-assigned this Feb 10, 2024
Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Some notes for the reviewers!

Comment on lines +48 to +57
if ('file' in options) {
return {
file: options.file,
...fileUploadJob,
};
}
throw errorWithCode(
new Error('Either a file or content field is required for valid file upload. You must supply one'),
ErrorCode.FileUploadInvalidArgumentsError,
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to check for the existence of file or whatever key is being done for stronger type safety. The error case would be thrown by the API and is moved ahead a bit here. I think this is also checked earlier in the call too, but isn't picked up by the types.

Comment on lines +1 to +2
/** Omit all keys K from possible union types T */
export type ExcludeFromUnion<T, K extends string> = T extends T ? Omit<T, K> : never;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omit seems to act strange with union types so instead this helper type is added! The T extends T is used to force the type checker to apply the Omit to each member of the union.

Comment on lines +156 to +159
export type FilesUploadV2Arguments = TokenOverridable & (
| FileUploadV2
| (Omit<FileUploadV2, 'file' | 'content'> & FilesUploadV2ArgumentsMultipleFiles)
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Omit seems alright sense the file and content remain optional in the second case! There's magic deep within the type checker.

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Thanks for resolving this!

@zimeg zimeg merged commit 738784b into main Feb 12, 2024
17 checks passed
@zimeg zimeg deleted the fix-1743-files-uploadv2-argument-types branch February 12, 2024 16:12
renovate bot referenced this pull request in adobe/spacecat-shared Feb 17, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[@aws-sdk/client-dynamodb](https://togithub.com/aws/aws-sdk-js-v3/tree/main/clients/client-dynamodb)
([source](https://togithub.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-dynamodb))
| [`3.511.0` ->
`3.515.0`](https://renovatebot.com/diffs/npm/@aws-sdk%2fclient-dynamodb/3.511.0/3.515.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@aws-sdk%2fclient-dynamodb/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@aws-sdk%2fclient-dynamodb/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@aws-sdk%2fclient-dynamodb/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@aws-sdk%2fclient-dynamodb/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[@aws-sdk/client-s3](https://togithub.com/aws/aws-sdk-js-v3/tree/main/clients/client-s3)
([source](https://togithub.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-s3))
| [`3.511.0` ->
`3.515.0`](https://renovatebot.com/diffs/npm/@aws-sdk%2fclient-s3/3.511.0/3.515.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@aws-sdk%2fclient-s3/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@aws-sdk%2fclient-s3/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@aws-sdk%2fclient-s3/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@aws-sdk%2fclient-s3/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[@aws-sdk/client-sqs](https://togithub.com/aws/aws-sdk-js-v3/tree/main/clients/client-sqs)
([source](https://togithub.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-sqs))
| [`3.511.0` ->
`3.515.0`](https://renovatebot.com/diffs/npm/@aws-sdk%2fclient-sqs/3.511.0/3.515.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@aws-sdk%2fclient-sqs/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@aws-sdk%2fclient-sqs/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@aws-sdk%2fclient-sqs/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@aws-sdk%2fclient-sqs/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[@aws-sdk/lib-dynamodb](https://togithub.com/aws/aws-sdk-js-v3/tree/main/lib/lib-dynamodb)
([source](https://togithub.com/aws/aws-sdk-js-v3/tree/HEAD/lib/lib-dynamodb))
| [`3.511.0` ->
`3.515.0`](https://renovatebot.com/diffs/npm/@aws-sdk%2flib-dynamodb/3.511.0/3.515.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@aws-sdk%2flib-dynamodb/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@aws-sdk%2flib-dynamodb/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@aws-sdk%2flib-dynamodb/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@aws-sdk%2flib-dynamodb/3.511.0/3.515.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [@slack/web-api](https://slack.dev/node-slack-sdk/web-api)
([source](https://togithub.com/slackapi/node-slack-sdk)) | [`7.0.1` ->
`7.0.2`](https://renovatebot.com/diffs/npm/@slack%2fweb-api/7.0.1/7.0.2)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@slack%2fweb-api/7.0.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@slack%2fweb-api/7.0.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@slack%2fweb-api/7.0.1/7.0.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@slack%2fweb-api/7.0.1/7.0.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [husky](https://togithub.com/typicode/husky) | [`9.0.6` ->
`9.0.11`](https://renovatebot.com/diffs/npm/husky/9.0.10/9.0.11) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/husky/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/husky/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/husky/9.0.10/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/husky/9.0.10/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [husky](https://togithub.com/typicode/husky) | [`9.0.10` ->
`9.0.11`](https://renovatebot.com/diffs/npm/husky/9.0.10/9.0.11) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/husky/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/husky/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/husky/9.0.10/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/husky/9.0.10/9.0.11?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>aws/aws-sdk-js-v3 (@&#8203;aws-sdk/client-dynamodb)</summary>

###
[`v3.515.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-dynamodb/CHANGELOG.md#35150-2024-02-15)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.514.0...v3.515.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/client-dynamodb](https://togithub.com/aws-sdk/client-dynamodb)

###
[`v3.514.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-dynamodb/CHANGELOG.md#35140-2024-02-14)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.513.0...v3.514.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/client-dynamodb](https://togithub.com/aws-sdk/client-dynamodb)

###
[`v3.513.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-dynamodb/CHANGELOG.md#35130-2024-02-13)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.511.0...v3.513.0)

##### Features

- **experimentalIdentityAndAuth:** release phase for services without
customizations
([#&#8203;5787](https://togithub.com/aws/aws-sdk-js-v3/issues/5787))
([4004ff6](https://togithub.com/aws/aws-sdk-js-v3/commit/4004ff68a8ad20f6e60e8fab1f8952928f92f4b7))

</details>

<details>
<summary>aws/aws-sdk-js-v3 (@&#8203;aws-sdk/client-s3)</summary>

###
[`v3.515.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-s3/CHANGELOG.md#35150-2024-02-15)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.514.0...v3.515.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/client-s3](https://togithub.com/aws-sdk/client-s3)

###
[`v3.514.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-s3/CHANGELOG.md#35140-2024-02-14)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.513.0...v3.514.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/client-s3](https://togithub.com/aws-sdk/client-s3)

###
[`v3.513.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-s3/CHANGELOG.md#35130-2024-02-13)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.511.0...v3.513.0)

##### Features

- **experimentalIdentityAndAuth:** release phase for services without
customizations
([#&#8203;5787](https://togithub.com/aws/aws-sdk-js-v3/issues/5787))
([4004ff6](https://togithub.com/aws/aws-sdk-js-v3/commit/4004ff68a8ad20f6e60e8fab1f8952928f92f4b7))

</details>

<details>
<summary>aws/aws-sdk-js-v3 (@&#8203;aws-sdk/client-sqs)</summary>

###
[`v3.515.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-sqs/CHANGELOG.md#35150-2024-02-15)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.514.0...v3.515.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/client-sqs](https://togithub.com/aws-sdk/client-sqs)

###
[`v3.514.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-sqs/CHANGELOG.md#35140-2024-02-14)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.513.0...v3.514.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/client-sqs](https://togithub.com/aws-sdk/client-sqs)

###
[`v3.513.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/clients/client-sqs/CHANGELOG.md#35130-2024-02-13)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.511.0...v3.513.0)

##### Features

- **experimentalIdentityAndAuth:** release phase for services without
customizations
([#&#8203;5787](https://togithub.com/aws/aws-sdk-js-v3/issues/5787))
([4004ff6](https://togithub.com/aws/aws-sdk-js-v3/commit/4004ff68a8ad20f6e60e8fab1f8952928f92f4b7))

</details>

<details>
<summary>aws/aws-sdk-js-v3 (@&#8203;aws-sdk/lib-dynamodb)</summary>

###
[`v3.515.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/lib/lib-dynamodb/CHANGELOG.md#35150-2024-02-15)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.514.0...v3.515.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/lib-dynamodb](https://togithub.com/aws-sdk/lib-dynamodb)

###
[`v3.514.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/lib/lib-dynamodb/CHANGELOG.md#35140-2024-02-14)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.513.0...v3.514.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/lib-dynamodb](https://togithub.com/aws-sdk/lib-dynamodb)

###
[`v3.513.0`](https://togithub.com/aws/aws-sdk-js-v3/blob/HEAD/lib/lib-dynamodb/CHANGELOG.md#35130-2024-02-13)

[Compare
Source](https://togithub.com/aws/aws-sdk-js-v3/compare/v3.511.0...v3.513.0)

**Note:** Version bump only for package
[@&#8203;aws-sdk/lib-dynamodb](https://togithub.com/aws-sdk/lib-dynamodb)

</details>

<details>
<summary>slackapi/node-slack-sdk (@&#8203;slack/web-api)</summary>

###
[`v7.0.2`](https://togithub.com/slackapi/node-slack-sdk/releases/tag/%40slack/web-api%407.0.2)

[Compare
Source](https://togithub.com/slackapi/node-slack-sdk/compare/@slack/web-api@7.0.1...@slack/web-api@7.0.2)

#### What's Changed

Nothing major or even minor in this release! Just a few patches:

- web-api: prevent apps.event.authorizations.list API from ever sending
token in the body by [@&#8203;filmaj](https://togithub.com/filmaj) in
[https://github.com/slackapi/node-slack-sdk/pull/1737](https://togithub.com/slackapi/node-slack-sdk/pull/1737)
- web-api(fix): include file or contents types in file_uploads arguments
by [@&#8203;zimeg](https://togithub.com/zimeg) in
[https://github.com/slackapi/node-slack-sdk/pull/1744](https://togithub.com/slackapi/node-slack-sdk/pull/1744)
- web-api(fix): share tokens provided as arguments in files upload to
upload jobs by [@&#8203;zimeg](https://togithub.com/zimeg) in
[https://github.com/slackapi/node-slack-sdk/pull/1745](https://togithub.com/slackapi/node-slack-sdk/pull/1745)
- web-api(chore): release
[@&#8203;slack/web-api](https://togithub.com/slack/web-api)[@&#8203;7](https://togithub.com/7).0.2
by [@&#8203;zimeg](https://togithub.com/zimeg) in
[https://github.com/slackapi/node-slack-sdk/pull/1746](https://togithub.com/slackapi/node-slack-sdk/pull/1746)

**Full Changelog**:
https://github.com/slackapi/node-slack-sdk/compare/[@&#8203;slack/cli-hooks](https://togithub.com/slack/cli-hooks)[@&#8203;1](https://togithub.com/1).0.0...[@&#8203;slack/web-api](https://togithub.com/slack/web-api)[@&#8203;7](https://togithub.com/7).0.2

</details>

<details>
<summary>typicode/husky (husky)</summary>

###
[`v9.0.11`](https://togithub.com/typicode/husky/releases/tag/v9.0.11)

[Compare
Source](https://togithub.com/typicode/husky/compare/v9.0.10...v9.0.11)

- chore: update package-lock.json by
[@&#8203;btea](https://togithub.com/btea) in
[https://github.com/typicode/husky/pull/1383](https://togithub.com/typicode/husky/pull/1383)
- fix: husky=0 in init (fixes
[#&#8203;1393](https://togithub.com/typicode/husky/issues/1393)) by
[@&#8203;typicode](https://togithub.com/typicode) in
[https://github.com/typicode/husky/pull/1395](https://togithub.com/typicode/husky/pull/1395)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 2pm on Saturday" in timezone
Europe/Zurich, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/adobe/spacecat-shared).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xOTEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE5MS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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 bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented pkg:web-api applies to `@slack/web-api` semver:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typescript type error for web.files.uploadV2() arguments when using file_uploads
2 participants