Skip to content

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

Merged
jaredpalmer merged 1 commit intojaredpalmer:masterfrom
agilgur5:sade-types-pre-refactor
Feb 4, 2020
Merged

(types): add @types/sade, fix transpileOnly default#476
jaredpalmer merged 1 commit intojaredpalmer:masterfrom
agilgur5:sade-types-pre-refactor

Conversation

@agilgur5
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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