testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.26k stars 467 forks source link

Buildqueries types do not allow passing in a 3rd argument for options #1145

Closed KonnorRogers closed 2 years ago

KonnorRogers commented 2 years ago

Relevant code or config:

// Label Text
function queryAllByShadowLabelText<T extends HTMLElement = HTMLElement>(container: HTMLElement, id: Matcher, options?: ShadowSelectorMatcherOptions | undefined): T[] {
  const elements = deepQuerySelectorAll(container, ":scope *")

  return elements.map((el) => queryAllByLabelText(el as HTMLElement, id, options)).flat(Infinity) as T[]
}

const getMultipleLabelTextError = (_c: Element | null, id: Matcher) =>
  `Found multiple elements with the label text of: ${id}`
const getMissingLabelTextError = (_c: Element | null, id: Matcher) =>
  `Unable to find an element with the label text of: ${id}`

const [
  queryByShadowLabelText,
  getAllByShadowLabelText,
  getByShadowLabelText,
  findAllByShadowLabelText,
  findByShadowLabelText,
] = buildQueries(queryAllByShadowLabelText, getMultipleLabelTextError, getMissingLabelTextError)

test('byShadowLabelText', async () => {
  const { container } = render(<TextArea />)

  // Types shipped by DOM-testing-library with buildQueries don't have options attached to them.
  // @ts-expect-error
  expect(await findByShadowLabelText(container, /omments/, {exact: false})).toBeInTheDocument()
})

What you did:

I used the buildQueries function and the types generated do not match the arguments the functions actually receive. The test above actually passes.

What happened:

2022-07-07

Reproduction:

(Will provide once I push up changes on the repo)

Problem description:

The types are not correct causing TS users to have to use // @ts-expect-error

Suggested solution:

buildQueries should match what the actual functions take in

KonnorRogers commented 2 years ago

I managed to workaround the type issue by typecasting to what are the correct arguments.

https://github.com/ParamagicDev/shadow-dom-testing-library/pull/1/files#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80

eps1lon commented 2 years ago

The problem is that buildQueries tries to infer the parameters from the given functions (queryAllByShadowLabelText, getMultipleLabelTextError, getMissingLabelTextError). But in your usage these functions don't have the same parameters so TypeScript infers the smallest type that satisfies all 3 functions: [id: Matcher].

In order to fix it you either have to explicitly pass the parameters or use the same signature for all functions.

Same signature:

function queryAllByShadowLabelText<T extends HTMLElement = HTMLElement>(container: HTMLElement, id: Matcher, options?: ShadowSelectorMatcherOptions | undefined): T[] {
  const elements = deepQuerySelectorAll(container, ":scope *")

  return elements.map((el) => queryAllByLabelText(el as HTMLElement, id, options)).flat(Infinity) as T[]
}

const getMultipleLabelTextError = (_c: Element | null, id: Matcher, options?: ShadowSelectorMatcherOptions | undefined) =>
  `Found multiple elements with the label text of: ${id}`
const getMissingLabelTextError = (_c: Element | null, id: Matcher, options?: ShadowSelectorMatcherOptions | undefined) =>
  `Unable to find an element with the label text of: ${id}`

Playground Link

Explicit parameter type:

] = buildQueries<[id: Matcher, options?: ShadowSelectorMatcherOptions | undefined]>(queryAllByShadowLabelText, getMultipleLabelTextError, getMissingLabelTextError)

Playground Link

KonnorRogers commented 2 years ago

@eps1lon ahhh this makes sense! thank you! Much appreciated!