Skip to content

Make CompressionInputConfig properties optional #1133

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

Merged
merged 2 commits into from
Jan 4, 2024

Conversation

trivikr
Copy link
Contributor

@trivikr trivikr commented Jan 4, 2024

Issue #, if available:
Refs: aws/aws-sdk-js-v3#5643 (comment)

Description of changes:
Make CompressionInputConfig properties optional

  • The properties need to be optional, as they don't need to be passed during client creation.
  • The properties need to be required during resolution, since they're resolved by runtimeConfig.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@trivikr trivikr requested review from a team as code owners January 4, 2024 16:43
@trivikr trivikr requested a review from milesziemer January 4, 2024 16:43
Comment on lines -8 to 13
export const resolveCompressionConfig = <T>(input: T & CompressionInputConfig): T & CompressionResolvedConfig => ({
export const resolveCompressionConfig = <T>(
input: T & Required<CompressionInputConfig>
): T & CompressionResolvedConfig => ({
...input,
disableRequestCompression: normalizeProvider(input.disableRequestCompression),
requestMinCompressionSizeBytes: async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is it safer to guard against undefined values for disableRequestCompression and requestMinCompressionSizeBytes, and not make CompressionInputConfig required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There won't be undefined values when calling resolveCompressionConfig as runtimeConfig resolution ensure that it's defined. If it doesn't, the client compilation should fail like it's happening in protocol tests.

Also, the Required doesn't allow undefined.
Playground Link

@trivikr trivikr merged commit f48de63 into main Jan 4, 2024
@trivikr trivikr deleted the middleware-compression-config-optional branch January 4, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants