From 0dd387e2be5b7c74c27e81be2e0d56e3b73a1279 Mon Sep 17 00:00:00 2001 From: Marco Moretti Date: Mon, 22 Jun 2020 00:28:26 +0200 Subject: [PATCH] fix: add message if type/paste function is called with an invalid element (#375) * test: add failing test * fix: add an explicit error on type function If the user tries to call type function with an invalid element, an explicit error will be thrown. This error should be better then 'TypeError: Cannot read property 'length' of undefined' * refactor: replace error with typerror * test: add fail test for paste * fix: throw TypeError for paste and type functions --- README.md | 1 + src/__tests__/paste.js | 9 +++++++++ src/__tests__/type.js | 9 +++++++++ src/paste.js | 12 ++++++++++-- src/type.js | 30 +++++++++++++++++++++--------- src/utils.js | 1 + 6 files changed, 51 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 9f1297d4..2d1d9703 100644 --- a/README.md +++ b/README.md @@ -552,6 +552,7 @@ Thanks goes to these people ([emoji key][emojis]): + This project follows the [all-contributors][all-contributors] specification. diff --git a/src/__tests__/paste.js b/src/__tests__/paste.js index 10997c4a..4872c236 100644 --- a/src/__tests__/paste.js +++ b/src/__tests__/paste.js @@ -94,3 +94,12 @@ test('should replace selected text all at once', () => { userEvent.paste(element, 'friend') expect(element).toHaveValue('hello friend') }) + +test('should give error if we are trying to call paste on an invalid element', () => { + const {element} = setup('
') + expect(() => + userEvent.paste(element, "I'm only a div :("), + ).toThrowErrorMatchingInlineSnapshot( + `"the current element is of type DIV and doesn't have a valid value"`, + ) +}) diff --git a/src/__tests__/type.js b/src/__tests__/type.js index 1444656e..f12c10cc 100644 --- a/src/__tests__/type.js +++ b/src/__tests__/type.js @@ -706,3 +706,12 @@ test('typing an invalid input value', () => { // but the badInput should actually be "true" if the user types "3-3" 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('
') + 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"`, + ) +}) diff --git a/src/paste.js b/src/paste.js index 3306eb50..5c091290 100644 --- a/src/paste.js +++ b/src/paste.js @@ -8,7 +8,11 @@ function paste( {initialSelectionStart, initialSelectionEnd} = {}, ) { if (element.disabled) return - + if (typeof element.value === 'undefined') { + throw new TypeError( + `the current element is of type ${element.tagName} and doesn't have a valid value`, + ) + } element.focus() // by default, a new element has it's selection start and end at 0 @@ -30,7 +34,11 @@ function paste( fireEvent.paste(element, init) if (!element.readOnly) { - const {newValue, newSelectionStart} = calculateNewValue(text, element) + const {newValue, newSelectionStart} = calculateNewValue( + text, + element, + element.value, + ) fireEvent.input(element, { inputType: 'insertFromPaste', target: {value: newValue}, diff --git a/src/type.js b/src/type.js index 9a137be3..b3ebb80d 100644 --- a/src/type.js +++ b/src/type.js @@ -53,7 +53,16 @@ async function typeImpl( // and not just the element if the active element could change while the function // is being run (for example, functions that are and/or fire events). const currentElement = () => getActiveElement(element.ownerDocument) - const currentValue = () => currentElement().value + 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 + } const setSelectionRange = ({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 @@ -98,6 +107,7 @@ async function typeImpl( const eventCallbackMap = getEventCallbackMap({ currentElement, + currentValue, fireInputEventIfNeeded, setSelectionRange, }) @@ -212,7 +222,7 @@ async function typeImpl( } const inputEvent = fireInputEventIfNeeded({ - ...calculateNewValue(newEntry, currentElement()), + ...calculateNewValue(newEntry, currentElement(), currentValue()), eventOverrides: { data: key, inputType: 'insertText', @@ -262,8 +272,8 @@ async function typeImpl( // 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) { - const {selectionStart, selectionEnd, value} = element +function calculateNewBackspaceValue(element, value) { + const {selectionStart, selectionEnd} = element let newValue, newSelectionStart if (selectionStart === null) { @@ -295,8 +305,8 @@ function calculateNewBackspaceValue(element) { return {newValue, newSelectionStart} } -function calculateNewDeleteValue(element) { - const {selectionStart, selectionEnd, value} = element +function calculateNewDeleteValue(element, value) { + const {selectionStart, selectionEnd} = element let newValue if (selectionStart === null) { @@ -325,6 +335,7 @@ function calculateNewDeleteValue(element) { function getEventCallbackMap({ currentElement, + currentValue, fireInputEventIfNeeded, setSelectionRange, }) { @@ -383,6 +394,7 @@ function getEventCallbackMap({ const {newValue, newSelectionStart} = calculateNewValue( '\n', currentElement(), + currentValue(), ) fireEvent.input(currentElement(), { target: {value: newValue}, @@ -432,7 +444,7 @@ function getEventCallbackMap({ if (keyPressDefaultNotPrevented) { fireInputEventIfNeeded({ - ...calculateNewDeleteValue(currentElement()), + ...calculateNewDeleteValue(currentElement(), currentValue()), eventOverrides: { inputType: 'deleteContentForward', ...eventOverrides, @@ -460,7 +472,7 @@ function getEventCallbackMap({ if (keyPressDefaultNotPrevented) { fireInputEventIfNeeded({ - ...calculateNewBackspaceValue(currentElement()), + ...calculateNewBackspaceValue(currentElement(), currentValue()), eventOverrides: { inputType: 'deleteContentBackward', ...eventOverrides, @@ -478,7 +490,7 @@ function getEventCallbackMap({ // the user can actually select in several different ways // we're not going to choose, so we'll *only* set the selection range '{selectall}': () => { - currentElement().setSelectionRange(0, currentElement().value.length) + currentElement().setSelectionRange(0, currentValue().length) }, } diff --git a/src/utils.js b/src/utils.js index d38745a8..9057126a 100644 --- a/src/utils.js +++ b/src/utils.js @@ -94,6 +94,7 @@ function getActiveElement(document) { 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)