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
Add tests for Json
and Hex
type
#42
Conversation
test-d/package.json
Outdated
{ | ||
"name": "test-d", | ||
"private": true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tsd
seems to be built for declaration-only packages, as it looks at the types
field in package.json
(i.e., dist/index.d.ts
, with no way to disable that behaviour it seems. For that reason, I moved it into a separate folder and added a dummy package.json
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like, judging from the README, that if you pass a path to tsd
, then it won't look in the types
field. Is this not true in your experience? Do you think that people will try to run tsd
on its own instead of yarn test:types
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to get it to work using --typings
or by passing a path, but apparently the behaviour is different if you specify a directory
in the config. Seems to work fine now with the test being in the src
folder.
f222bc8
to
732837e
Compare
732837e
to
03622d1
Compare
@Gudahtt Pinging you on this PR since you added these types originally! |
Simplified the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but I want to make sure Mark sees this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The test that was removed was rendered unnecessary by the exactOptionalPropertyTypes
TypeScript option. An optional property can no longer be undefined
.
Closes #40.
The test is almost completely copied from
@metamask/types
, with just one small adjustment. The following test would result in a compile-time type error:I added
undefined
toa
's type, which makes it not assignable toJson
, which seems to be the behaviour we want.