Skip to content
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

Provide typescript definitions #11

Merged
merged 3 commits into from
May 15, 2018
Merged

Provide typescript definitions #11

merged 3 commits into from
May 15, 2018

Conversation

gnapse
Copy link
Member

@gnapse gnapse commented May 3, 2018

What:

Extend jest matchers interface with typescript definitions for custom matchers provided by this library. This is a follow up on the work done in #10.

Why:

So users using this library in typescript projects don't need to deal with declaring and keeping updated the types themselves.

How:

Added a index.d.ts file with the type definitions, and declared it as the types entry in the package.json file.

Checklist:

  • Removed documentation about users of this lib needing to keep the type definitions themselves.
  • Check that this actually works when importing the updated lib in typescript projects.
  • Ready to be merged

@codecov-io
Copy link

codecov-io commented May 3, 2018

Codecov Report

Merging #11 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #11   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines          41     41           
  Branches       12     12           
=====================================
  Hits           41     41

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 7cdb616...4e26fb7. Read the comment docs.

@InExtremaRes
Copy link

I am testing this with TypeScript 2.8.3 and Jest 22.1.4 (with ts-jest 22.0.1) but there is a little issue.

If I do:

import 'jest-dom/extend-expect';

it('to be in the dom', () => {
    const div = document.createElement('div');
    expect(div).toBeInTheDOM();
});

then things work at runtime as expected but TS compilation fails since toBeInTheDOM isn't defined.

If I do this instead:

import {} from 'jest-dom';
import 'jest-dom/extend-expect';

it('to be in the dom', () => {
    const div = document.createElement('div');
    expect(div).toBeInTheDOM();
});

then everything works fine.

I guess this happens since types declarations are in file index.d.ts so it just covers the "root module" jest-dom. I will try to do further experimentation.

@InExtremaRes
Copy link

Well, this seems related.

If I put the types definitions in the file jest-dom/extend-expect.d.ts next to the .js file (and not within the dist directory) then types work well with the code:

import 'jest-dom/extend-expect';

So I guess the solutions could be 1) to put a new file as I mentioned before, or 2) to generate that file at build stage. Option 1 should be simpler at the cost of polluting a little bit the root directory.

@gnapse
Copy link
Member Author

gnapse commented May 9, 2018

Thanks a lot @InExtremaRes! I think I'll go with the simpler alternative, first of all because it's what I'd know how to do right now, and also because it's not that big of a deal anyway to have this extra file in the root directory.

I'm a bit of a newbie when it comes to typescript, though I'd like to learn. And even more of a newbie when it comes to maintaining a node library like this one. That issue that you linked to in the typescript repo is very instructive about this. Thanks again.

@InExtremaRes
Copy link

Very glad to help even if it's just commenting.

@InExtremaRes
Copy link

InExtremaRes commented May 9, 2018

Oh, BTW it won't work for TS if just import some matchers and register those with expect as suggested in the readme. First of all the typings doesn't export the function signatures (it does just the matchers) so something like

import {toHaveClass} from 'jest-dom'

will file with some TS error: ... has no exported member 'toHaveClass'.

It is easy for us to add those types so the import is valid, but then the manual import and expect.extend() will not augment the jest namespace and TS will not recognize the matchers 😔. The user will have to take care of that.

I suspect it is possible to tweak the typings a little so every time a function is imported the namespace is augmented for that particular matcher, but that, even if possible, has some drawbacks too.

Maybe this could be address in a issue so it's easy to discuss different solutions and proposals. Nonetheless, my guess is that most users will import the whole module (even maybe in a global setup file) and this PR covers that very well. Perhaps a note in the readme could be enough for now.

@gnapse
Copy link
Member Author

gnapse commented May 9, 2018

Thanks for the further clarification. Yes, I think that what's provided here would be enough for now, and I'll add the note in the README for the moment, clarifying that typescript won't work as expected when importing matchers individually.

Please, feel free to open the issue you suggest, to keep this part of the discussion visible, and hopefully with the help of you or others it could be solved. I myself lack a more complete understanding of TS to even attempt to do it now.

@InExtremaRes
Copy link

What is the status of this? Is anything else needed before merging?

@gnapse
Copy link
Member Author

gnapse commented May 15, 2018

The note in the README about the caveat that you mentioned. I'll wrap it up today. Thanks for the reminder.

@gnapse gnapse merged commit bbdd96c into master May 15, 2018
@gnapse gnapse deleted the pr/typescript-definitions branch May 15, 2018 20:35
@gnapse
Copy link
Member Author

gnapse commented May 15, 2018

🎉 This PR is included in version 1.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Couto added a commit to yldio/conference-template that referenced this pull request Aug 28, 2019
- Add jest
- Configure jest to work with [TypeScript][1] and [Gatsby][2]
- Add [testing-library][3] to help with unit-testing
- Refine storybook's file-glob search to match the new filename
  conventions

In order to remove some noise, components will follow a new name
convention, where each component will be be accompanied by
`stories.mdx` and `test.tsx` files.

TypeScript definitions for the testing-library matchers [are not used by
the compiler][4], unless we manually import the matchers on each test,
instead of using the `setupAfterEnvFiles` property from Jest.

[1]: https://kulshekhar.github.io/ts-jest/user/config/
[2]: https://www.gatsbyjs.org/docs/unit-testing/
[3]: https://testing-library.com/docs/dom-testing-library/example-intro
[4]: testing-library/jest-dom#11 (comment)
Couto added a commit to yldio/conference-template that referenced this pull request Aug 28, 2019
- Add jest
- Configure jest to work with [TypeScript][1] and [Gatsby][2]
- Add [testing-library][3] to help with unit-testing
- Refine storybook's file-glob search to match the new filename
  conventions

In order to remove some noise, components will follow a new name
convention, where each component will be be accompanied by
`stories.mdx` and `test.tsx` files.

TypeScript definitions for the testing-library matchers [are not used by
the compiler][4], unless we manually import the matchers on each test,
instead of using the `setupAfterEnvFiles` property from Jest.

[1]: https://kulshekhar.github.io/ts-jest/user/config/
[2]: https://www.gatsbyjs.org/docs/unit-testing/
[3]: https://testing-library.com/docs/dom-testing-library/example-intro
[4]: testing-library/jest-dom#11 (comment)
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.

None yet

3 participants