fireEvent.keyPress not fire in test

[zhaozhiming]

• react-testing-library version: 5.4.4
• react version: 16.5.2
• node version: 10.0.0
• npm (or yarn) version: yarn 1.9.2

Relevant code or config:

  test("input key press correctly", () => { // failed
    const doSomething = jest.fn();
    const doOtherThing = jest.fn();
    const { container } = render(
      <App doSomething={doSomething} doOtherThing={doOtherThing} />
    );
    const input = getByTestId(container, "input");
    fireEvent.keyPress(input, { key: "Enter", code: 13 });
    expect(doSomething.mock.calls.length).toBe(1);
  });

What you did:

run test

What happened:

test failed

Reproduction:

https://codesandbox.io/s/jrv9v845v

Problem description:

When use fireEvent.keyPress, the input keyPress event not fired.
I found the doc about this part of the content, but I don't know what different between the keyPress event and change event.

Suggested solution:

I have no idea.

[baruchvlz]

I decided to dig into this a little bit. Adding input.addEventListener('keypress', console.log); shows that the event is indeed dispatched but for some reason React isn't calling the function.

This is as far as I got, I'm no React guru so I wouldn't know where to go from here.

[puemos]

Ok so i found out what is the issue:

TL;DR

https://codesandbox.io/s/n4p6z5o270

when you fire an keypress event you have to set the charCode

const input = getByTestId(container, "input");
fireEvent.keyPress(input, { key: "Enter", code: 13, charCode: 13 });
expect(doSomething.mock.calls.length).toBe(1);

Why? here:

switch (topLevelType) {
    case DOMTopLevelEventTypes.TOP_KEY_PRESS:
        // Firefox creates a keypress event for function keys too. This removes
        // the unwanted keypress events. Enter is however both printable and
        // non-printable. One would expect Tab to be as well (but it isn't).
        if (getEventCharCode(nativeEvent) === 0) {
            return null;
        }
}

https://github.com/facebook/react/blob/b87aabdfe1b7461e7331abb3601d9e6bb27544bc/packages/react-dom/src/events/SimpleEventPlugin.js#L226

/**
 * `charCode` represents the actual "character code" and is safe to use with
 * `String.fromCharCode`. As such, only keys that correspond to printable
 * characters produce a valid `charCode`, the only exception to this is Enter.
 * The Tab-key is considered non-printable and does not have a `charCode`,
 * presumably because it does not produce a tab-character in browsers.
 *
 * @param {object} nativeEvent Native browser event.
 * @return {number} Normalized `charCode` property.
 */
function getEventCharCode(nativeEvent) {
  var charCode = void 0;
  var keyCode = nativeEvent.keyCode;

  if ('charCode' in nativeEvent) {
    charCode = nativeEvent.charCode;

    // FF does not set `charCode` for the Enter-key, check against `keyCode`.
    if (charCode === 0 && keyCode === 13) {
      charCode = 13;
    }
  } else {
    // IE8 does not implement `charCode`, but `keyCode` has the correct value.
    charCode = keyCode;
  }

  // IE and Edge (on Windows) and Chrome / Safari (on Windows and Linux)
  // report Enter as charCode 10 when ctrl is pressed.
  if (charCode === 10) {
    charCode = 13;
  }

  // Some non-printable keys are reported in `charCode`/`keyCode`, discard them.
  // Must not discard the (non-)printable Enter-key.
  if (charCode >= 32 || charCode === 13) {
    return charCode;
  }

  return 0;
}

https://github.com/facebook/react/blob/b87aabdfe1b7461e7331abb3601d9e6bb27544bc/packages/react-dom/src/events/getEventCharCode.js

[kentcdodds]

Awesome! Would anyone be willing to add a note about this in the docs?

[puemos]

I'm on it 😎

[puemos]

@kentcdodds maybe we should set the defaults for the KeyboardEvent to include charCode: 0 as the browser does.

*Try to create an new KeyboardEvent()

[baruchvlz]

Maybe have a fallback in the library where if the user has code and not charCode then set charCode to the value of code.

if (init.code && init.charCode === undefined) {
  init.charCode = init.code;
}

I feel this is too much coding for failure, but I still wanted to suggest it.

[kentcdodds]

Ah, good ideas.

I think let's start with the suggestion from @puemos. Not sure how I feel about @baruchvlz's idea. I worry it could lead to confusion and potentially hide problems 🤔

[baruchvlz]

@kentcdodds Yea, I agree that it could lead to confusion. It also feels like it's trying to solve one specific scenario which doesn't go at-par with the library. In any case, I still wanted to suggest it 😃

[kentcdodds]

This fix by @puemos will be released momentarily! Thanks!