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

Click, keyboard & type TypeScript types are resolved as "any" because of wrong imports in typings #594

Closed
mkapal opened this issue Mar 18, 2021 · 12 comments · Fixed by #596 or #599
Closed
Labels
bug Something isn't working released

Comments

@mkapal
Copy link

mkapal commented Mar 18, 2021

  • @testing-library/dom version: 7.30.0
  • @testing-library/user-event version: 13.0.1
  • typescript version: 4.2.3

Problem description:
The file typings/index.d.ts contains type imports from ../src/ folder, which does not exist in the distributed package, so all those types are resolved to any.

Codesandbox: https://codesandbox.io/s/romantic-shadow-uio28

image

typings/index.d.ts in the corresponding node_modules folder:

image

This concerns the following types:

  • click, clickOptions
  • keyboard, keyboardOptions, keyboardKey,
  • type, typeOptions
@ph-fritsche ph-fritsche added the bug Something isn't working label Mar 18, 2021
@ph-fritsche
Copy link
Member

Might be that removing the typings folder and adding declarations of the JS modules in src is easier than to get rollup(?) to resolve these. It's for sure better than to continue maintaining the seperated typings.

@mkapal
Copy link
Author

mkapal commented Mar 19, 2021

Maybe it would be better to do it the same way as other @testing-library packages do it - to have all type declaration files in the types folder with no imports from src. You can see it in the dom-testing-library for example.

I can try fixing it if you agree.

@ph-fritsche
Copy link
Member

If you fix the typings, I would of course merge it as a fix for now. 🚀

The seperate typings are necessary because the libraries have been written in JS. I converted a part of this one to TS and planning to convert the rest. So seperate definition files for already converted modules are duplicates that we have to update and review by hand.

The cleaner solution would be to convert src/index.js and add missing module definitions in src.
This means we could drop the typings folder.

Eventually the solution has to be to convert the remaining JS modules to discover the false type assumptions that might be hidden in the code.

@github-actions
Copy link

🎉 This issue has been resolved in version 13.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ph-fritsche
Copy link
Member

@mkapal Thanks a lot for the reports. Please check out the new version. 😃

@ph-fritsche
Copy link
Member

@all-contributors add @mkapal bug

@allcontributors
Copy link
Contributor

@ph-fritsche

I've put up a pull request to add @mkapal! 🎉

@mkapal
Copy link
Author

mkapal commented Mar 19, 2021

@mkapal Thanks a lot for the reports. Please check out the new version. 😃

Unfortunately, it is still not correct. Now the distributed .d.ts files contain any types, and also, there are .d.js files generated, which I'm guessing shouldn't be there.

For example, the bundled click.d.ts file looks like this:

export function click(element: any, init: any, { skipHover, clickCount }?: {
    skipHover?: boolean | undefined;
    clickCount?: number | undefined;
}): void;
export function dblClick(element: any, init: any): void;

@ph-fritsche
Copy link
Member

See #596 (comment)
.d.js won't hurt

Rollup removed the references to lib.dom.d.ts. I'm on it.

@ph-fritsche
Copy link
Member

@mkapal Could you have a look at #599 ?

@github-actions
Copy link

🎉 This issue has been resolved in version 13.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mkapal
Copy link
Author

mkapal commented Mar 20, 2021

@mkapal Could you have a look at #599 ?

It works great now, thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
2 participants