-
Notifications
You must be signed in to change notification settings - Fork 661
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
Conversation
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.
📝 Some notes for the reviewers!
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, | ||
); |
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.
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.
/** Omit all keys K from possible union types T */ | ||
export type ExcludeFromUnion<T, K extends string> = T extends T ? Omit<T, K> : never; |
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.
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.
export type FilesUploadV2Arguments = TokenOverridable & ( | ||
| FileUploadV2 | ||
| (Omit<FileUploadV2, 'file' | 'content'> & FilesUploadV2ArgumentsMultipleFiles) | ||
); |
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.
This Omit
seems alright sense the file
and content
remain optional in the second case! There's magic deep within the type checker.
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.
Cool! Thanks for resolving this!
[![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 (@​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 [@​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 [@​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 ([#​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 (@​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 [@​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 [@​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 ([#​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 (@​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 [@​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 [@​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 ([#​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 (@​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 [@​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 [@​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 [@​aws-sdk/lib-dynamodb](https://togithub.com/aws-sdk/lib-dynamodb) </details> <details> <summary>slackapi/node-slack-sdk (@​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 [@​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 [@​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 [@​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 [@​slack/web-api](https://togithub.com/slack/web-api)[@​7](https://togithub.com/7).0.2 by [@​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/[@​slack/cli-hooks](https://togithub.com/slack/cli-hooks)[@​1](https://togithub.com/1).0.0...[@​slack/web-api](https://togithub.com/slack/web-api)[@​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 [@​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 [#​1393](https://togithub.com/typicode/husky/issues/1393)) by [@​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>
Summary
This PR ensures both the
file
andcontent
arguments are included in the type forfile_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:
Multiple file uploads
Multiple file uploads can be done without needing to specify a top-level
file
orcontent
. 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
andcontent
withinfile_uploads
:Requirements