Skip to content

Commit debc470

Browse files
jerome-benoitjerome-benoit
authored
fix(types): preserve EventTarget listener fallbacks (#646)
* fix(types): preserve EventTarget listener fallbacks * test: reuse shared TypeScript config * test(types): assert exact Workerd listener inference * fix(types): remove remaining EventTarget prototype access * chore(deps): align Workerd types version range
1 parent 95bd73a commit debc470

10 files changed

Lines changed: 252 additions & 61 deletions

File tree

‎.github/workflows/qa.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ jobs:
8181
- name: Type check
8282
run: pnpm typecheck
8383

84+
- name: Type check Workerd declarations
85+
run: pnpm typecheck:workerd
86+
8487
qa-bun:
8588
runs-on: ${{ matrix.os }}
8689
strategy:

‎eslint.config.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import neostandard, { plugins } from 'neostandard'
77

88
export default defineConfig([
99
{
10-
ignores: ['docs/**', 'package/**'],
10+
ignores: ['docs/**', 'package/**', 'type-tests/workerd/**'],
1111
},
1212
cspellConfigs.recommended,
1313
{

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"prepublishOnly": "pnpm build && rm -rf ./package && clean-publish",
2222
"postpublish": "rm -rf ./package",
2323
"typecheck": "tsc --noEmit",
24+
"typecheck:workerd": "pnpm build && tsc -p type-tests/workerd/tsconfig.json",
2425
"typedoc": "typedoc",
2526
"lint": "eslint --cache src test examples eslint.config.js tsdown.config.ts vitest.config.ts vitest.pretest.config.ts",
2627
"lint:fix": "eslint --cache --fix src test examples eslint.config.js tsdown.config.ts vitest.config.ts vitest.pretest.config.ts",
@@ -49,6 +50,7 @@
4950
},
5051
"license": "MIT",
5152
"devDependencies": {
53+
"@cloudflare/workers-types": "^5.20260823.1",
5254
"@commitlint/cli": "^21.2.2",
5355
"@commitlint/config-conventional": "^21.2.2",
5456
"@cspell/eslint-plugin": "^10.1.0",

‎pnpm-lock.yaml‎

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/bench.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ const subtractTimerOverheadConcurrencyError =
4141
* The Bench class keeps track of the benchmark tasks and controls them.
4242
*/
4343
export class Bench extends EventTarget implements BenchLike {
44-
declare addEventListener: <K extends BenchEvents>(
44+
declare addEventListener: (<K extends BenchEvents>(
4545
type: K,
4646
listener: EventListener<K> | EventListenerObject<K> | null,
4747
options?: AddEventListenerOptionsArgument
48-
) => void
48+
) => void) & EventTarget['addEventListener']
4949

5050
/**
5151
* Executes tasks concurrently based on the specified concurrency mode.
@@ -74,11 +74,11 @@ export class Bench extends EventTarget implements BenchLike {
7474
/**
7575
* Removes a previously registered event listener.
7676
*/
77-
declare removeEventListener: <K extends BenchEvents>(
77+
declare removeEventListener: (<K extends BenchEvents>(
7878
type: K,
7979
listener: EventListener<K> | EventListenerObject<K> | null,
8080
options?: RemoveEventListenerOptionsArgument
81-
) => void
81+
) => void) & EventTarget['removeEventListener']
8282

8383
readonly retainSamples: boolean
8484

‎src/task.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,17 @@ const startedTaskResult: TaskResult = { state: 'started' }
7272
* results, name, the task function, the number times the task function has been executed, ...
7373
*/
7474
export class Task extends EventTarget {
75-
declare addEventListener: <K extends TaskEvents>(
75+
declare addEventListener: (<K extends TaskEvents>(
7676
type: K,
7777
listener: EventListener<K, 'task'> | EventListenerObject<K, 'task'> | null,
7878
options?: AddEventListenerOptionsArgument
79-
) => void
79+
) => void) & EventTarget['addEventListener']
8080

81-
declare removeEventListener: <K extends TaskEvents>(
81+
declare removeEventListener: (<K extends TaskEvents>(
8282
type: K,
8383
listener: EventListener<K, 'task'> | EventListenerObject<K, 'task'> | null,
8484
options?: RemoveEventListenerOptionsArgument
85-
) => void
85+
) => void) & EventTarget['removeEventListener']
8686

8787
/**
8888
* The estimated effective timer resolution observed during the last run,

‎src/types.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export type { BenchEvent } from '../src/event'
77
* Options for adding an event listener
88
*/
99
export type AddEventListenerOptionsArgument = Parameters<
10-
typeof EventTarget.prototype.addEventListener
10+
EventTarget['addEventListener']
1111
>[2]
1212

1313
/**
@@ -52,11 +52,11 @@ export interface BenchLike extends EventTarget {
5252
/**
5353
* Adds a listener for the specified event type.
5454
*/
55-
addEventListener: <K extends BenchEvents>(
55+
addEventListener: (<K extends BenchEvents>(
5656
type: K,
5757
listener: EventListener<K> | EventListenerObject<K> | null,
5858
options?: AddEventListenerOptionsArgument
59-
) => void
59+
) => void) & EventTarget['addEventListener']
6060
/**
6161
* Executes tasks concurrently based on the specified concurrency mode, if set.
6262
*
@@ -77,11 +77,11 @@ export interface BenchLike extends EventTarget {
7777
/**
7878
* Removes a previously registered event listener.
7979
*/
80-
removeEventListener: <K extends BenchEvents>(
80+
removeEventListener: (<K extends BenchEvents>(
8181
type: K,
8282
listener: EventListener<K> | EventListenerObject<K> | null,
8383
options?: RemoveEventListenerOptionsArgument
84-
) => void
84+
) => void) & EventTarget['removeEventListener']
8585

8686
/**
8787
* Should samples be retained for further custom processing
@@ -462,7 +462,7 @@ export type NowFn = () => number
462462

463463
// @types/node doesn't have these types globally, and we don't want to bring "dom" lib for everyone
464464
export type RemoveEventListenerOptionsArgument = Parameters<
465-
typeof EventTarget.prototype.removeEventListener
465+
EventTarget['removeEventListener']
466466
>[2]
467467

468468
/**

‎test/events-properties.test.ts‎

Lines changed: 103 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,36 @@
1-
import { test } from 'vitest'
2-
3-
import { Bench, type Task } from '../src'
4-
5-
/**
6-
* This helper function is used to assert that a value is assignable to a specific type.
7-
* It does not produce any runtime code and is only used for type checking during development.
8-
* @param value - The value to be checked for type assignability.
9-
* @returns The same value that was passed in.
10-
*/
11-
function expectAssignable<T> (value: T) {
12-
return value
13-
}
1+
import { expectTypeOf, test } from 'vitest'
2+
3+
import {
4+
Bench,
5+
type BenchEvent,
6+
type BenchLike,
7+
type Task,
8+
} from '../src'
9+
10+
type NativeAddEventListenerParameters = Parameters<
11+
EventTarget['addEventListener']
12+
>
13+
type NativeRemoveEventListenerParameters = Parameters<
14+
EventTarget['removeEventListener']
15+
>
1416

1517
test('events properties', () => {
16-
const bench = new Bench()
18+
const bench = new Bench().add('foo', () => undefined)
19+
const benchLike: BenchLike = bench
20+
21+
// The native fallback must remain the final overload on every public surface.
22+
expectTypeOf<Parameters<Bench['addEventListener']>>()
23+
.toEqualTypeOf<NativeAddEventListenerParameters>()
24+
expectTypeOf<Parameters<Bench['removeEventListener']>>()
25+
.toEqualTypeOf<NativeRemoveEventListenerParameters>()
26+
expectTypeOf<Parameters<Task['addEventListener']>>()
27+
.toEqualTypeOf<NativeAddEventListenerParameters>()
28+
expectTypeOf<Parameters<Task['removeEventListener']>>()
29+
.toEqualTypeOf<NativeRemoveEventListenerParameters>()
30+
expectTypeOf<Parameters<BenchLike['addEventListener']>>()
31+
.toEqualTypeOf<NativeAddEventListenerParameters>()
32+
expectTypeOf<Parameters<BenchLike['removeEventListener']>>()
33+
.toEqualTypeOf<NativeRemoveEventListenerParameters>()
1734

1835
const fooTask = bench.getTask('foo')
1936

@@ -22,84 +39,124 @@ test('events properties', () => {
2239
// Task events
2340

2441
fooTask.addEventListener('abort', evt => {
25-
expectAssignable(evt.task)
26-
expectAssignable(evt.error)
42+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'abort', 'task'>>()
2743
})
2844

2945
fooTask.addEventListener('complete', evt => {
30-
expectAssignable(evt.task)
31-
expectAssignable(evt.error)
46+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'complete', 'task'>>()
3247
})
3348

3449
fooTask.addEventListener('cycle', evt => {
35-
expectAssignable(evt.task)
36-
expectAssignable(evt.error)
50+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'cycle', 'task'>>()
3751
})
3852

3953
fooTask.addEventListener('error', evt => {
40-
expectAssignable(evt.task)
41-
expectAssignable(evt.error)
54+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'error', 'task'>>()
4255
})
4356

4457
fooTask.addEventListener('reset', evt => {
45-
expectAssignable(evt.task)
46-
expectAssignable(evt.error)
58+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'reset', 'task'>>()
4759
})
4860

4961
fooTask.addEventListener('start', evt => {
50-
expectAssignable(evt.task)
51-
expectAssignable(evt.error)
62+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'start', 'task'>>()
5263
})
5364

5465
fooTask.addEventListener('warmup', evt => {
55-
expectAssignable(evt.task)
56-
expectAssignable(evt.error)
66+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'warmup', 'task'>>()
67+
})
68+
69+
fooTask.addEventListener('warning', evt => {
70+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'warning', 'task'>>()
71+
})
72+
73+
fooTask.addEventListener('custom', evt => {
74+
expectTypeOf(evt).not.toBeAny()
75+
expectTypeOf(evt).toEqualTypeOf<Event>()
76+
})
77+
78+
fooTask.removeEventListener('custom', evt => {
79+
expectTypeOf(evt).not.toBeAny()
80+
expectTypeOf(evt).toEqualTypeOf<Event>()
81+
})
82+
83+
fooTask.removeEventListener('abort', evt => {
84+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'abort', 'task'>>()
5785
})
5886

5987
// Bench events
6088

6189
bench.addEventListener('abort', evt => {
62-
expectAssignable<Task | undefined>(evt.task)
63-
expectAssignable(evt.error)
90+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'abort'>>()
6491
})
6592

6693
bench.addEventListener('add', evt => {
67-
expectAssignable(evt.task)
68-
expectAssignable(evt.error)
94+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'add'>>()
6995
})
7096

7197
bench.addEventListener('complete', evt => {
72-
expectAssignable(evt.task)
73-
expectAssignable(evt.error)
98+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'complete'>>()
7499
})
75100

76101
bench.addEventListener('cycle', evt => {
77-
expectAssignable(evt.task)
78-
expectAssignable(evt.error)
102+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'cycle'>>()
79103
})
80104

81105
bench.addEventListener('error', evt => {
82-
expectAssignable(evt.task)
83-
expectAssignable(evt.error)
106+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'error'>>()
84107
})
85108

86109
bench.addEventListener('remove', evt => {
87-
expectAssignable(evt.task)
88-
expectAssignable(evt.error)
110+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'remove'>>()
89111
})
90112

91113
bench.addEventListener('reset', evt => {
92-
expectAssignable(evt.task)
93-
expectAssignable(evt.error)
114+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'reset'>>()
94115
})
95116

96117
bench.addEventListener('start', evt => {
97-
expectAssignable(evt.task)
98-
expectAssignable(evt.error)
118+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'start'>>()
119+
})
120+
121+
bench.addEventListener('warning', evt => {
122+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'warning'>>()
99123
})
100124

101125
bench.addEventListener('warmup', evt => {
102-
expectAssignable(evt.task)
103-
expectAssignable(evt.error)
126+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'warmup'>>()
127+
})
128+
129+
bench.addEventListener('custom', evt => {
130+
expectTypeOf(evt).not.toBeAny()
131+
expectTypeOf(evt).toEqualTypeOf<Event>()
132+
})
133+
134+
bench.removeEventListener('custom', evt => {
135+
expectTypeOf(evt).not.toBeAny()
136+
expectTypeOf(evt).toEqualTypeOf<Event>()
137+
})
138+
139+
bench.removeEventListener('abort', evt => {
140+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'abort'>>()
141+
})
142+
143+
// BenchLike events
144+
145+
benchLike.addEventListener('abort', evt => {
146+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'abort'>>()
147+
})
148+
149+
benchLike.removeEventListener('abort', evt => {
150+
expectTypeOf(evt).toEqualTypeOf<BenchEvent<'abort'>>()
151+
})
152+
153+
benchLike.addEventListener('custom', evt => {
154+
expectTypeOf(evt).not.toBeAny()
155+
expectTypeOf(evt).toEqualTypeOf<Event>()
156+
})
157+
158+
benchLike.removeEventListener('custom', evt => {
159+
expectTypeOf(evt).not.toBeAny()
160+
expectTypeOf(evt).toEqualTypeOf<Event>()
104161
})
105162
})

0 commit comments

Comments
 (0)