Skip to content

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

Merged
merged 4 commits into from
Jun 9, 2025

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Jun 4, 2025

🧭 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.

@shortcuts shortcuts self-assigned this Jun 4, 2025
@algolia-bot
Copy link
Collaborator

algolia-bot commented Jun 4, 2025

✔️ Code generated!

Name Link
🪓 Triggered by 808f5271e1a7d0464ce16f8501b23523eae75194
🍃 Generated commit c9ab39ad5cf5829ce7a2b6d08e995774251b22d7
🌲 Generated branch generated/fix/javascript-exactOptionalPropertyTypes
📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Req/s
javascript 1647

@shortcuts shortcuts marked this pull request as ready for review June 4, 2025 13:05
@shortcuts shortcuts requested a review from a team as a code owner June 4, 2025 13:05
@shortcuts shortcuts requested review from morganleroi and Fluf22 June 4, 2025 13:05
Copy link
Collaborator

@millotp millotp left a 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> => {
Copy link
Collaborator

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

Copy link
Member Author

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

Copy link
Collaborator

@millotp millotp left a 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

@millotp
Copy link
Collaborator

millotp commented Jun 6, 2025

gg for taking care of it

@shortcuts
Copy link
Member Author

I hate this ts option, it makes the syntax redundant

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

@shortcuts shortcuts enabled auto-merge (squash) June 9, 2025 08:32
@shortcuts shortcuts merged commit e0689b5 into main Jun 9, 2025
12 checks passed
@shortcuts shortcuts deleted the fix/javascript-exactOptionalPropertyTypes branch June 9, 2025 08:46
algolia-bot added a commit that referenced this pull request Jun 9, 2025
… (generated) [skip ci]

Co-authored-by: Clément Vannicatte <[email protected]>
algolia-bot added a commit to algolia/algoliasearch-client-javascript that referenced this pull request Jun 9, 2025
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.

[bug]: Support exactOptionalPropertyTypes
3 participants