fix: Exact optional property types #205
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bucketId?: string -> bucketId: string | undefined
JeongJuhyeon
changed the title
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Bug fix and TS config update
What is the current behavior?
The
StorageFileApi.bucketIdfield type isbucketId?: string, which breaks whenexactOptionalPropertyTypesis on. This causes headaches for folks trying to use the library with the flag on.What is the new behavior?
StorageFileApi.bucketIdfield type:bucketId: string | undefined"exactOptionalPropertyTypes": truein TS configNow it should play nice with the flag on without breaking existing setups.
Additional context
Here's why I think this change is useful:
TypeScript itself recommends using this flag. The reason it is not in
strictis just to avoid breaking too much existing user code.Right now, anyone with this flag on can't use the library without it failing to compile. Using
skipLibCheckisn't great because it skips all.d.tsfiles, including user-made ones (see this issue).Except for the one field I fixed, the package already follows this rule.
In my current project with 58 libraries, this is the only one that breaks with the flag on, indicating it's already a standard in the ecosystem.
These tweaks should make the library more robust and play better with strict TS setups, without causing issues for current users. Let me know if you need any more info!