Add TS declaration file and testing #2
Conversation
francisdaigle
force-pushed
the
feature/add-ts-support
branch
4 times, most recently
from
1e16ede
to
e78cc7a
Compare
There was a problem hiding this comment.
Thank you for the PR!
I have put some comments for some lines.
Unfortunately, I have no experience with TypeScript. I just did not get that why you wrote some tests in TypeScript? the test.js file in the root directory already contains more comprehensive tests in vanilla js. I think that it's not necessary to rewrite them in TypeScript.
types/tsconfig.json
Outdated
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "extends": "../node_modules/fastify-tsconfig/tsconfig.json", | |||
There was a problem hiding this comment.
It looks like TypeScript resolves node_modules by default since v3.2
.github/funding.yml
Outdated
| @@ -1 +0,0 @@ | |||
| github: hsynlms | |||
There was a problem hiding this comment.
Is there any specific reason to remove the file?
There was a problem hiding this comment.
No reason! Can be replaced 👍
|
Hi @hsynlms :)
Possibly not. The TS tests do not replicate the JS tests in their entirety -- just the basics. Very helpful when determining/illustrating what should (and should not) be included in the declaration file. The tests can be removed if you would prefer. |
francisdaigle
force-pushed
the
feature/add-ts-support
branch
from
e78cc7a
to
84a9a43
Compare
francisdaigle
force-pushed
the
feature/add-ts-support
branch
from
84a9a43
to
0e899f0
Compare
There was a problem hiding this comment.
Hi @francisdaigle :)
Thank you again for your efforts. Now it looks better. Please feel free to open new PR for TypeScript testing changes.
The last remained thing I am going to ask you that the library is Standard JS compatible. Can you also make the changes compatible as well?
francisdaigle
force-pushed
the
feature/add-ts-support
branch
from
0e899f0
to
f7480fa
Compare
No description provided.