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

Clean typeguard syntax #163

Closed
wants to merge 1 commit into from
Closed

Clean typeguard syntax #163

wants to merge 1 commit into from

Conversation

slifty
Copy link
Member

@slifty slifty commented Jun 18, 2022

This PR creates a somewhat cleaner type guard syntax.

Functionality should basically be the same.

This is the first typeguard in the repository and the syntax I used was
a bit more verbose than necessary.  By casting once we save a whole
bunch of redundancy and the guard becomes easier to understand
@codecov-commenter
Copy link

Codecov Report

Merging #163 (c217737) into main (10be492) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #163   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines           48        49    +1     
  Branches        16        16           
=========================================
+ Hits            48        49    +1     
Impacted Files Coverage Δ
src/types/PayloadParameters.ts 100.00% <100.00%> (ø)

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

@slifty
Copy link
Member Author

slifty commented Jun 18, 2022

(Not sure this is better yet -- gonna sleep on it)

@slifty
Copy link
Member Author

slifty commented Jun 20, 2022

Some thoughts related to this.

  1. This might not actually be better.
  2. @jasonaowen pointed out that joi or something like it might be a good tool for this.
  3. I was realizing "oh, this is maybe the only time I need a type guard" but then Jason reminded me "since this is a library that may be used outside of TypeScript it's actually not a bad idea to explicitly type guard ALL inputs regardless)

I'm likely going to close this issue without merging, and instead opt for a validation library (and then validate more inputs in more places even if they're typed)

@slifty
Copy link
Member Author

slifty commented Jun 20, 2022

Closing this since #167 makes it obsolete.

@slifty slifty closed this Jun 20, 2022
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