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

chore(TS): Fix when options null enforce type #8

Merged

Conversation

borracciaBlu
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #8 (aad9f5a) into main (c3b4d7f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main       #8   +/-   ##
=======================================
  Coverage   95.02%   95.02%           
=======================================
  Files          58       58           
  Lines         603      603           
=======================================
  Hits          573      573           
  Misses         30       30           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@borracciaBlu
Copy link
Contributor Author

Hi @medikoo ,

sorry, deeply shame about it. When options was null it was matching the case for {isOptional?: boolean} and so thinking was in a T | null scenario instead of T.

This should fix it.

Let me know if you have any comment on this.

Copy link
Owner

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

No worries, happens.

I wonder whether we can setup some simple test to test those typings?

I think we cannot test it with real tests, as they go into real edge cases, which typings do not cover, and it'll be really hard for typing to adapt to that

@medikoo medikoo merged commit 4a54066 into medikoo:main Aug 5, 2022
@borracciaBlu borracciaBlu deleted the update/ts-types-optional-no-options-case branch August 5, 2022 14:15
@borracciaBlu
Copy link
Contributor Author

In local I'm using dtslint as in DefinitelyTyped but I neglected to test it against actual code, and that was the big mistake.

I guess the lesson would be that the ultimate test is actual code in ts and have tsc happy at compile time...

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.

2 participants