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

refactor: enable exactOptionalPropertyTypes #4733

Closed
wants to merge 4 commits into from

Conversation

DeJayDev
Copy link

@DeJayDev DeJayDev commented May 26, 2024

Which problem is this PR solving?

I aim to close #3713 by enabling exactOptionalPropertyTypes in the tsconfig.base.json.

Fixes #3713.

Short description of the changes

Per the description of the linked PR, this will allow OpenTelemetry typings to resolve without error in modern projects or projects who are not using skipLibCheck.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@DeJayDev DeJayDev requested a review from a team May 26, 2024 20:16
Copy link

linux-foundation-easycla bot commented May 26, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@DeJayDev
Copy link
Author

I'm going to need help here, it's not as easy as just flipping the option it seems :/

My understanding is this would require a very large rewrite that I just cannot do. In it's current position, all tests run and pass locally (leading to the creation of the PR) but running compile like the Actions do will fail.

@pichlermarc
Copy link
Member

I'm going to need help here, it's not as easy as just flipping the option it seems :/

My understanding is this would require a very large rewrite that I just cannot do. In it's current position, all tests run and pass locally (leading to the creation of the PR) but running compile like the Actions do will fail.

Yes all the errors one is seeing locally when using it with exactOptionalPropertyTypes will also occur in internal code and tests when the switch is turned on (it's all written with exactOptionalPropertyTypes off) . Tests may work as it's not re-compiled yet - possibly the version of ts-node that's used does not error on this option. The amount of work required for us to do this is the reason why we have not made the change yet - there are other issues that currently don't have a work-around yet so they get priority.

It'll be a massive change to the code-base. I wish it was as easy as flipping a switch 😓

@pichlermarc
Copy link
Member

@DeJayDev I don't have time to work on this at the moment, so with your permission I'd like to close this PR to indicate to others that this is still up-for-grabs.

@DeJayDev DeJayDev closed this Jun 28, 2024
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.

Set the exactOptionalPropertyTypes tsconfig option
2 participants