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

(types): add @types/sade, fix transpileOnly default #476

Merged
merged 1 commit into from
Feb 4, 2020

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented Jan 30, 2020

  • transpileOnly default being the wrong type was found as a result of
    the better typing

I originally made this as a follow-up to #407 , as prog is passed around as an argument there (and is any without these types), but that hasn't gotten a response in almost a month now 😐 😐
So figured I'd separate it out.

Just as a note, the @types/sade package was reviewed by DT, but never got a response from the maintainer, lukeed/sade#27 (comment)

@agilgur5
Copy link
Collaborator Author

Huh, this test is failing locally too and I'm not totally sure why 🤔 Is 'false' getting evaluated wrong?

- transpileOnly default being the wrong type was found as a result of
  the better typing
  - can only be string | number, not boolean
@agilgur5 agilgur5 force-pushed the sade-types-pre-refactor branch from 810bf28 to 9f772e3 Compare February 1, 2020 14:03
@agilgur5
Copy link
Collaborator Author

agilgur5 commented Feb 1, 2020

Yep, looks like 'false' was kept as a string. The boolean worked before but didn't type-check and the docs say:

You probably only want to define a default value if you're expecting a String or Number value type.

So changed it to not have a default and modified the conditional

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