Dom-testing-library: Baking-in `closest()`

[will-stone]

Hi Kent! Thanks so much for this and RTL, it's made testing really quite easy 😀

Describe the feature you'd like:

If I have this...

<button>
  <span>testing</span>
</button>

...I have found myself having to use .closest() when trying to find the button, where the span isn't guaranteed to exist, and this works really well.

getByText('testing').closest('button')

However, I thought this would be what the selector option was supposed to provide but that didn't seem to work for me. I get:

Unable to find an element with the text: testing. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

Therefore I'm not really sure if this is a bug report or feature request. If...

getByText('testing', { selector: 'button'})

... should find the button, then this is a bug report. If it's not, then maybe...

getByText('testing', { closest: 'button'})

...could be useful and some docs to explain this situation? Or maybe this is all overkill, what are your thoughts?

Happy to help, just want to know if I'm barking up the correct tree to begin with.

[kentcdodds]

Hi @will-stone!

After looking into this a little bit I _think_ I want to change the behavior of getNodeText when you specify a selector other than * (the default).

(side-note, I just realized that closest isn't supported in jsdom yet which is a real surprise. I'm sure they'd welcome a PR, and it seems reasonably simple to implement).

I think I want to make this possible:

import 'jest-dom/extend-expect'
import {render} from './helpers/test-utils'

test('selector can be provided', () => {
  const {getByText} = render(
    '<button data-testid="button"><span>hello</span></button>',
  )
  const button = getByText('hello', {selector: 'button'})
  expect(button).toHaveAttribute('data-testid', 'button')
})

The reason this doesn't work currently is because the button doesn't actually have any direct children which are text nodes. So when you specify the selector of button, then this query returns an array with only the button node and because it doesn't contain the direct children with that text it doesn't find any node with that text and throws the error.

The reason things are this way currently is because if we were to just simply accept any node that contains the given text content the document.body would always match which is not what we want, so we limit it to only the nodes that have the text you're specifically looking for.

The reason I don't want support for a closest as you suggest is it means that the tests will contain the implementation detail that the user couldn't be aware of (whether the button has a span in it or not in this example).

So, what I'm thinking is if you specify a selector then we can be more certain that you can scope things down to exactly the node that you want (specifically the desire would be to select a <button> or <a> etc.) and that way we could just look at textContent rather than only the direct children.

---

Thinking about this further though...... Maybe _that_ is also an implementation detail that the user doesn't care about. All the user cares about is that the thing is focusable/clickable. So perhaps what would be even better is something like this:

import 'jest-dom/extend-expect'
import {render} from './helpers/test-utils'

test('selector can be provided', () => {
  const {getByText} = render(
    '<button data-testid="my-button"><span>hello</span></button>',
  )
  const button = getByText('hello', {clickable: true}) // or maybe {closest: 'clickable'} ?
  expect(button).toHaveAttribute('data-testid', 'my-button')
})

With that, we'd find the node with the text the normal way, but then we'd look for the closest focusable/clickable parent element. We'd have to look up the semantics of this, but I _think_ it'd be any button, a, form control (?), or element with a tabindex.

What do you think? I'm not 100% certain I like this solution.

[kentcdodds]

Maybe instead we should have a new query called getClickableByText?

[sompylasar]

I have some related food for thought in Cypress utils. Sorry for abrupt snippets, cannot share more now.

// in cy.assertElementFoundByExactText
// `chain` is `cy` from Cypress, but that's irrelevant

    if (typeof textOrRegexp === 'string') {
      // Using the `should` assertion to make it show up in the log.
      // `have.text` shows the text in the log so it's preferred if the text is a string.
      chain = chain.getByText(makeExactMatchRegexp(textOrRegexp)).should('have.text', textOrRegexp);
    }

// in cy.getVisuallyBelow

    const candidateMatcher = (
      typeof matcher === 'function'
        ? (candidate) => matcher(candidate.el)
        : (candidate) => domTestingLibraryTextMatches(getNodeFullText(candidate.el), candidate.el, matcher)
    );

// in cy.formFindNextButton

  const isButton = (el) => (el.nodeName === 'BUTTON');
  const innerTextMatches = (el) => (getNodeFullText(el) === text);
  const matcher = (el) => isButton(el) && innerTextMatches(el);
  matcher.toString = () => 'isButton && innerTextMatches(' + text + ')';

// ...
    chain
      .getVisuallyBelow(matcher, { timeout, log: false })

// in cy.formFindNextTextInput

  const isInput = makeInputTagMatcher();
  const matcher = (el) => isInput(el) && (filter ? filter(el) : true);
  matcher.toString = () => 'isInput' + (filter ? ' && ' + filter.toString() : '');

// ...
    chain
      .getVisuallyBelow(matcher, { timeout, log: false })

// in cy.formFindNextLabel

// ...
    chain
      .getVisuallyBelow(makeExactMatchRegexp(label), { timeout, log: false })

// getNodeFullText renders a deep text content with normalized whitespace

export function getNodeFullText(node) {
  return Array.from(node.childNodes)
    .filter((child) => (
      child.nodeType === Node.TEXT_NODE || child.nodeType === Node.ELEMENT_NODE
    ))
    .map((child) => (
      child.nodeType === Node.TEXT_NODE
        ? child.textContent
        : getNodeFullText(child)
    ))
    .filter(Boolean)
    .join(' ')
    .trim()
    .replace(/\s+/g, ' ');
}

// Matcher factories

function escapeForRegexp(text) {
  return text.replace(/[.?*+[\]{}()$^]/g, (specialChar) => ('\\' + specialChar));
}

export function makeExactMatchRegexp(text) {
  if (text instanceof RegExp) {
    return new RegExp(text.source.replace(/^\^*/, '^').replace(/\$*$/, '$'));
  }
  return new RegExp('^' + escapeForRegexp(text) + '$');
}

export function makeStartsWithMatchRegexp(text) {
  if (text instanceof RegExp) {
    return new RegExp(text.source.replace(/^\^*/, '^'));
  }
  return new RegExp('^' + escapeForRegexp(text));
}

export function makeInputTagMatcher() {
  return (el) => el.nodeName === 'INPUT';
}

[skbolton]

@kentcdodds there is an active pr for closest in jsdom
https://github.com/jsdom/jsdom/pull/1951

[will-stone]

@kentcdodds yes, I see the conundrum here; we don't want the to search-up through the tree because we could end up with false positives if a parent is also clickable.

To give you some more context, the reason this came about for me was because I was trying to assert the disabled status on the button. Which is when I noticed that the UI library I use, wrapped the inner text of the button with a span and it was this that I was now pointing to.

[will-stone]

Ah! I realise what I'm doing wrong. I should have been following the spirit of the library and testing for what the user expects to happen not what attributes are on the button. I went from this:

...
const { getByText } = wrapper;
const continueButton = getByText('Continue').closest('button');
expect(continueButton).toHaveAttribute('disabled');

to:

...
const { getByText } = wrapper;
const continueButton = getByText('Continue');
fireEvent.click(continueButton);
expect(onContinueClick).toHaveBeenCalledTimes(0);

Also, I've noticed that wrapping the inner text of the button with a span, when the button is a submit button in a form doesn't work. e.g.

Works:

<form onSubmit={onContinueClick}>
  <button type="submit">Continue</button>
</form>

Works:

<form onSubmit={onContinueClick}>
  <button type="submit" onClick={onContinueClick}>Continue</button>
</form>

Works:

<form onSubmit={onContinueClick}>
  <button type="submit" onSubmit={onContinueClick}><span>Continue</span></button>
</form>

Doesn't work:

<form onSubmit={onContinueClick}>
  <button type="submit"><span>Continue</span></button>
</form>

[sompylasar]

Also, I've noticed that wrapping the inner text of the button with a span, when the button is a submit button in a form doesn't work. e.g.

This looks like a bug or a missing feature in the click-to-submit simulation. The case when it doesn't work is when the form submit is triggered automagically by the browser when a button type=submit click occurs.

[sompylasar]

I found this which was closed as fixed 2 years ago: https://github.com/jsdom/jsdom/issues/1026

[gnapse]

@will-stone is it by any chance material-ui the library you're referring to? Because I'm having this issue too right now, and it's with it. I'm interested to see how this will be solved, and I'm more than happy to help making it possible.

[will-stone]

@gnapse With the span in a button? It was with BlueprintJS that I first noticed the problem but it'll affect any spans in buttons. As for the form submit issue, just put the onSubmit function on the button's onClick too:

<form onSubmit={onContinueClick}>
  <button type="submit" onClick={onContinueClick}><span>Continue</span></button>
</form>

This is probably just a workaround though, until the clever chaps here have found a way to fix the library to fire the form's onSubmit when clicking a span in a type="submit" button.

[noinkling]

Just an update on the .closest() tangent: it's now supported in JSDOM as of v11.12.0

[kentcdodds]

Fantastic news! Thank you @noinkling

[kentcdodds]

This is no longer actionable, so I'm going to close this one.