-
Notifications
You must be signed in to change notification settings - Fork 51
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
Typescript declarations #188
base: main
Are you sure you want to change the base?
Conversation
otherwise typescript would not pick up the 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.
Not sure on what the stance is with typescript support, but at the very least we need a solution that doesn't add 3 extra dependencies that the vast majority of users will never use.
@@ -64,6 +68,8 @@ | |||
"klona": "^2.0.6", | |||
"mocha": "^10.7.0", | |||
"mocha-lcov-reporter": "^1.3.0", | |||
"rimraf": "^6.0.1", |
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.
is rimraf really a main dependency? https://www.npmjs.com/package/rimraf
This seems like an entry in devDepencies
also do @types/node
and typescript
have to be in the dependencies for all users? It seems like that introduces a lot of bloat in this library. Are you sure those aren't devDependencies
also?
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.
Those definitively belong in the devDependencies
and they are already.
You are also right that rimraf
is not a necessary dependency and only used during development to clear up between builds. You could also use the usual rm
command on linux, but then it would not be cross-plattform compatible. You can also discard the clear-up completely, because the build step is meant to be used automatically when publishing a package, but then it could be irritating during development.
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.
- The
rimraf
dependency is only for the convenience of the developer, so that previous builds must not be cleared manually during development. You can see it's usage in the package.json-build-types
-script. - The
typescript
dependency is obviously necessary to build the types. You can see usage oftsc
in thebuild-types
-script. - The
@types/node
dependency is used to give typescript the necessary types in the NodeJS environment, so that it will not warn of missing types and validate the usage of internal classes and API.
These are just the basics of typescript development and should not really be a surprise. If you install the package in production those dependencies will not even be downloaded. The published types should then already be included in the package. Alternatively you can publish proper types in the DefinitelyTyped repository, but then you should not just use automatically generated types.
If you do not want extra dependencies, then just include manual types. From what I can expect from the maintainers I did not believe that they were willing to manually type their code or switch to typescript all along. Even then, If you need further information about typescript declarations, then read the documentation: |
Because of issue #139 I created a simple solution to generate typescript declarations for the package.
These declarations will help developers to use the module more effectively, because they will get better code intellisence and typescript support if needed.
Declarations will only be generated automatically on packing/publishing and are excluded from git for now. On generation, the previous types folder will be cleared automatically and typescript will generate the new declarations. They can be generated manually of course.
There are minor changes to the package.json as well
Additionally to the declarations, typescript will also generate type-maps to make it easier for IDEs to match the related source code on code navigation.