-
Notifications
You must be signed in to change notification settings - Fork 113
Add typings #74
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 typings #74
Conversation
Followed recommendations in [dtslint tool](https://github.com/microsoft/dtslint) and [TypeScript documentation](http://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html). Took tsconfig.json and tslint.json from @testing-library/dom, although it looks like they just copied the examples in the dtslint docs. Closes testing-library#60.
Codecov Report
Merging #74 into master will not change coverage.
The diff coverage isn/a.
@@ Coverage Diff @@ ## master #74 +/- ## ===================================== Coverage 100% 100% ===================================== Files 1 1 Lines 61 61 Branches 10 10 ===================================== Hits 61 61
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update b98df9b...ce7b824. Read the comment docs.
Thank you very much for this!
I think we should wait before merging this since there are several substantial changes incoming (such as #75 and the DOM Testing Library version bumping).
Moreover, I just noticed this comment regarding typings and its maintainability. I have little to no experience with TypeScript, so I wouldn't be able to keep typings up to date. What are your thoughts about the proposed solution by Kent?
No problem. Yeah, we could put this on DefinitelyTyped if that's the direction that dom-testing-library is going. It doesn't change the maintainability situation, but it does mean you won't be responsible for fielding issues for it.
For what it's worth, since these definitions only express the interface, changes like #75 don't impact them much. The only relevant change I can see is that the main fireEvent() function now returns a promise. These types don't use many advanced TypeScript features, but the ones for dom-testing-library do (BoundFunction being one), so I can see how that might make things harder.
For reference, typings from DTL and RTL are moving to DefinitelyTyped:
testing-library/dom-testing-library#337
testing-library/react-testing-library#437
The TypeScript team recommends that packages not written in TypeScript have their types in DefinitelyTyped (@types on npm) instead of in the same package: https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html
You should be able to reuse most of this if you'd like to make a pull request to DefinitelyTyped, though they have some additional tooling and code style requirements.
I'll submit the DefinitelyTyped PR once the dom-testing-library typings get moved there. (Otherwise it's a whole process to add @testing-library/dom to a whitelist, only to then remove it.) I'll close this and add a comment when I get that PR submitted.
Thanks @cimbul ❤️
Followed recommendations in dtslint tool and TypeScript documentation.
Took tsconfig.json and tslint.json from @testing-library/dom, although it looks like they just copied the examples in the dtslint docs.
Closes #60.