userEvent.type doesn't work well when a value is formatted after the onChange is fired

[WretchedDade]

• @testing-library/user-event version: 12.0.2
• Testing Framework and version: jest (24.9.0 locally and whatever version CodeSandbox uses)
• DOM Environment: @testing-library/react's render function, so jsdom?

React component logic

export default function App() {
	const [rawPhone, setRawPhone] = useState('');
	const [formattedPhone, setFormattedPhone] = useState('');

	return (
		<div className="App">
			<form>
				<label htmlFor="raw-phone">Raw Phone: </label>
				<input id="raw-phone" type="text" value={rawPhone} onChange={event => setRawPhone(event.target.value)} />

				<label htmlFor="formatted-phone">Formatted Phone: </label>
				<input id="formatted-phone" type="text" value={formattedPhone} onChange={event => setFormattedPhone(formatPhone(event.target.value)) } />
			</form>
		</div>
	);
}

Format function

export function formatPhone(unformattedPhone?: string): string {
	if (unformattedPhone == null)  return '';

	// Remove all formatting from input
	let matchedValue = unformattedPhone.replace(/[^0-9]*/g, '').match(/\d*/g);

	const cleanedValue = matchedValue ? matchedValue.join('') : '';

	// group between areaCode, firstGroup, and secondGroup
	matchedValue = cleanedValue.match(/(\d{0,3})(\d{0,3})(\d{0,4})/);

	const [areaCode, firstGroup, secondGroup] = matchedValue ? matchedValue.slice(1) : ['', '', '']; 

	// initialize phoneNumber
	let phoneNumber = '';

	// begin with '(' after any digit input
	if (areaCode !== '') phoneNumber = `(${areaCode}`;

	// end area code with ')' if there are more than 3 digits (at least 1 digit in firstGroup)
	if (firstGroup !== '') phoneNumber = `${phoneNumber}) ${firstGroup}`;

	// add '-' if there are more than 6 digits (at least 1 digit in secondGroup)
	if (secondGroup !== '') phoneNumber = `${phoneNumber}-${secondGroup}`;

	return phoneNumber;
}

Tests

test('Raw phone should set the value to match excatly what is typed', async () => {
	render(<App />);
	userEvent.type(screen.getByLabelText(/Raw Phone/i), '1234567890');
	expect(screen.getByLabelText(/Raw Phone/i)).toHaveValue('1234567890');
});

test('Formatted phone should format the value as it is typed', async () => {
	render(<App />);
	userEvent.type(screen.getByLabelText(/Formatted Phone/i), '1234567890');
	expect(screen.getByLabelText(/Formatted Phone/i)).toHaveValue('(123) 456-7890');
});

What you did:

I have a field where we collect a phone number and format it as the user types. To do so, we format the value onChange before we call the setState function from the useState hook`.

What happened:

When attempting to test this by calling userEvent.type(..., '1234567890') the value was being set to (098) 765-4321 instead of (123) 456-7890.

Reproduction repository:

https://codesandbox.io/s/usereventtype-reverses-value-mgi2n?file=/src/App.spec.tsx

Problem description:

This was extremely confusing at first and led me to do a lot of digging in the source code of userEvent.type(...). Once I did so for a while I came across the idea that it may be an issue with the selected location returning to the beginning of the input. I think it may have something to do with the newValue being different than it expected.

Suggested solution:

I don't know that I have a suggested solution, as I don't have a complete grasp over the source code and the cause of the specific problem. However, I'm more than happy to help discuss options and take a crack at implementing a fix with some guidance.

[nickmccurdy]

Have you tried wrapping your expect calls in waitFor? setState updates are asynchronous in React so I believe you'd need to wait for them to update in your tests.

[WretchedDade]

That was my first thought but seeing as the test fails because the value is reversed rather than not having a value, i was able to rule that out.

[nickmccurdy]

Ah sorry, I think I misunderstood the issue. Have you tried paste? It writes the input all at once, so I wonder if that would avoid the bug.

[WretchedDade]

Paste did avoid the bug, but that would mean I'm not testing the same thing right? Or is it close enough to typing that its still a valid test for how the component responds to a user typing?

[kentcdodds]

I'm 90% certain that your test as written above should pass and this is a bug, probably related to: #346

Reproducing this would probably be a matter of adding another test here:

user-event/src/__tests__/type.js

Line 708 in f4eae9c

})

I'm not sure about the solution, but that's where I'd start first to know for certain that this is a user-event bug.

I don't have the bandwidth to work on this myself unfortunately.

[nickmccurdy]

To be clear, I wasn't suggesting to actually keep paste in your test. You're right that it would be different behavior, I was just interested in it for troubleshooting.

[WretchedDade]

I'll pull down the repo and see if I can figure out what's going on.

[WretchedDade]

After digging into this some, here is what I found:

The assumed new value and next location to insert text is calculated by the calculateNewValue function in order to call _dom.fireEvent.input(...). However, when this event is fired the value is formatted and changes the value of currentElement().value. Because of this, the newSelectionStart value is inaccurate to where I would expect the next insert location to be.

user-event/src/utils.js

Lines 95 to 135 in f4eae9c

	function calculateNewValue(newEntry, element) {
	const {selectionStart, selectionEnd, value} = element
	// can't use .maxLength property because of a jsdom bug:
	// https://github.com/jsdom/jsdom/issues/2927
	const maxLength = Number(element.getAttribute('maxlength') ?? -1)
	let newValue, newSelectionStart
	if (selectionStart === null) {
	// at the end of an input type that does not support selection ranges
	// https://github.com/testing-library/user-event/issues/316#issuecomment-639744793
	newValue = value + newEntry
	} else if (selectionStart === selectionEnd) {
	if (selectionStart === 0) {
	// at the beginning of the input
	newValue = newEntry + value
	} else if (selectionStart === value.length) {
	// at the end of the input
	newValue = value + newEntry
	} else {
	// in the middle of the input
	newValue =
	value.slice(0, selectionStart) + newEntry + value.slice(selectionEnd)
	}
	newSelectionStart = selectionStart + newEntry.length
	} else {
	// we have something selected
	const firstPart = value.slice(0, selectionStart) + newEntry
	newValue = firstPart + value.slice(selectionEnd)
	newSelectionStart = firstPart.length
	}
	if (maxLength < 0) {
	return {newValue, newSelectionStart}
	} else {
	return {
	newValue: newValue.slice(0, maxLength),
	newSelectionStart:
	newSelectionStart > maxLength ? maxLength : newSelectionStart,
	}
	}
	}

user-event/src/type.js

Lines 158 to 161 in f4eae9c

	fireEvent.input(currentElement(), {
	target: {value: newValue},
	...eventOverrides,
	})

I'm not proposing it as a change, but by forcing newSelectionStart to be currentElement().value.length my issue is resolved. Perhaps the selection calculation needs to be moved to after the input event is fired?

user-event/src/type.js

Line 163 in f4eae9c

setSelectionRange({newValue, newSelectionStart})

setSelectionRange({ newValue, newSelectionStart: currentElement().value.length});

Edit: I should note, the above change makes 7 tests fail, 4 of which are snapshots.

[nickmccurdy]

Hm, that's interesting. If the snapshot changes look okay to you feel free to start a PR and we can hopefully figure out the other failures.

[WretchedDade]

Well, the tests are failing because this prevents any scenario of inserting text in the middle of the input. I'll probably take a crack at implementing something that works for both scenarios sometime in the next couple days and open a PR if I get something working.

[WretchedDade]

The problem I ran into is I'm not sure how to convert my react test into one for this library (since its framework agnostic), so if anyone as ideas on that it would be appreciated.

[nickmccurdy]

You're probably looking for the setup function from utils which will let you get a DOM element or list of events that you can snapshot, for example:

user-event/src/__tests__/paste.js

Lines 1 to 19 in f4eae9c

	import userEvent from '../'
	import {setup} from './helpers/utils'
	test('should paste text in input', () => {
	const {element, getEventSnapshot} = setup('<input />')
	const text = 'Hello, world!'
	userEvent.paste(element, text)
	expect(element).toHaveValue(text)
	expect(getEventSnapshot()).toMatchInlineSnapshot(`
	Events fired on: input[value="Hello, world!"]
	input[value=""] - focus
	input[value=""] - paste
	input[value="Hello, world!"] - input
	"{CURSOR}" -> "Hello, world!{CURSOR}"
	input[value="Hello, world!"] - select
	`)
	})

[WretchedDade]

Thanks, that put me in the right direction and I've got a test written locally now. I tried to replicate what I had done in my react test with formatting the value on change but it doesn't seem to be firing a change event. When I format the value on the input event, it works fine. So I'm stumped now.

The test I wrote in __tests__/type.js:

test('typing a value that is formatted on change', () => {
  function formatPhone(unformattedPhone) {
    if (unformattedPhone == null) return ''

    // Remove all formatting from input
    let matchedValue = unformattedPhone.replace(/[^0-9]*/g, '').match(/\d*/g)

    const cleanedValue = matchedValue ? matchedValue.join('') : ''

    // group between areaCode, firstGroup, and secondGroup
    matchedValue = cleanedValue.match(/(\d{0,3})(\d{0,3})(\d{0,4})/)
    const [areaCode, firstGroup, secondGroup] = matchedValue
      ? matchedValue.slice(1)
      : ['', '', '']

    // initialize phoneNumber
    let phoneNumber = ''

    // begin with '(' after any digit input
    if (areaCode !== '') phoneNumber = `(${areaCode}`

    // end area code with ')' if there are more than 3 digits (at least 1 digit in firstGroup)
    if (firstGroup !== '') phoneNumber = `${phoneNumber}) ${firstGroup}`

    // add '-' if there are more than 6 digits (at least 1 digit in secondGroup)
    if (secondGroup !== '') phoneNumber = `${phoneNumber}-${secondGroup}`

    return phoneNumber
  }

  const {element} = setup('<input />', {
    eventHandlers: {
      input: e => {
        console.log('Input event fired!')
        e.target.value = formatPhone(e.target.value)
      },
      change: e => {
        console.log('Change event fired!')
        e.target.value = formatPhone(e.target.value)
      },
    },
  })

  userEvent.type(element, '1234567890')

  expect(element).toHaveValue('(123) 456-7890')
})

[kentcdodds]

I'm not sure 😬 you may have to do a little more digging.

[WretchedDade]

So I did a little more digging and made a lot of changes. Which ultimately helped me understand the problem and reasons behind why the code was structured how it was. I then learned that the issue was ultimately how the browser responded to a programattic change and how userEvent.type responded to it.

I learned what the intentions were behind the following code and that the problem was if the value was changed programattically, the selection would never update from it's initial value.

user-event/src/type.js

Lines 64 to 70 in 44b9011

	if (currentValue() === newValue) {
	setSelectionRangeIfNecessary(
	currentElement(),
	newSelectionStart,
	newSelectionStart,
	)
	}

Therefore, I have opened a PR recommending updating it to:

user-event/src/type.js

Lines 64 to 77 in e6261e0

	const value = currentValue()
	if (value === newValue) {
	setSelectionRangeIfNecessary(
	currentElement(),
	newSelectionStart,
	newSelectionStart,
	)
	} else {
	// If the currentValue is different than the expected newValue and we *can*
	// change the selection range, than we should set it to the length of the
	// currentValue to ensure that the browser behavior is mimicked.
	setSelectionRangeIfNecessary(currentElement(), value.length, value.length)
	}

[kentcdodds]

🎉 This issue has been resolved in version 12.0.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[terraelise]

I am experiencing this issue with 13.5.0, had to downgrade to 12.0.4 to solve it.

[ph-fritsche]

@terraelise @mickcastle Please don't add noise to the issue board. File an issue with a reproducible example.

Also try upgrading to the latest version.
The issue above has been resolved. There might be another issue with 13.5.0 that might or might not be already resolved.