From ab78f3f50d8ec9a2fa8b3794ceb9a9b1b02fb3a6 Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Thu, 12 May 2022 12:31:21 +0200 Subject: [PATCH 1/2] fix: guard against selection without range (#953) --- src/utils/focus/selection.ts | 5 +-- tests/utils/focus/selection.ts | 76 ++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/utils/focus/selection.ts b/src/utils/focus/selection.ts index c09282c2..2149309d 100644 --- a/src/utils/focus/selection.ts +++ b/src/utils/focus/selection.ts @@ -217,14 +217,13 @@ export function moveSelection(node: Element, direction: -1 | 1) { } else { const selection = node.ownerDocument.getSelection() - /* istanbul ignore if */ - if (!selection) { + if (!selection?.focusNode) { return } if (selection.isCollapsed) { const nextPosition = getNextCursorPosition( - selection.focusNode as Node, + selection.focusNode, selection.focusOffset, direction, ) diff --git a/tests/utils/focus/selection.ts b/tests/utils/focus/selection.ts index 6c09b617..16c68e6b 100644 --- a/tests/utils/focus/selection.ts +++ b/tests/utils/focus/selection.ts @@ -4,6 +4,7 @@ import { setSelection, setSelectionRange, modifySelection, + moveSelection, } from '#src/utils' import {setup} from '#testHelpers' @@ -183,3 +184,78 @@ describe('update selection when moving focus into element with own selection imp expect(document.getSelection()).toHaveProperty('focusOffset', 0) }) }) + +describe('move selection', () => { + test('do nothing without a selection range', () => { + const {element} = setup(`
`) + document.getSelection()?.removeAllRanges() + + moveSelection(element, 1) + + expect(document.getSelection()).toHaveProperty('rangeCount', 0) + }) + + test('move to next cursor position', () => { + const {element} = setup(`
foo
`, { + selection: {focusNode: 'div/text()', focusOffset: 1}, + }) + + moveSelection(element, 1) + + expect(document.getSelection()).toHaveProperty( + 'focusNode', + element.firstChild, + ) + expect(document.getSelection()).toHaveProperty('focusOffset', 2) + }) + + test('move to next cursor position', () => { + const {element} = setup(`
foo
`, { + selection: {focusNode: 'div/text()', focusOffset: 1}, + }) + + moveSelection(element, 1) + + expect(document.getSelection()).toHaveProperty( + 'focusNode', + element.firstChild, + ) + expect(document.getSelection()).toHaveProperty('focusOffset', 2) + expect(document.getSelection()).toHaveProperty('isCollapsed', true) + }) + + test('collapse range', () => { + const {element} = setup(`
foo
`, { + selection: {focusNode: 'div/text()', anchorOffset: 1, focusOffset: 2}, + }) + + moveSelection(element, 1) + + expect(document.getSelection()).toHaveProperty( + 'focusNode', + element.firstChild, + ) + expect(document.getSelection()).toHaveProperty('focusOffset', 2) + expect(document.getSelection()).toHaveProperty('isCollapsed', true) + }) + + test('move cursor in input', () => { + const {element} = setup(``) + + moveSelection(element, 1) + + expect(element).toHaveProperty('selectionStart', 1) + expect(element).toHaveProperty('selectionEnd', 1) + }) + + test('collapse range in input', () => { + const {element} = setup(``, { + selection: {anchorOffset: 1, focusOffset: 2}, + }) + + moveSelection(element, 1) + + expect(element).toHaveProperty('selectionStart', 2) + expect(element).toHaveProperty('selectionEnd', 2) + }) +}) From 6f55feebffcb331f627c8467b856723d90e06896 Mon Sep 17 00:00:00 2001 From: Philipp Fritsche Date: Thu, 12 May 2022 12:33:45 +0200 Subject: [PATCH 2/2] fix: wait after each method before leaving `asyncWrapper` (#952) --- src/setup/setup.ts | 8 +++++++- tests/keyboard/index.ts | 4 +--- tests/keyboard/keyboardAction.ts | 6 ++++-- tests/pointer/index.ts | 4 +--- tests/setup/_mockApis.ts | 24 ++++++++++++----------- tests/setup/index.ts | 33 +++++++++++++++++++++++++++++++- 6 files changed, 58 insertions(+), 21 deletions(-) diff --git a/src/setup/setup.ts b/src/setup/setup.ts index fc5b2971..b2f7cb3f 100644 --- a/src/setup/setup.ts +++ b/src/setup/setup.ts @@ -8,6 +8,7 @@ import { attachClipboardStubToView, getDocumentFromNode, setLevelRef, + wait, } from '../utils' import type {Instance, UserEvent, UserEventApi} from './index' import {Config} from './config' @@ -80,7 +81,12 @@ function wrapAndBindImpl< function method(...args: Args) { setLevelRef(instance[Config], ApiLevel.Call) - return wrapAsync(() => impl.apply(instance, args)) + return wrapAsync(() => + impl.apply(instance, args).then(async ret => { + await wait(instance[Config]) + return ret + }), + ) } Object.defineProperty(method, 'name', {get: () => impl.name}) diff --git a/tests/keyboard/index.ts b/tests/keyboard/index.ts index 5e8763ec..af482eaf 100644 --- a/tests/keyboard/index.ts +++ b/tests/keyboard/index.ts @@ -123,9 +123,7 @@ describe('delay', () => { const time0 = performance.now() await user.keyboard('foo') - // we don't call delay after the last action - // TODO: Should we call it? - expect(spy).toBeCalledTimes(2) + expect(spy.mock.calls.length).toBeGreaterThanOrEqual(2) expect(time0).toBeLessThan(performance.now() - 20) }) diff --git a/tests/keyboard/keyboardAction.ts b/tests/keyboard/keyboardAction.ts index c937ebff..12dbd95c 100644 --- a/tests/keyboard/keyboardAction.ts +++ b/tests/keyboard/keyboardAction.ts @@ -184,7 +184,9 @@ test('do not call setTimeout with delay `null`', async () => { const {user} = setup(`
`) const spy = jest.spyOn(global, 'setTimeout') await user.keyboard('ab') - expect(spy).toBeCalledTimes(1) + expect(spy.mock.calls.length).toBeGreaterThanOrEqual(1) + + spy.mockClear() await user.setup({delay: null}).keyboard('cd') - expect(spy).toBeCalledTimes(1) + expect(spy).not.toBeCalled() }) diff --git a/tests/pointer/index.ts b/tests/pointer/index.ts index ab9ed688..670ef111 100644 --- a/tests/pointer/index.ts +++ b/tests/pointer/index.ts @@ -73,9 +73,7 @@ describe('delay', () => { '[/MouseLeft]', ]) - // we don't call delay after the last action - // TODO: Should we call it? - expect(spy).toBeCalledTimes(2) + expect(spy.mock.calls.length).toBeGreaterThanOrEqual(2) expect(time0).toBeLessThan(performance.now() - 20) }) diff --git a/tests/setup/_mockApis.ts b/tests/setup/_mockApis.ts index 0879054e..ce1a46bc 100644 --- a/tests/setup/_mockApis.ts +++ b/tests/setup/_mockApis.ts @@ -6,13 +6,12 @@ import {Instance, UserEventApi} from '#src/setup' // `const` are not initialized when mocking is executed, but `function` are when prefixed with `mock` function mockApis() {} // access the `function` as object -type mockApisRefHack = (() => void) & - { - [name in keyof UserEventApi]: { - mock: APIMock - real: UserEventApi[name] - } +type mockApisRefHack = (() => void) & { + [name in keyof UserEventApi]: { + mock: APIMock + real: UserEventApi[name] } +} // make the tests more readable by applying the typecast here export function getSpy(k: keyof UserEventApi) { @@ -34,6 +33,10 @@ interface APIMock this: Instance, ...args: Parameters ): ReturnType + originalMockImplementation: ( + this: Instance, + ...args: Parameters + ) => ReturnType } jest.mock('#src/setup/api', () => { @@ -44,15 +47,14 @@ jest.mock('#src/setup/api', () => { } ;(Object.keys(real) as Array).forEach(key => { - const mock = jest.fn(function mockImpl( - this: Instance, - ...args: unknown[] - ) { + const mock = jest.fn(mockImpl) as unknown as APIMock + function mockImpl(this: Instance, ...args: unknown[]) { Object.defineProperty(mock.mock.lastCall, 'this', { get: () => this, }) return (real[key] as Function).apply(this, args) - }) as unknown as APIMock + } + mock.originalMockImplementation = mockImpl Object.defineProperty(mock, 'name', { get: () => `mock-${key}`, diff --git a/tests/setup/index.ts b/tests/setup/index.ts index 4b97e397..38773976 100644 --- a/tests/setup/index.ts +++ b/tests/setup/index.ts @@ -1,6 +1,7 @@ +import {getConfig} from '@testing-library/dom' import {getSpy} from './_mockApis' import userEvent from '#src' -import {Config, UserEventApi} from '#src/setup' +import {Config, Instance, UserEventApi} from '#src/setup' import {render} from '#testHelpers' type ApiDeclarations = { @@ -80,6 +81,13 @@ declare module '#src/options' { } } +// eslint-disable-next-line @typescript-eslint/unbound-method +const realAsyncWrapper = getConfig().asyncWrapper +afterEach(() => { + getConfig().asyncWrapper = realAsyncWrapper + jest.restoreAllMocks() +}) + test.each(apiDeclarationsEntries)( 'call `%s` api on instance', async (name, {args = [], elementArg, elementHtml = ``}) => { @@ -95,11 +103,34 @@ test.each(apiDeclarationsEntries)( expect(apis[name]).toHaveProperty('name', `mock-${name}`) + // Replace the asyncWrapper to make sure that a delayed state update happens inside of it + const stateUpdate = jest.fn() + spy.mockImplementation(async function impl( + this: Instance, + ...a: Parameters + ) { + const ret = spy.originalMockImplementation.apply(this, a) + void ret.then(() => setTimeout(stateUpdate)) + return ret + } as typeof spy['originalMockImplementation']) + const asyncWrapper = jest.fn(async (cb: () => Promise) => { + stateUpdate.mockClear() + const ret = cb() + expect(stateUpdate).not.toBeCalled() + await ret + expect(stateUpdate).toBeCalled() + return ret + }) + getConfig().asyncWrapper = asyncWrapper + await (apis[name] as Function)(...args) expect(spy).toBeCalledTimes(1) expect(spy.mock.lastCall?.this?.[Config][opt]).toBe(true) + // Make sure the asyncWrapper mock has been used in the API call + expect(asyncWrapper).toBeCalled() + const subApis = apis.setup({}) await (subApis[name] as Function)(...args)