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

fix(packages): maintain singleton config object #6959

Merged
merged 1 commit into from
Mar 24, 2025

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented Mar 21, 2025

Related to smithy-lang/smithy-typescript#1550

This change makes all config resolvers mutate the input config instead of creating a shallow copy. The shallow copies, if preserved by references, can result in reading inconsistent config data.

@kuhe kuhe requested a review from a team as a code owner March 21, 2025 23:56
@kuhe kuhe force-pushed the fix/object-ownership branch from 7c7bac3 to 0b7c699 Compare March 21, 2025 23:58
@kuhe kuhe marked this pull request as draft March 22, 2025 00:00
@kuhe kuhe force-pushed the fix/object-ownership branch 2 times, most recently from e84ade3 to 3dcd29f Compare March 22, 2025 18:42
@kuhe kuhe marked this pull request as ready for review March 22, 2025 18:42
@kuhe kuhe force-pushed the fix/object-ownership branch 2 times, most recently from f8bc334 to 9e28d8a Compare March 23, 2025 15:16
endpointDiscoveryEnabled !== undefined
? () => Promise.resolve(endpointDiscoveryEnabled)
: endpointDiscoveryEnabledProvider,
isClientEndpointDiscoveryEnabled: endpointDiscoveryEnabled !== undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this boolean seems wrong... what if endpointDiscoveryEnabled === false?

not related to this PR though

export const resolveTokenConfig = <T>(input: T & TokenInputConfig): T & TokenResolvedConfig => {
const { token } = input;
return Object.assign(input, {
token: token ? normalizeTokenProvider(token) : tokenDefaultProvider(input as any),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this fallback makes no sense to me since input has no overlap with FromSsoInit, the type of the falsy branch.

@kuhe kuhe force-pushed the fix/object-ownership branch from 9e28d8a to 4249188 Compare March 24, 2025 19:00
@kuhe kuhe merged commit 6034850 into aws:main Mar 24, 2025
4 checks passed
@kuhe kuhe deleted the fix/object-ownership branch March 24, 2025 20:50
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.

2 participants