From e1423edeb0ba354f4eb2e4cdad31ae1916ce841b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rg=20Bayreuther?= <jb@visualjerk.de>
Date: Fri, 24 Jul 2020 22:46:42 +0200
Subject: [PATCH] fix(type): type on no value elements (#414)

Closes: #407
---
 src/__tests__/type-modifiers.js | 76 ++++++++++++++++++++++---
 src/__tests__/type.js           | 24 ++++----
 src/paste.js                    |  6 +-
 src/type.js                     | 99 +++++++++++----------------------
 4 files changed, 113 insertions(+), 92 deletions(-)

diff --git a/src/__tests__/type-modifiers.js b/src/__tests__/type-modifiers.js
index cf79b20e..17f318b8 100644
--- a/src/__tests__/type-modifiers.js
+++ b/src/__tests__/type-modifiers.js
@@ -443,6 +443,36 @@ test('{enter} on a button', () => {
   `)
 })
 
+test('{enter} with preventDefault on a button', () => {
+  const {element, getEventSnapshot} = setup('<button />', {
+    eventHandlers: {
+      keyDown: e => e.preventDefault(),
+    },
+  })
+
+  userEvent.type(element, '{enter}')
+
+  expect(getEventSnapshot()).toMatchInlineSnapshot(`
+    Events fired on: button
+
+    button - pointerover
+    button - pointerenter
+    button - mouseover: Left (0)
+    button - mouseenter: Left (0)
+    button - pointermove
+    button - mousemove: Left (0)
+    button - pointerdown
+    button - mousedown: Left (0)
+    button - focus
+    button - focusin
+    button - pointerup
+    button - mouseup: Left (0)
+    button - click: Left (0)
+    button - keydown: Enter (13)
+    button - keyup: Enter (13)
+  `)
+})
+
 test('{space} on a button', () => {
   const {element, getEventSnapshot} = setup('<button />')
 
@@ -471,6 +501,34 @@ test('{space} on a button', () => {
   `)
 })
 
+test(`' ' on a button is the same as '{space}'`, () => {
+  const {element, getEventSnapshot} = setup('<button />')
+
+  userEvent.type(element, ' ')
+
+  expect(getEventSnapshot()).toMatchInlineSnapshot(`
+    Events fired on: button
+
+    button - pointerover
+    button - pointerenter
+    button - mouseover: Left (0)
+    button - mouseenter: Left (0)
+    button - pointermove
+    button - mousemove: Left (0)
+    button - pointerdown
+    button - mousedown: Left (0)
+    button - focus
+    button - focusin
+    button - pointerup
+    button - mouseup: Left (0)
+    button - click: Left (0)
+    button - keydown: (32)
+    button - keypress: (32)
+    button - keyup: (32)
+    button - click: Left (0)
+  `)
+})
+
 test('{space} with preventDefault keydown on button', () => {
   const {element, getEventSnapshot} = setup('<button />', {
     eventHandlers: {
@@ -498,14 +556,17 @@ test('{space} with preventDefault keydown on button', () => {
     button - click: Left (0)
     button - keydown: (32)
     button - keyup: (32)
-    button - click: Left (0)
   `)
 })
 
-test(`' ' on a button`, () => {
-  const {element, getEventSnapshot} = setup('<button />')
+test('{space} with preventDefault keyup on button', () => {
+  const {element, getEventSnapshot} = setup('<button />', {
+    eventHandlers: {
+      keyUp: e => e.preventDefault(),
+    },
+  })
 
-  userEvent.type(element, ' ')
+  userEvent.type(element, '{space}')
 
   expect(getEventSnapshot()).toMatchInlineSnapshot(`
     Events fired on: button
@@ -526,7 +587,6 @@ test(`' ' on a button`, () => {
     button - keydown: (32)
     button - keypress: (32)
     button - keyup: (32)
-    button - click: Left (0)
   `)
 })
 
@@ -559,7 +619,7 @@ test('{space} on an input', () => {
   `)
 })
 
-test('{enter} on an input type="color"', () => {
+test('{enter} on an input type="color" fires same events as a button', () => {
   const {element, getEventSnapshot} = setup(
     `<input value="#ffffff" type="color" />`,
   )
@@ -589,7 +649,7 @@ test('{enter} on an input type="color"', () => {
   `)
 })
 
-test('{space} on an input type="color"', () => {
+test('{space} on an input type="color" fires same events as a button', () => {
   const {element, getEventSnapshot} = setup(
     `<input value="#ffffff" type="color" />`,
   )
@@ -619,7 +679,7 @@ test('{space} on an input type="color"', () => {
   `)
 })
 
-test('" " on an input type="color"', () => {
+test(`' ' on input type="color" is the same as '{space}'`, () => {
   const {element, getEventSnapshot} = setup(
     `<input value="#ffffff" type="color" />`,
   )
diff --git a/src/__tests__/type.js b/src/__tests__/type.js
index 04d40acf..73ab1892 100644
--- a/src/__tests__/type.js
+++ b/src/__tests__/type.js
@@ -707,13 +707,12 @@ test('typing an invalid input value', () => {
   expect(element.validity.badInput).toBe(false)
 })
 
-test('should give error if we are trying to call type on an invalid element', async () => {
-  const {element} = setup('<div  />')
-  await expect(() =>
-    userEvent.type(element, "I'm only a div :("),
-  ).rejects.toThrowErrorMatchingInlineSnapshot(
-    `"the current element is of type BODY and doesn't have a valid value"`,
-  )
+test('should not throw error if we are trying to call type on an element without a value', () => {
+  const {element} = setup('<div />')
+  expect.assertions(0)
+  return userEvent
+    .type(element, "I'm only a div :(")
+    .catch(e => expect(e).toBeUndefined())
 })
 
 test('typing on button should not alter its value', () => {
@@ -752,11 +751,8 @@ test('typing on input type submit should not alter its value', () => {
   expect(element).toHaveValue('foo')
 })
 
-test('typing on input type file should not trigger input event', () => {
-  const inputHandler = jest.fn()
-  const {element} = setup('<input type="file" />', {
-    eventHandlers: {input: inputHandler},
-  })
-  userEvent.type(element, 'bar')
-  expect(inputHandler).toHaveBeenCalledTimes(0)
+test('typing on input type file should not result in an error', () => {
+  const {element} = setup('<input type="file" />')
+  expect.assertions(0)
+  return userEvent.type(element, 'bar').catch(e => expect(e).toBeUndefined())
 })
diff --git a/src/paste.js b/src/paste.js
index ce88ea26..30100e69 100644
--- a/src/paste.js
+++ b/src/paste.js
@@ -38,11 +38,7 @@ function paste(
   fireEvent.paste(element, init)
 
   if (!element.readOnly) {
-    const {newValue, newSelectionStart} = calculateNewValue(
-      text,
-      element,
-      element.value,
-    )
+    const {newValue, newSelectionStart} = calculateNewValue(text, element)
     fireEvent.input(element, {
       inputType: 'insertFromPaste',
       target: {value: newValue},
diff --git a/src/type.js b/src/type.js
index 3c62b81a..c4324c28 100644
--- a/src/type.js
+++ b/src/type.js
@@ -88,17 +88,6 @@ async function typeImpl(
   // The focused element could change between each event, so get the currently active element each time
   const currentElement = () => getActiveElement(element.ownerDocument)
 
-  const currentValue = () => {
-    const activeElement = currentElement()
-    const value = activeElement.value
-    if (typeof value === 'undefined') {
-      throw new TypeError(
-        `the current element is of type ${activeElement.tagName} and doesn't have a valid value`,
-      )
-    }
-    return value
-  }
-
   // by default, a new element has it's selection start and end at 0
   // but most of the time when people call "type", they expect it to type
   // at the end of the current input value. So, if the selection start
@@ -107,14 +96,16 @@ async function typeImpl(
   // the only time it would make sense to pass the initialSelectionStart or
   // initialSelectionEnd is if you have an input with a value and want to
   // explicitely start typing with the cursor at 0. Not super common.
+  const value = currentElement().value
   if (
+    value != null &&
     currentElement().selectionStart === 0 &&
     currentElement().selectionEnd === 0
   ) {
     setSelectionRangeIfNecessary(
       currentElement(),
-      initialSelectionStart ?? currentValue().length,
-      initialSelectionEnd ?? currentValue().length,
+      initialSelectionStart ?? value.length,
+      initialSelectionEnd ?? value.length,
     )
   }
 
@@ -145,7 +136,6 @@ async function typeImpl(
       if (!currentElement().disabled) {
         const returnValue = callback({
           currentElement,
-          currentValue,
           prevWasMinus,
           prevWasPeriod,
           prevValue,
@@ -215,26 +205,21 @@ function getSpecialCharCallback(remainingString) {
 
 function getTypeCallback(remainingString) {
   const character = remainingString[0]
-  const callback = createTypeCharacter(character)
+  const callback = context => typeCharacter(character, context)
   return {
     callback,
     remainingString: remainingString.slice(1),
   }
 }
 
-function setSelectionRange({
-  currentElement,
-  currentValue,
-  newValue,
-  newSelectionStart,
-}) {
+function setSelectionRange({currentElement, newValue, newSelectionStart}) {
   // if we *can* change the selection start, then we will if the new value
   // is the same as the current value (so it wasn't programatically changed
   // when the fireEvent.input was triggered).
   // The reason we have to do this at all is because it actually *is*
   // programmatically changed by fireEvent.input, so we have to simulate the
   // browser's default behavior
-  const value = currentValue()
+  const value = currentElement().value
 
   if (value === newValue) {
     setSelectionRangeIfNecessary(
@@ -251,13 +236,12 @@ function setSelectionRange({
 }
 
 function fireInputEventIfNeeded({
+  currentElement,
   newValue,
   newSelectionStart,
   eventOverrides,
-  currentValue,
-  currentElement,
 }) {
-  const prevValue = currentValue()
+  const prevValue = currentElement().value
   if (
     !currentElement().readOnly &&
     !isClickable(currentElement()) &&
@@ -270,7 +254,6 @@ function fireInputEventIfNeeded({
 
     setSelectionRange({
       currentElement,
-      currentValue,
       newValue,
       newSelectionStart,
     })
@@ -279,15 +262,10 @@ function fireInputEventIfNeeded({
   return {prevValue}
 }
 
-function createTypeCharacter(character) {
-  return context => typeCharacter(character, context)
-}
-
 function typeCharacter(
   char,
   {
     currentElement,
-    currentValue,
     prevWasMinus = false,
     prevWasPeriod = false,
     prevValue = '',
@@ -313,7 +291,7 @@ function typeCharacter(
       ...eventOverrides,
     })
 
-    if (keyPressDefaultNotPrevented) {
+    if (currentElement().value != null && keyPressDefaultNotPrevented) {
       let newEntry = char
       if (prevWasMinus) {
         newEntry = `-${char}`
@@ -322,13 +300,12 @@ function typeCharacter(
       }
 
       const inputEvent = fireInputEventIfNeeded({
-        ...calculateNewValue(newEntry, currentElement(), currentValue()),
+        ...calculateNewValue(newEntry, currentElement()),
         eventOverrides: {
           data: key,
           inputType: 'insertText',
           ...eventOverrides,
         },
-        currentValue,
         currentElement,
       })
       prevValue = inputEvent.prevValue
@@ -340,7 +317,7 @@ function typeCharacter(
       // to typing an invalid character (typing "-a3" results in "-3")
       // same applies for the decimal character.
       if (currentElement().type === 'number') {
-        const newValue = currentValue()
+        const newValue = currentElement().value
         if (newValue === prevValue && newEntry !== '-') {
           nextPrevWasMinus = prevWasMinus
         } else {
@@ -373,8 +350,8 @@ function typeCharacter(
 // and you may be tempted to create a shared abstraction.
 // If you, brave soul, decide to so endevor, please increment this count
 // when you inevitably fail: 1
-function calculateNewBackspaceValue(element, value) {
-  const {selectionStart, selectionEnd} = element
+function calculateNewBackspaceValue(element) {
+  const {selectionStart, selectionEnd, value} = element
   let newValue, newSelectionStart
 
   if (selectionStart === null) {
@@ -406,8 +383,8 @@ function calculateNewBackspaceValue(element, value) {
   return {newValue, newSelectionStart}
 }
 
-function calculateNewDeleteValue(element, value) {
-  const {selectionStart, selectionEnd} = element
+function calculateNewDeleteValue(element) {
+  const {selectionStart, selectionEnd, value} = element
   let newValue
 
   if (selectionStart === null) {
@@ -471,7 +448,7 @@ function createModifierCallbackEntries({name, key, keyCode, modifierProperty}) {
   }
 }
 
-function handleEnter({currentElement, currentValue, eventOverrides}) {
+function handleEnter({currentElement, eventOverrides}) {
   const key = 'Enter'
   const keyCode = 13
 
@@ -489,19 +466,17 @@ function handleEnter({currentElement, currentValue, eventOverrides}) {
       charCode: keyCode,
       ...eventOverrides,
     })
-  }
-
-  if (isClickable(currentElement())) {
-    fireEvent.click(currentElement(), {
-      ...eventOverrides,
-    })
+    if (isClickable(currentElement())) {
+      fireEvent.click(currentElement(), {
+        ...eventOverrides,
+      })
+    }
   }
 
   if (currentElement().tagName === 'TEXTAREA') {
     const {newValue, newSelectionStart} = calculateNewValue(
       '\n',
       currentElement(),
-      currentValue(),
     )
     fireEvent.input(currentElement(), {
       target: {value: newValue},
@@ -510,7 +485,6 @@ function handleEnter({currentElement, currentValue, eventOverrides}) {
     })
     setSelectionRange({
       currentElement,
-      currentValue,
       newValue,
       newSelectionStart,
     })
@@ -545,7 +519,7 @@ function handleEsc({currentElement, eventOverrides}) {
   })
 }
 
-function handleDel({currentElement, currentValue, eventOverrides}) {
+function handleDel({currentElement, eventOverrides}) {
   const key = 'Delete'
   const keyCode = 46
 
@@ -558,13 +532,12 @@ function handleDel({currentElement, currentValue, eventOverrides}) {
 
   if (keyPressDefaultNotPrevented) {
     fireInputEventIfNeeded({
-      ...calculateNewDeleteValue(currentElement(), currentValue()),
+      ...calculateNewDeleteValue(currentElement()),
       eventOverrides: {
         inputType: 'deleteContentForward',
         ...eventOverrides,
       },
       currentElement,
-      currentValue,
     })
   }
 
@@ -576,7 +549,7 @@ function handleDel({currentElement, currentValue, eventOverrides}) {
   })
 }
 
-function handleBackspace({currentElement, currentValue, eventOverrides}) {
+function handleBackspace({currentElement, eventOverrides}) {
   const key = 'Backspace'
   const keyCode = 8
 
@@ -589,13 +562,12 @@ function handleBackspace({currentElement, currentValue, eventOverrides}) {
 
   if (keyPressDefaultNotPrevented) {
     fireInputEventIfNeeded({
-      ...calculateNewBackspaceValue(currentElement(), currentValue()),
+      ...calculateNewBackspaceValue(currentElement()),
       eventOverrides: {
         inputType: 'deleteContentBackward',
         ...eventOverrides,
       },
       currentElement,
-      currentValue,
     })
   }
 
@@ -607,10 +579,10 @@ function handleBackspace({currentElement, currentValue, eventOverrides}) {
   })
 }
 
-function handleSelectall({currentElement, currentValue}) {
+function handleSelectall({currentElement}) {
   // the user can actually select in several different ways
   // we're not going to choose, so we'll *only* set the selection range
-  currentElement().setSelectionRange(0, currentValue().length)
+  currentElement().setSelectionRange(0, currentElement().value.length)
 }
 
 function handleSpace(context) {
@@ -641,21 +613,18 @@ function handleSpaceOnClickable({currentElement, eventOverrides}) {
     })
   }
 
-  fireEvent.keyUp(currentElement(), {
+  const keyUpDefaultNotPrevented = fireEvent.keyUp(currentElement(), {
     key,
     keyCode,
     which: keyCode,
     ...eventOverrides,
   })
 
-  fireEvent.click(currentElement(), {
-    ...eventOverrides,
-  })
+  if (keyDownDefaultNotPrevented && keyUpDefaultNotPrevented) {
+    fireEvent.click(currentElement(), {
+      ...eventOverrides,
+    })
+  }
 }
 
 export {type}
-
-/*
-eslint
-  no-loop-func: "off",
-*/