Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: identify browser/jsdom differences #1091

Merged
merged 17 commits into from
Jan 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
76a438c
refactor: avoid declaring optional property per nullish coalescing as…
ph-fritsche Jan 12, 2025
2f25f81
test: `window.getComputedStyle()` returns resolved style in browser
ph-fritsche Dec 27, 2022
dcaaf90
test: do not expect to report origin of `pointer-events: none` in bro…
ph-fritsche Jan 9, 2025
98bc5e0
test: expect `DataTransfer.types` to follow specs
ph-fritsche Jan 2, 2023
5d0dcaf
chore: add TODO to await `DataTransferItem.getAsString` callback
ph-fritsche Jan 9, 2025
9cf8d4c
test: `Selection.setBaseAndExtent()` resets input selection in browser
ph-fritsche Jan 2, 2023
4e420c0
test: work around race condition with event handler
ph-fritsche Jan 2, 2023
1401e4e
test: exclude `select` events from screenshots
ph-fritsche Jan 2, 2023
2a3979d
test: range on focused contenteditable is defined in browser
ph-fritsche Jan 3, 2023
039cde4
test: `HTMLInputElement.focus()` in contenteditable changes selection…
ph-fritsche Jan 3, 2023
b58b19a
test: get text nodes in wrapper per relative path
ph-fritsche Jan 3, 2023
3180629
chore: print content of `console.error` calls
ph-fritsche Jan 2, 2025
ff4eee3
chore: upgrade testenv `react18`
ph-fritsche Jan 2, 2025
5057e39
test: convert flaky assertion on `<input type="file"/>` to TODO
ph-fritsche Jan 9, 2025
57c9b65
chore: add TODO to simulate `FocusEvent` in browsers
ph-fritsche Jan 9, 2025
a76db96
chore: add TODO to update `updateSelectionOnFocus`
ph-fritsche Jan 9, 2025
139d18f
test: reset DTL `asyncWrapper` before asserting `setTimeout` calls
ph-fritsche Jan 10, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions scripts/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ const env = await serveDir('testenv')
const {cli, connectCoverageReporter} = await setupToolboxTester(
['src', 'tests'],
[
setupNodeConductor('Node, DTL8, React18', [
setupNodeConductor('Node, DTL10, React18', [
new URL('../testenv/node.js', import.meta.url),
new URL('./libs/dom8/index.bundle.js', env.url),
new URL('./libs/dom10/index.bundle.js', env.url),
new URL('./libs/react18/index.bundle.js', env.url),
]),
setupNodeConductor('Node, DTL8, React17', [
new URL('../testenv/node.js', import.meta.url),
new URL('./libs/dom8/index.bundle.js', env.url),
new URL('./libs/react17/index.bundle.js', env.url),
]),
setupChromeConductor('Chrome, DTL8, React18', [
setupChromeConductor('Chrome, DTL10, React18', [
new URL('./browser.bundle.js', env.url),
new URL('./libs/dom8/index.bundle.js', env.url),
new URL('./libs/dom10/index.bundle.js', env.url),
new URL('./libs/react18/index.bundle.js', env.url),
]),
],
Expand Down
3 changes: 3 additions & 0 deletions src/event/focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import {findClosest, getActiveElement, isFocusable} from '../utils'
import {updateSelectionOnFocus} from './selection'
import {wrapEvent} from './wrapEvent'

// Browsers do not dispatch FocusEvent if the document does not have focus.
// TODO: simulate FocusEvent in browsers

/**
* Focus closest focusable element.
*/
Expand Down
5 changes: 5 additions & 0 deletions src/event/selection/updateSelectionOnFocus.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import {getContentEditable, hasOwnSelection} from '../../utils'

// The browser implementation seems to have changed.
// When focus is inside <input type="text"/>,
// Chrome updates Selection to be collapsed at the position of the input element.
// TODO: update implementation to match that of current browsers

/**
* Reset the Document Selection when moving focus into an element
* with own selection implementation.
Expand Down
61 changes: 43 additions & 18 deletions src/system/keyboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,51 @@ export class KeyboardHost {
Symbol: false,
SymbolLock: false,
}
readonly pressed: Record<
string,
{
keyDef: keyboardKey
unpreventedDefault: boolean
private readonly pressed = new (class {
registry: {
[k in string]?: {
keyDef: keyboardKey
unpreventedDefault: boolean
}
} = {}
add(code: string, keyDef: keyboardKey) {
this.registry[code] ??= {
keyDef,
unpreventedDefault: false,
}
}
has(code: string) {
return !!this.registry[code]
}
setUnprevented(code: string) {
const o = this.registry[code]
if (o) {
o.unpreventedDefault = true
}
}
> = {}
isUnprevented(code: string) {
return !!this.registry[code]?.unpreventedDefault
}
delete(code: string) {
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete this.registry[code]
}
values() {
return Object.values(this.registry) as NonNullable<
typeof this.registry[string]
>[]
}
})()
carryChar = ''
private lastKeydownTarget: Element | undefined = undefined
private readonly modifierLockStart: Record<string, boolean> = {}

isKeyPressed(keyDef: keyboardKey) {
return !!this.pressed[String(keyDef.code)]
return this.pressed.has(String(keyDef.code))
}

getPressedKeys() {
return Object.values(this.pressed).map(p => p.keyDef)
return this.pressed.values().map(p => p.keyDef)
}

/** Press a key */
Expand All @@ -97,11 +125,7 @@ export class KeyboardHost {
const target = getActiveElementOrBody(instance.config.document)
this.setKeydownTarget(target)

// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
this.pressed[code] ??= {
keyDef,
unpreventedDefault: false,
}
this.pressed.add(code, keyDef)

if (isModifierKey(key)) {
this.modifiers[key] = true
Expand All @@ -117,7 +141,9 @@ export class KeyboardHost {
this.modifierLockStart[key] = true
}

this.pressed[code].unpreventedDefault ||= unprevented
if (unprevented) {
this.pressed.setUnprevented(code)
}

if (unprevented && this.hasKeyPress(key)) {
instance.dispatchUIEvent(
Expand All @@ -138,14 +164,13 @@ export class KeyboardHost {
const key = String(keyDef.key)
const code = String(keyDef.code)

const unprevented = this.pressed[code].unpreventedDefault
const unprevented = this.pressed.isUnprevented(code)

// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete this.pressed[code]
this.pressed.delete(code)

if (
isModifierKey(key) &&
!Object.values(this.pressed).find(p => p.keyDef.key === key)
!this.pressed.values().find(p => p.keyDef.key === key)
) {
this.modifiers[key] = false
}
Expand Down
6 changes: 2 additions & 4 deletions src/system/pointer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ export class PointerHost {
private readonly buttons

private readonly devices = new (class {
private registry = {} as Record<string, Device>
private registry: {[k in string]?: Device} = {}

get(k: string) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
this.registry[k] ??= new Device()
return this.registry[k]
return (this.registry[k] ??= new Device())
}
})()

Expand Down
1 change: 1 addition & 0 deletions src/utils/dataTransfer/DataTransfer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export function getBlobFromDataTransferItem(
if (item.kind === 'file') {
return item.getAsFile() as File
}
// TODO: await callback
let data: string = ''
item.getAsString(s => {
data = s
Expand Down
3 changes: 3 additions & 0 deletions testenv/libs/dom10/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as DomTestingLibrary from '@testing-library/dom'

globalThis.DomTestingLibrary = DomTestingLibrary
6 changes: 6 additions & 0 deletions testenv/libs/dom10/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "module",
"dependencies": {
"@testing-library/dom": "^10"
}
}
2 changes: 1 addition & 1 deletion testenv/libs/react18/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"type": "module",
"dependencies": {
"@testing-library/react": "^13",
"@testing-library/react": "^16",
"react": "^18",
"react-dom": "^18"
},
Expand Down
31 changes: 13 additions & 18 deletions testenv/modules/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,18 @@ beforeEach(() => {
})

afterEach(() => {
if (isCI && console.error.mock.calls.length) {
throw new Error(`console.error should not be called in tests`)
for (const k of ['error', 'log', 'warn', 'info']) {
const calls = console[k].mock.calls
if (isCI && calls.length) {
throw new Error(`console.${k} should not be calls in tests and was called ${calls.length} times:\n`
+ calls.map((args, i) => (`\n#${i}:\n` + args.map(a => (
(typeof a === 'object' || typeof a === 'function'
? typeof a
: JSON.stringify(a)
) + '\n'
))))
)
}
console[k].mockRestore()
}
console.error.mockRestore()

if (isCI && console.log.mock.calls.length) {
throw new Error(`console.log should not be called in tests`)
}
console.log.mockRestore()

if (isCI && console.warn.mock.calls.length) {
throw new Error(`console.warn should not be called in tests`)
}
console.warn.mockRestore()

if (isCI && console.info.mock.calls.length) {
throw new Error(`console.info should not be called in tests`)
}
console.info.mockRestore()
})
27 changes: 27 additions & 0 deletions tests/_helpers/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,32 @@
// this is pretty helpful:
// https://codesandbox.io/s/quizzical-worker-eo909

import { configure, getConfig } from '@testing-library/dom'

export {render, setup} from './setup'
export {addEventListener, addListeners} from './listeners'

export function isJsdomEnv() {
return window.navigator.userAgent.includes(' jsdom/')
}

/**
* Reset the DTL wrappers
*
* Framework libraries configure the wrappers in DTL as side effect when being imported.
* In the Toolbox testenvs that side effect is triggered by including the library as a setup file.
*/
export function resetWrappers() {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { asyncWrapper, eventWrapper } = {...getConfig()}

configure({
asyncWrapper: (cb) => cb(),
eventWrapper: (cb) => cb(),
})

return () => configure({
asyncWrapper,
eventWrapper,
})
}
1 change: 1 addition & 0 deletions tests/_helpers/listeners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export function addListeners(

function getEventSnapshot() {
const eventCalls = eventHandlerCalls
.filter(({event}) => event.type !== 'select')
.map(({event, elementDisplayName}) => {
const firstLine = [
`${elementDisplayName} - ${event.type}`,
Expand Down
4 changes: 2 additions & 2 deletions tests/clipboard/copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ test('copy selected value', async () => {

test('copy selected text outside of editable', async () => {
const {getEvents, user} = setup(`<div tabindex="-1">foo bar baz</div>`, {
selection: {focusNode: '//text()', anchorOffset: 1, focusOffset: 5},
selection: {focusNode: './/text()', anchorOffset: 1, focusOffset: 5},
})

const dt = await user.copy()
Expand All @@ -32,7 +32,7 @@ test('copy selected text outside of editable', async () => {

test('copy selected text in contenteditable', async () => {
const {getEvents, user} = setup(`<div contenteditable>foo bar baz</div>`, {
selection: {focusNode: '//text()', anchorOffset: 1, focusOffset: 5},
selection: {focusNode: './/text()', anchorOffset: 1, focusOffset: 5},
})

const dt = await user.copy()
Expand Down
4 changes: 2 additions & 2 deletions tests/clipboard/cut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ test('cut selected value', async () => {

test('cut selected text outside of editable', async () => {
const {getEvents, user} = setup(`<div tabindex="-1">foo bar baz</div>`, {
selection: {focusNode: '//text()', anchorOffset: 1, focusOffset: 5},
selection: {focusNode: './/text()', anchorOffset: 1, focusOffset: 5},
})

const dt = await user.cut()
Expand All @@ -36,7 +36,7 @@ test('cut selected text in contenteditable', async () => {
const {element, getEvents, user} = setup(
`<div contenteditable>foo bar baz</div>`,
{
selection: {focusNode: '//text()', anchorOffset: 1, focusOffset: 5},
selection: {focusNode: './/text()', anchorOffset: 1, focusOffset: 5},
},
)

Expand Down
17 changes: 17 additions & 0 deletions tests/environment/computedStyle.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import {isJsdomEnv, render} from '#testHelpers'

test('window.getComputedStyle returns resolved inherited style in browser', () => {
const {element, xpathNode} = render(`
<div style='pointer-events: none'>
<button></button>
</div>`)

expect(window.getComputedStyle(element)).toHaveProperty(
'pointer-events',
'none',
)
expect(window.getComputedStyle(xpathNode('//button'))).toHaveProperty(
'pointer-events',
isJsdomEnv() ? '' : 'none',
)
})
48 changes: 48 additions & 0 deletions tests/environment/select.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import {isJsdomEnv, render} from '#testHelpers'
import {waitFor} from '@testing-library/dom'

test('`Selection.setBaseAndExtent()` resets input selection in browser', async () => {
const {element} = render<HTMLInputElement>(`<input value="foo"/>`, {
selection: {focusOffset: 3},
})
expect(element.selectionStart).toBe(3)

element.ownerDocument.getSelection()?.setBaseAndExtent(element, 0, element, 0)

expect(element.selectionStart).toBe(isJsdomEnv() ? 3 : 0)
})

test('events are not dispatched on same microtask in browser', async () => {
const {element} = render<HTMLInputElement>(`<input value="foo"/>`)
const onSelect = mocks.fn()
element.addEventListener('select', onSelect)

element.setSelectionRange(1, 2)

expect(onSelect).toBeCalledTimes(isJsdomEnv() ? 1 : 0)

await waitFor(() => expect(onSelect).toBeCalledTimes(1))
})

test('`HTMLInputElement.focus()` in contenteditable changes `Selection` in browser', () => {
const {element, xpathNode} = render<HTMLInputElement>(
`<div contenteditable="true"><input/></div><span></span>`,
{
selection: {
focusNode: '//span',
},
},
)

expect(element.ownerDocument.getSelection()).toHaveProperty(
'anchorNode',
xpathNode('//span'),
)

xpathNode('//input').focus()

expect(element.ownerDocument.getSelection()).toHaveProperty(
'anchorNode',
isJsdomEnv() ? xpathNode('//span') : element,
)
})
2 changes: 1 addition & 1 deletion tests/event/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ cases(
`<div contenteditable="true">abcd</div>`,
{
selection: {
focusNode: '//text()',
focusNode: './/text()',
anchorOffset: range[0],
focusOffset: range[1],
},
Expand Down
Loading
Loading