Skip to content

fix: destination fetch option XOR typing#6701

Open
davidkna-sap wants to merge 3 commits into
mainfrom
davidkna-sap_fix-service-type
Open

fix: destination fetch option XOR typing#6701
davidkna-sap wants to merge 3 commits into
mainfrom
davidkna-sap_fix-service-type

Conversation

@davidkna-sap

@davidkna-sap davidkna-sap commented Jun 24, 2026

Copy link
Copy Markdown
Member

Related to https://github.com/SAP/ai-sdk-js-backlog/issues/598.

The service key was still hidden due to the way types were composed a higher levels.

@davidkna-sap davidkna-sap force-pushed the davidkna-sap_fix-service-type branch from 7385dda to e9757cf Compare June 24, 2026 08:03
@davidkna-sap davidkna-sap marked this pull request as ready for review June 24, 2026 08:17
InjunPark-sap
InjunPark-sap previously approved these changes Jun 24, 2026

@KavithaSiva KavithaSiva left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing the bug, I have added some follow-up questions, suggestions.

Comment on lines +481 to +483
type WithoutExclusive<T, U> = {
[P in Exclude<keyof T, keyof U>]?: never;
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[req] Consider moving WithoutExclusive to packages/util/src/types.ts and also updating XOR type to start using this type instead.

Although, updating XOR is a breaking change, I believe the update would enforce correct behaviour of the type, for underlying types with shared keys.

@marikaner WDY think?

Comment thread test-packages/type-tests/test/destination-or-fetch-options.test-d.ts Outdated
* Stripping `destinationName` from `DestinationFetchOptions` and letting the inner discriminated
* union of `DestinationFromServiceBindingOptions` own that key fixes it.
*/
type FetchOptionsLeaf = Omit<DestinationFetchOptions, 'destinationName'> &

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[q] I find the name cryptic, what about DestinationFetchOrServiceOptions instead?

…t-d.ts

Co-authored-by: KavithaSiva <kavitha.sivakumar@sap.com>
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