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 466 forks source link

Collapse whitespace issue with   #883

Open herecydev opened 3 years ago

herecydev commented 3 years ago

Had an issue that stemmed from Cypress testing library but debugged into Dom testing library. The issue is this function.

When searching for text with a non breaking space such as "٤ UK£", the regex replaces the nbsp into a space. It's fairly obvious why the regex was used, but it's effectively converting all types of spaces into a "regular space". This then causes the text match to fail.

In my opinion, that's an unintentional side effect of collapseWhitespace.

eps1lon commented 3 years ago

By default, normalization consists of trimming whitespace from the start and end of text, and collapsing multiple adjacent whitespace characters into a single space.

-- https://testing-library.com/docs/queries/about/#normalization

The documentation matches the implementation as far as I can tell. Did you consider passing your own normalizer as documented in https://testing-library.com/docs/queries/about/#normalization-examples?

herecydev commented 3 years ago

I absoutely can try, there doesn't seem to be a way to configure this globally though (I have to do this in many places). Is that something that you could see being added?

I'm not sure I agree with the docs here:

By default, normalization consists of trimming whitespace from the start and end of text, and collapsing multiple adjacent whitespace characters into a single space.

Consider the following " a b ". I would expect the first and last spaces to be removed but there are no "multiple adjacent whitespace characters". So the inner space (non breaking) should have been preserved.

I'm using the Intl.NumberFormat in my application which is outputting that nbsp, and then in Cypress I'm querying for that text which is highlighting that mismatch. Would the default normalizer benefit from collapsing multiple spaces but preserving the "type of space"?

Alternatively, should the same normalization be applied to the textContent?

astorije commented 3 years ago

I'm getting a similar issue with non-breaking spaces and I can't seem to find a way to target them easily. This is what I have tried:

test('non-breaking space', () => {
  render(<div>&nbsp;</div>);

  // None of the following pass:
  expect(screen.getByText(' ')).toBeInTheDocument(); // Regular space
  expect(screen.getByText(' ')).toBeInTheDocument(); // Non-breaking space
  expect(screen.getByText(/ /)).toBeInTheDocument(); // Regular space
  expect(screen.getByText(/ /)).toBeInTheDocument(); // Non-breaking space
  expect(screen.getByText(/\u00a0/)).toBeInTheDocument();
  expect(screen.getByText('&nbsp;')).toBeInTheDocument();
  expect(screen.getByText(/&nbsp;/)).toBeInTheDocument();
});

I have not tried using a custom text matcher function or a custom normalizer, but I feel like there is probably an easy built-in way to do this without going too crazy on customizations. Any ideas?

nejcjelovcan commented 3 years ago

Encountered a similar issue when migrating to Intl.NumberFormat for the purposes of currency formatting, which uses non-breaking space (in certain locales) to separate the number from currency code/symbol.

Just to expand on the case given by @astorije:

test('text with non-breaking space', () => {
    render(<div>1,000.00&nbsp;EUR</div>)

    // assertion passes with regular space
    expect(screen.getByText('1,000.00 EUR')).toBeInTheDocument()

    // assertion fails with non-breaking space
    expect(screen.getByText('1,000.00 EUR')).toBeInTheDocument()
})

Passing a custom function to getByText allows for actually checking if the space is non-breaking.

AndreAffonso commented 3 years ago

Same Problem when using Intl.NumberFormat.

ciceropablo commented 2 years ago

Any news about it?

danny-does-stuff commented 2 years ago

This remains an issue. If you want to have two words stay on the same line as each other, for example in the case of @nejcjelovcan, we should still be able to select that using a query

herecydev commented 2 years ago

@eps1lon what are your thoughts? I've responded and it doesn't feel like this is getting any love at all

paul-pro commented 2 years ago

If someone would find this issue and unlike @herecydev they would be happy with a local override, property that @eps1lon mentioned should do the trick:

import {screen, getDefaultNormalizer} from '@testing-library/react'

screen.getByText(textWithNbsp, {
  normalizer: getDefaultNormalizer({collapseWhitespace: false}),
})

or even less verbose, but for some reason discouraged (?) outdated API:

import {screen, getDefaultNormalizer} from '@testing-library/react'

screen.getByText(textWithNbsp, {
  collapseWhitespace: false,
})
paul-pro commented 2 years ago

@eps1lon, I would agree with @herecydev @astorije and @nejcjelovcan that default normalizer does quite unexpected thing completely replacing any combination of whitespaces with a single space character.

Deduplication regexp replace like https://regex101.com/r/8rGdqm/4 would work more predictably from this perspective (despite breaking some existing library tests)

normalizedText.replace(/(\s)\1+/g, '$1')
image