Skip to content

findBy* & findAllBy* have wrong argument types for TS #534

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
denysdovhan opened this issue Apr 24, 2020 · 7 comments · Fixed by #574
Closed

findBy* & findAllBy* have wrong argument types for TS #534

denysdovhan opened this issue Apr 24, 2020 · 7 comments · Fixed by #574
Labels
bug Something isn't working help wanted Extra attention is needed released TypeScript

Comments

@denysdovhan
Copy link

denysdovhan commented Apr 24, 2020

  • DOM Testing Library version: 7.2.0
  • node version: 12.13.0
  • yarn version: 1.22

Relevant code or config:

export const queryAllByAttribute = (
    container: HTMLElement,
    attribute: string,
    value: Matcher,
    options?: MatcherOptions
) => queryHelpers.queryAllByAttribute(attribute, container, value, options);

const includesAutomationId = (content: string, automationId: string) =>
    content.split(/\s+/).some(id => id === automationId);

export const queryAllByAutomationId = (
    container: HTMLElement,
    automationId: string | string[],
    options?: MatcherOptions
) =>
    queryAllByAttribute(
        container,
        TEST_ID_ATTRIBUTE,
        content => Array.isArray(automationId) ?
            automationId.every(id => includesAutomationId(content, id)) :
            includesAutomationId(content, automationId)
        ,
        options
    );

export const [
    queryByAutomationId,
    getAllByAutomationId,
    getByAutomationId,
    findAllByAutomationId,
    findByAutomationId
] = buildQueries(queryAllByAutomationId, getMultipleError, getMissingError);

const waitForOptions = { timeout: 2000 }

findByAutomationId(root, testId, options, waitForOptions), // <-  Expected 2-3 arguments, but got 4.ts(2554)
findAllByAutomationId(root, testId, options, waitForOptions), // <-  Expected 2-3 arguments, but got 4.ts(2554)

What you did:

I generated custom findBy and findAllBy queries by buildQueries.

What happened:

Generated queries have lost TypeScript types for 4th argument waitForOptions.

Problem description:

I've noticed all findBy*/findAllBy* methods have 4th param, for example:

export type FindAllByText = (
    container: HTMLElement,
    id: Matcher,
    options?: SelectorMatcherOptions,
    waitForElementOptions?: WaitForElementOptions,
) => Promise<HTMLElement[]>;

However, buildQueries returns FindBy and FindAllBy queries without that param:

export type QueryMethod<Arguments extends any[], Return> = (container: HTMLElement, ...args: Arguments) => Return;

export type FindAllBy<Arguments extends any[]> = QueryMethod<Arguments, Promise<HTMLElement[]>>;
export type FindBy<Arguments extends any[]> = QueryMethod<Arguments, Promise<HTMLElement>>;

Suggested solution:

Add 4th param type to the QueryMethod type or make special FindQueryMethod type with that param.

@denysdovhan denysdovhan changed the title findBy* & findAllBy* have wrong argument types findBy* & findAllBy* have wrong argument types for TS Apr 24, 2020
@Lagily
Copy link
Contributor

Lagily commented May 6, 2020

Can you add your definition of queryAllByAutomationId, so I can replicate this or create a sandbox where this behaviour is visible?

@denysdovhan
Copy link
Author

@Lagily I updated the original issue's text with definitions of that function.

@kentcdodds
Copy link
Member

We recently moved the types into this package and I think this was fixed. Please open a new issue if that's not the case.

Thank you!

@denysdovhan
Copy link
Author

@kentcdodds this wasn't. findBy and findAllBy queries have correct types. This issue relates to the queries generated with buildQueries.

@Lagily
Copy link
Contributor

Lagily commented May 12, 2020

@denysdovhan Do you think you could make a pull request that fixes this? I see the problem, but I am unable to find a way to do this in TypeScript currently, as the passed params need to be put in between the HtmlElement and waitFor params that are always returned

And yeah this is sadly not fixed, issue could be reopened

@kentcdodds kentcdodds reopened this May 12, 2020
@denysdovhan
Copy link
Author

denysdovhan commented May 13, 2020

@Lagily I'm not familiar with TS typings so well :(

@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 7.5.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed released TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants