Skip to content

(TS) Type SelectorMatcherOptions is missing timeout #43

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

Closed
ShimiTheFirst opened this issue May 30, 2019 · 7 comments
Closed

(TS) Type SelectorMatcherOptions is missing timeout #43

ShimiTheFirst opened this issue May 30, 2019 · 7 comments

Comments

@ShimiTheFirst
Copy link

  • cypress-testing-library version: 3.0.1
  • node version: 10.13.0
  • yarn version: 1.13.0

Relevant code or config

cy.queryByText("foo", { timeout: 100 })

What you did:
Tried to change the value of the timeout option on queryByText.

What happened:
The test correctly waited for the set amount of time (100ms) but TS validation reports an error:
ctl-type-err

Problem description:
Queries using the SelectorMatcherOptions type are missing the type definition for the timeout option.

Suggested solution:
If this is not intentional, take the same approach as for MatcherOptions in #28 – update typings/index.d.ts.

How it is now:

import {
  SelectorMatcherOptions,
  Matcher,
  MatcherOptions as DTLMatcherOptions,
  getByTestId,
} from 'dom-testing-library'

export interface CTLMatcherOptions {
  timeout?: number
}

export type MatcherOptions = DTLMatcherOptions | CTLMatcherOptions

How it could be:


import {
  SelectorMatcherOptions as DTLSelectorMatcherOptions,
  Matcher,
  MatcherOptions as DTLMatcherOptions,
  getByTestId,
} from 'dom-testing-library'


export interface CTLMatcherOptions {
  timeout?: number
}

export type MatcherOptions = DTLMatcherOptions | CTLMatcherOptions
export type SelectorMatcherOptions = DTLSelectorMatcherOptions | CTLMatcherOptions
@kentcdodds
Copy link
Member

Thanks for this. Could you make a pull request for this?

@ShimiTheFirst
Copy link
Author

I could try. It would be my very first PR though. I'll try to follow your course on egghead and contributing guide and see how far can I make it 🙂.

@kentcdodds
Copy link
Member

Please note that I'm not currently using the TypeScript types in any project so I'll need another reviewer to review/merge these changes. Thanks!

@ppi-buck
Copy link
Collaborator

ppi-buck commented Jun 3, 2019

@ShimiTheFirst your solution looks fine to me. Just did just that locally and it works.

For a simple change like this I would simply use the GitHub web frontend to create a PR. In fact I did just that #48

kentcdodds pushed a commit that referenced this issue Jun 3, 2019
Simply created a PR based on the changes ShimiTheFirst suggested here #43
@kentcdodds
Copy link
Member

@all-contributors please add @ShimiTheFirst for bugs

@allcontributors
Copy link
Contributor

@kentcdodds

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

@kentcdodds
Copy link
Member

Thanks to both of you for this!

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

No branches or pull requests

3 participants