[@testing-library/cypress] New type definition

[simjes]

This is related to testing-library/cypress-testing-library#68, moving typings to DefinitelyTyped.

A couple of questions:

• Is the import of jquery for typings correct in index.d.ts? I was thinking I would need to add it to paths in tsconfig.json, but npm run test will fail with In JQuery, unexpected path mapping for JQuery: 'jquery'.
• Any suggestions to improve the typings to get rid of unnecessary generics? I've currently disabled the rule in linting, but I see this is located under common mistakes

Any other feedback to improve this is appreciated

---

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change. (Run with npm test.)
• Follow the advice from the readme.
• Avoid common mistakes. See question
• Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If adding a new definition:

• The package does not already provide its own types, or cannot have its .d.ts files generated via --declaration
• If this is for an NPM package, match the name. If not, do not conflict with the name of an NPM package.
• Create it with dts-gen --dt, not by basing it on an existing project. Inspired by other typings for testing-library__*
• tslint.json should be present, and tsconfig.json should have noImplicitAny, noImplicitThis, strictNullChecks, and strictFunctionTypes set to true.

[typescript-bot]

@simjes Thank you for submitting this PR!

Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

[typescript-bot]

👋 Hi there! I’ve run some quick performance metrics against master and your PR. This is still an experiment, so don’t panic if I say something crazy! I’m still learning how to interpret these metrics.

Let’s review the numbers, shall we?

These typings are for a package that doesn’t yet exist on master, so I don’t have anything to compare against yet! In the future, I’ll be able to compare PRs to testing-library__cypress with its source on master.

Comparison details 📊

Batch compilation
Type count	16922
Assignability cache size	38544
Subtype cache size	0
Identity cache size	16
Language service measurements
Samples taken	73
Identifiers in tests	73
getCompletionsAtPosition
Mean duration (ms)	593.5
Median duration (ms)	584.4
Mean CV	13.7%
Worst duration (ms)	725.2
Worst identifier	queryAllByAltText
getQuickInfoAtPosition
Mean duration (ms)	591.8
Median duration (ms)	584.6
Mean CV	13.1%
Worst duration (ms)	728.7
Worst identifier	queryAllByTestId
System information
Node version	v10.16.1
CPU count	2
CPU speed	2.294 GHz
CPU model	Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz
CPU Architecture	x64
Memory	6.8 GiB
Platform	linux
Release	4.15.0-1052-azure

---

If you have any questions or comments about me, you can ping @andrewbranch. Have a nice day!

[weyert]

@johnnyreilly Could you have a look at it? :)

[wKovacs64]

@simjes do you have an example project of these working? I'm running into JQuery conflicts and I think it's because these new types import the latest version of @types/jquery, but Cypress ships its own version, maybe?

[simjes]

@wKovacs64 check out https://github.com/simjes/cypress-testing-library-example. Had a question on the PR about the JQuery typings but it was merged without any feedback, so I assumed it was correct.

[wKovacs64]

@simjes I ran npx typescript --noEmit --project cypress in your example repo and got 56 errors (the same JQuery conflicts I got in my own project). Does it come back clean for you?

If it is indeed a @types/jquery version conflict, I'm not sure of the best way to handle that. We could change the dependency version in @types/testing-library__cypress from * to the version Cypress is using (currently 3.3.6) but that means every time Cypress updates theirs, this would need updated as well (and maybe that's OK). A better solution might be to import JQuery from cypress/types/jquery instead of from @types/jquery but I have no idea if you can do that in DefinitelyTyped (edit: and it would probably require Cypress export it as they don't currently).

[simjes]

My bad, getting the same errors. I'll look into it, any tips on how to best solve this @johnnyreilly?

[wKovacs64]

Not sure who else to ping on something like this. Maybe we should specify the JQuery version for now, so at least the migration path is possible? 🤔

[sheetalkamat]

Please open new PR removing all the rules here and fix errors. We do not want new definition with custom rules. If the rule is absolutely necessary disable it at the location, instead of for whole package, so we can review correctness of it.

[sheetalkamat]

This needs to be fixed asap. There is no default being exported here..

// a.js
const x = require("@testing-library/cypress");
console.log(x.default);

> node a.js
undefined

[sheetalkamat]

@simjes We want you to create a PR fixing the additional comments that are our principle guidelines for PRs.
cc: @johnnyreilly Please note that we would want to hold back the PRs that dont meet the principle guidelines.

[sheetalkamat]

Also for the issue you are facing please try using cypress as the dependency in the types to see if that helps with the build errors

[simjes]

@sheetalkamat thanks for the feedback, I will start fixing.

[johnnyreilly]

cc: @johnnyreilly Please note that we would want to hold back the PRs that dont meet the principle guidelines.

That totally makes sense @sheetalkamat. Just it's not clear to me which principle guidelines for PRs that are being breached by this PR?

I've looked at the guidance offered here:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/.github/PULL_REQUEST_TEMPLATE.md

And here:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/README.md#create-a-new-package

But the points mentioned seem to be different to the ones you've been referencing. Have I missed something, or do these guidelines need updating with extra advice?

[wKovacs64]

Also for the issue you are facing please try using cypress as the dependency in the types to see if that helps with the build errors

Oh, depend on cypress itself instead of jquery? Interesting, hopefully that works! 🤞

[simjes]

@sheetalkamat I've made some progress on this, but I'm getting typings error from cypress/jquery when running dtslint, as it uses typescript@next. If I run dtslint with the --localTs flag, it completes with no errors using typescript@3.5.3.

Do you have any suggestions? Maybe I should move this to stackoverflow or open a new issue instead?

I've got a repo here if you would like to take a look. I've temporarly added typescript as a dependency on testing-library__cypress to test dtslint --localTs

The error:

Error: Errors in typescript@next for external dependencies:
node_modules/cypress/types/jquery/index.d.ts(8155,87): error TS2344: Type '"button" | "view" | "altKey" | "bubbles" | "cancelable" | "changedTouches" | "ctrlKey" | "detail" | "eventPhase" | "metaKey" | "pageX" | "pageY" | "shiftKey" | "char" | "charCode" | ... 13 more ... | "touches"' does not satisfy the constraint '"repeat" | "button" | "code" | "view" | "y" | "altKey" | "bubbles" | "cancelable" | "changedTouches" | "ctrlKey" | "detail" | "eventPhase" | "metaKey" | "pageX" | "pageY" | "shiftKey" | ... 54 more ... | "DOM_KEY_LOCATION_STANDARD"'.
  Type '"toElement"' is not assignable to type '"repeat" | "button" | "code" | "view" | "y" | "altKey" | "bubbles" | "cancelable" | "changedTouches" | "ctrlKey" | "detail" | "eventPhase" | "metaKey" | "pageX" | "pageY" | "shiftKey" | ... 54 more ... | "DOM_KEY_LOCATION_STANDARD"'.

[wKovacs64]

@simjes That error is a Cypress issue and I think they need to update the version of @types/jquery they are bundling. See cypress-io/cypress#5065. If you have a fix in place for the original jQuery conflicts and this is the only hold-up, we can probably push it through?