-
Notifications
You must be signed in to change notification settings - Fork 21
feat(javascript): add exactOptionalPropertyTypes
to tsconfig
#4935
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
Conversation
✔️ Code generated!
📊 Benchmark resultsBenchmarks performed on the method using a mock server, the results might not reflect the real-world performance.
|
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.
looks great ! I think the scope is a bit too large, this only needs to apply to types, the rest is redundant I think
@@ -17,7 +17,7 @@ export function createIterablePromise<TResponse>({ | |||
error, | |||
timeout = (): number => 0, | |||
}: CreateIterablePromise<TResponse>): Promise<TResponse> { | |||
const retry = (previousResponse?: TResponse): Promise<TResponse> => { | |||
const retry = (previousResponse?: TResponse | undefined): Promise<TResponse> => { |
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.
exactOptionalPropertyTypes is only for type
or interface
, you don't need to change the parameters in functions
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.
I prefer to stay consistent consider I don't really know what's the upstream impact of this, it added a bit more of work but at least there should be no surprise
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.
I hate this ts option, it makes the syntax redundant
gg for taking care of it |
i'm not a fan as well since it's just for the sake of explicitly typing things but if it makes things easier on the consumer side it seems like a good addition |
… (generated) [skip ci] Co-authored-by: Clément Vannicatte <[email protected]>
…ated) algolia/api-clients-automation#4935 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clément Vannicatte <[email protected]>
🧭 What and Why
🎟 JIRA Ticket: https://algolia.atlassian.net/browse/DI-3873
Changes included:
closes algolia/algoliasearch-client-javascript#1587
for sake of explicit typing, we can enable this option as requested in algolia/algoliasearch-client-javascript#1587 to explicitely add
undefined
to optional properties.